Skip to content

feat: durable restate destroy with retries and a destroy_failed state - #371

Merged
andrewn6 merged 7 commits into
mainfrom
feat/restate-destroy
Nov 14, 2026
Merged

andrewn6 merged 7 commits into
mainfrom
feat/restate-destroy

Conversation

@andrewn6

@andrewn6 andrewn6 commented Nov 13, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #339
Part of #334

With restate on (FUSE_RESTATE_MODE=self or cloud), every destroy runs as the Environment/{vm_id}/Destroy handler. It resumes after an orchestrator crash, retries a failing host, and when it runs out of retries ends in destroy_failed with a dead letter instead of sitting in destroying forever. Mode off is unchanged.

What changed

Orchestrator and restate handler (internal/orchestrator/destroy_durable.go, internal/durable/destroy.go)

  • Environment/{vm_id}/Destroy runs the issue's steps, each journaled: begin (fail the task, persist destroying), release_egress, host_destroy, unpublish_ingress, finish (dealloc, delete row, cancel interventions, drop grants, revoke keys). Each step is a FleetManager method that is safe to run again.
  • host_destroy retries under the Environment retry policy (FUSE_RESTATE_RETRY_ENVIRONMENT_*, default 10 attempts, 500ms to 30s). A host 404 counts as done, so a replay after the host already destroyed the vm does not dead-letter.
  • Out of retries: the vm moves to destroy_failed with the last error, a destroy dead letter is written, orchestrator_destroy_dead_letters_total goes up, and the invocation fails with that error. A later DELETE retries it.
  • Every teardown goes through the handler: DestroyVM (api, hitl-approved destroy, migrate source) waits for it; CompleteTask, the three create-cleanup go Destroy calls, the stuck/idle/drain expiry and retryFailedTeardowns send it one way.
  • Reconcile re-sends Destroy for a vm in destroying for 2 minutes (the existing teardownRetryAfter). Each host attempt refreshes the vm's clock, so a destroy still retrying is not re-sent.
  • orchestrator/main.go builds the restate client before fm.Start, so the first reconcile teardown already goes through restate.

New state destroy_failed (api, ent schema, swagger, CLI, all four SDKs, with a test in each)

  • VMStateDestroyFailed; it counts toward host allocation (the vm may still be on the host) and is reaped like destroying once the host no longer lists it. hitl requests and the intervention sweep treat it as gone.
  • StateDestroyFailed / STATE_DESTROY_FAILED / State.DestroyFailed / EnvironmentState::DestroyFailed, not terminal in any sdk. The cli renders it red.
  • ent state enum and its validator; the column is varchar, no migration.

Docs: concepts/environments.mdx (state), concepts/reconcile.mdx (durable teardown), go/python/typescript environments references, cli/environment/list.mdx, docs/runbooks/restate.mdx (stuck destroys, what a restate outage does to DELETE).

Choices worth a look

  • DELETE /v1/environments/{id} stays synchronous (204/404/500): it calls the handler through ingress and waits, bounded by the 30s ingress client timeout. A destroy that outlives it returns a 500 and keeps running in restate. The issue says one-way sends; I kept the api contract until [restate] operations api and Idempotency-Key across api, cli and all four sdks #340 adds operation handles.
  • orchestrator cannot import durable (cycle), so the fleet takes a DurableDestroyer (set with SetDurableDestroyer) and durable.Fleet embeds a Destroyer interface of the steps. Later ports can each add their own step interface file, which keeps them from conflicting.
  • ent code edited by hand for the enum: ent generate without the repo's feature flags deletes the upsert builders.
  • With restate down, DELETE fails with a 500 instead of falling back to the in-process path; background teardowns stay destroying and reconcile re-sends them.

Testing

  • gofmt -l ., go vet ./..., go build ./..., go test ./... -race -count=1: pass.
  • New unit tests in internal/orchestrator/destroy_durable_test.go (8): durable DELETE, 404 for an unknown vm, task completion and create cleanup send, host 404 as success, dead letter then retry, idempotent finish, reconcile re-send.
  • New integration tests in internal/durable/destroy_test.go against a local restate-server (FUSE_RESTATE_TEST_ADMIN_URL/_INGRESS_URL): retries until the host recovers without re-running earlier steps, dead letter after the configured attempts, cleanup of an untracked vm on its named host. They skip without restate-server, as the existing ones do.
  • sdks: go (-race), typescript (lint, typecheck, 91 tests, build), python (ruff, mypy, 96 tests), rust (fmt, clippy -D warnings, tests).
  • docs: bun run build passes. bun run lint fails locally on 8 files this branch does not touch (meta.ts, biome.json, tsconfig.json) with a freshly resolved biome; CI covers it.
  • golangci-lint: not run locally, CI covers it.
  • Not run: the issue's phantom acceptance. No script covers it yet; with the orchestrator on FUSE_RESTATE_MODE=self:
    • kill -9 mid-destroy: fuse environment destroy <id> & sleep 1; sudo pkill -9 -f fuse-orchestrator, restart, check the vm is gone from fuse environment list and curl -s localhost:<agent>/v1/vm.
    • fc-agent down: sudo systemctl stop fc-agent, destroy, watch retries in http://127.0.0.1:9070/ui, sudo systemctl start fc-agent, check it completes.
    • dead letter: keep fc-agent down past the retries, check destroy_failed on the environment, the destroy row in orchestrator_dead_letters and orchestrator_destroy_dead_letters_total on /metrics.

Not in scope

RetriggerView in GreptileConfidence Score: 1/5

Fix replacement-environment destruction, missing completion events, and skipped key revocation before merging.

Fix All in Claude CodeFindings

  1. P1 Old cleanup destroys replacements ▶
  2. P1 Retries lose lifecycle events ▶
  3. P1 Security Reaped environments leave active keys ▶
  4. P2 Failed destroys lose dead letters ▶
  5. P2 Slow sends stall reconciliation ▶

Summary

Routes environment teardown through journaled Restate steps and adds the retryable destroy_failed state across storage, CLI, SDKs, and docs.

  • Restate keeps environment teardown moving through host failures and orchestrator restarts.
  • Environments show when teardown runs out of retries.

Acknowledged choices from andrewn6: synchronous DELETE remains until #340; host idempotency keys are deferred to #338 / PR #370; reconciliation intentionally resends without first checking Restate.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Destroy request] --> B[Begin]
  B --> C[Release egress]
  C --> D[Destroy on host]
  D -->|Retry| D
  D -->|Success| E[Unpublish ingress]
  E --> F[Finish and revoke keys]
  D -->|Retries exhausted| G[destroy_failed and dead letter]
  G -->|Later DELETE| B
  G -->|Host no longer lists VM| H[Reaper removes record]
  H --> I[Key cleanup skipped]
Loading

Reviews (1) · Last reviewed commit: "style: wrap destroy_failed sdk test fixt..." · Reviewed by Greptile

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Nov 13, 2026 •

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
fuse 5d539e0 Commit Preview URL

Branch Preview URL
Nov 14 2026, 10:17 PM

if d := fm.durableDestroyer(); d != nil {
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
if err := d.SendDestroy(ctx, vmID, DestroyRequest{Reason: DestroyReasonCreateCleanup, HostID: hostID}); err != nil {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Old cleanup destroys replacements

After a failed create, a retry with the same task ID reuses the same VM ID. If the queued create_cleanup invocation starts afterward, DestroyBegin selects the replacement and its host, then destroys it. Create does not run through Restate, so the object's lock does not prevent this.

Keep the old attempt's identity through cleanup, or block reuse until cleanup finishes.

Fix in Claude Code

Comment on lines +237 to +243
if err := fm.store.DeleteVM(ctx, vmID); err != nil {
return fmt.Errorf("delete vm %s row: %w", vmID, err)
}
}
if ok {
fm.appendEvent(ctx, "vm", vmID, "vm.removed", map[string]any{"reason": reason})
fm.publishTerminalEvent(vmID, VMStateDestroyed, "")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Retries lose lifecycle events

DestroyFinish removes the VM from memory before DeleteVM succeeds. If that write fails once, the next attempt finds no VM and skips publishTerminalEvent, even when deletion succeeds. Existing event streams then keep sending heartbeats instead of reporting completion.

DestroyBegin has the same ordering problem: a failed persist leaves already true and suppresses the start event on retry. Preserve pending events across retries rather than using the changed memory state to decide whether they were sent.

Fix in Claude Code

toRemove := make([]string, 0)
for id, v := range fm.vms {
if v.state != VMStateDestroying {
if !v.state.tearingDown() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 security Reaped environments leave active keys

When a destroy_failed VM disappears from the host, this reaper deletes its record without dropping grants or revoking scoped keys. This can happen when the host removed the VM but its response was lost, or when someone later removes it on the host. The failed invocation has already ended without reaching DestroyFinish.

Run the remaining cleanup before forgetting the VM. Otherwise its keys remain usable until expiry, including against surviving forks covered by EnvCovered.

How this was verified: The failed destroy returns before DestroyFinish, while the reaper deletes the record without calling the grant deletion or key revocation methods.

Fix in Claude Code

Comment on lines +269 to +274
fm.recordDeadLetter(ctx, DeadLetterRecord{
Kind: DeadLetterDestroy,
EntityID: vmID,
Reason: lastErr,
Payload: dlPayload("reason", reason),
})

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Failed destroys lose dead letters

DestroyFailed calls the best-effort recordDeadLetter helper, which only logs write failures. It then reports success to the journaled step, so Restate never retries a failed dead-letter write. The VM stays in destroy_failed, but the dead letter is missing and the counter still increases.

Return the write error from this durable path so it can be retried.

Fix in Claude Code

Comment on lines +76 to +79
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
if err := d.SendDestroy(ctx, vmID, DestroyRequest{Reason: reason}); err != nil {
fm.logger.Error("send durable destroy failed, reconcile will retry", "vm", vmID, "reason", reason, "err", err)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Slow sends stall reconciliation

destroyAsync now waits for each send inside the reconcile loop. If Restate accepts connections but does not answer, each VM takes up to ten seconds before the next send starts. A batch of 100 expired VMs can hold up reconciliation for roughly 17 minutes, delaying health checks and other cleanup.

Send outside the loop's main work, with a bounded number of concurrent sends.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code

@andrewn6

Copy link
Copy Markdown
Collaborator Author

fuse-patterns review: #371 (feat/restate-destroy @ 1e32a07)

check.sh origin/main: no FAIL. The 4 WARNs (version.ts 0.21.0 vs 0.41.0, three fc-agent tests not wired into ci) are standing debt on main. This diff touches neither area, so they are dismissed.

Verified locally: go build ./..., gofmt clean, the new orchestrator unit tests pass, and the internal/durable restate integration tests (TestDestroyThroughRestate, TestDestroyDeadLettersThroughRestate, TestDestroyCleanupOfUntrackedVM, plus Run/Ping) pass against a local restate-server 1.6.2. When host_destroy runs out of retries it does return a terminal error, so the dead-letter branch is reachable.

P2

1. internal/api/openapi.yaml was not given destroy_failed (pattern 4). The swag annotation and swagger.{json,yaml} were updated, but the hand-written openapi spec, which the api reference docs are generated from, still lists only the old states:

  • :277: GET /v1/environments state query enum [provisioning, running, draining, destroying]
  • :3022: Environment.state enum, same list. GET /v1/environments/{id} now returns "state":"destroy_failed", which falls outside the documented enum, so a validating client rejects it.
  • :3103: SSE state event enum [..., destroying, destroyed, failed]. DestroyFailed publishes a destroy_failed state change on the stream.
  • :811: the event-semantics transition list.

Fix: add destroy_failed to all three enums, and add the destroying -> destroy_failed edge to the :811 prose.

P3

2. cli/environment_cmd.go:108: the --state help text leaves out destroy_failed (pattern 10/docs drift). docs/content/reference/cli/environment/list.mdx was updated but the flag help was not. Fix: add destroy_failed to the help string.

3. internal/orchestrator/fleet.go:1726: reapDestroyedVMs now reaps destroy_failed VMs without closing them out. Example: a host reboots and stops listing a destroy_failed VM. The VM is then deallocated and its row deleted, but nothing runs unpublishIngress, revokeEnvKeys or dropEnvGrants, because the durable handler that would have run DestroyFinish already ended in the dead-letter branch. (reconcileInterventions does sweep its interventions.) The reaper has the same gap for destroying in mode off, so the pattern predates this PR. In mode self/cloud, though, a destroy_failed VM is the only one that loses those steps, since a destroying VM still gets DestroyFinish from its handler. Fix: have the reaper run the same closeout as DestroyFinish for reaped VMs, or SendDestroy a reaped destroy_failed VM so finish runs.

4. internal/orchestrator/destroy_durable.go:78 (via reconcile.go:412, reconcile.go:526, fleet.go:1310): destroyAsync now blocks on the ingress send (pattern 8). These call sites used to be go fm.destroyAndRemove(...). Now each one runs SendDestroy inline under a 10s timeout, which means:

  • retryFailedTeardowns sends serially in its loop.
  • failExpiredVM sends on the reconcile goroutine.
  • CompleteTask sends inside the api request.

If restate ingress is unreachable without refusing connections (cloud mode over the internet, or a blackholed tunnel), every re-send or expiry stalls the reconcile cycle for up to 10s per VM. A local restate that is down refuses fast, so phantom-style self mode is mostly unaffected. Fix: send from a goroutine, or give the retry loop one shared deadline.

5. Docs gaps.

  • docs/content/docs/concepts/reconcile.mdx:266: the metrics table does not list orchestrator_destroy_dead_letters_total.
  • reconcile.mdx:223: the dead-letters paragraph does not name the new destroy kind (it is mentioned only in the new paragraph at :167).
  • docs/content/reference/go/environments.mdx:174-180: the const block is half re-aligned. StateProvisioning/StateRunning/StateDraining keep the old width while the rest use the new one, so the block is not gofmt-shaped.

Fix: add the metric row and the dead-letter kind, and re-align the const block.

Checked, nothing to report

  • Pattern 1: an unknown VM still returns 404 (ErrVMNotFound is checked before restate is called). The 500 when restate is down or a destroy outlives the 30s ingress timeout is a documented choice in the PR body.
  • Pattern 3 (sdk parity): destroy_failed was added in go/ts/python/rust, non-terminal in each, with a test in each.
  • Ent: the enum validator and migrate schema were updated, and the column is varchar with no CHECK, so persisting destroy_failed works.
  • Allocation: recomputeHostAllocations counts destroy_failed as live, which matches the "still holds its allocation" claim. hasTrackedDestroyingVM excluding it is consistent with that.
  • Retry clock: one attempt is at most 60s, plus 30s max backoff, which stays under the 2 min teardownRetryAfter, so reconcile does not re-send a destroy that is still retrying.
  • Lock safety: no destroyAsync caller holds fm.mu.
  • Patterns 5/6/7/11/14: the diff does not touch host-agent, shell helpers, secrets paths or new python tests.
  • Pattern 12: base is main, gofmt is clean, and go build ./... passes.

Findings: 0 P1 · 1 P2 · 4 P3

@andrewn6
andrewn6 merged commit 0bd99a6 into main Nov 14, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[restate] durable destroy on the Environment virtual object

1 participant