Repository navigation
feat: durable restate destroy with retries and a destroy_failed state - #371
Conversation
…failed dead letters and reconcile re-sends
Deploying with
|
| 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 { |
There was a problem hiding this comment.
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.
| 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, "") |
There was a problem hiding this comment.
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.
| toRemove := make([]string, 0) | ||
| for id, v := range fm.vms { | ||
| if v.state != VMStateDestroying { | ||
| if !v.state.tearingDown() { |
There was a problem hiding this comment.
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.
| fm.recordDeadLetter(ctx, DeadLetterRecord{ | ||
| Kind: DeadLetterDestroy, | ||
| EntityID: vmID, | ||
| Reason: lastErr, | ||
| Payload: dlPayload("reason", reason), | ||
| }) |
There was a problem hiding this comment.
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.
| 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) |
There was a problem hiding this comment.
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!
fuse-patterns review: #371 (feat/restate-destroy @ 1e32a07)
Verified locally: P21.
Fix: add P32. 3. 4.
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.
Fix: add the metric row and the dead-letter kind, and re-align the const block. Checked, nothing to report
Findings: 0 P1 · 1 P2 · 4 P3 |
Closes #339
Part of #334
With restate on (
FUSE_RESTATE_MODE=selforcloud), every destroy runs as theEnvironment/{vm_id}/Destroyhandler. It resumes after an orchestrator crash, retries a failing host, and when it runs out of retries ends indestroy_failedwith a dead letter instead of sitting indestroyingforever. Mode off is unchanged.What changed
Orchestrator and restate handler (
internal/orchestrator/destroy_durable.go,internal/durable/destroy.go)Environment/{vm_id}/Destroyruns the issue's steps, each journaled:begin(fail the task, persistdestroying),release_egress,host_destroy,unpublish_ingress,finish(dealloc, delete row, cancel interventions, drop grants, revoke keys). Each step is aFleetManagermethod that is safe to run again.host_destroyretries under theEnvironmentretry 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.destroy_failedwith the last error, adestroydead letter is written,orchestrator_destroy_dead_letters_totalgoes up, and the invocation fails with that error. A laterDELETEretries it.DestroyVM(api, hitl-approved destroy, migrate source) waits for it;CompleteTask, the three create-cleanupgo Destroycalls, the stuck/idle/drain expiry andretryFailedTeardownssend it one way.Destroyfor a vm indestroyingfor 2 minutes (the existingteardownRetryAfter). Each host attempt refreshes the vm's clock, so a destroy still retrying is not re-sent.orchestrator/main.gobuilds the restate client beforefm.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 likedestroyingonce 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.stateenum 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 toDELETE).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.orchestratorcannot importdurable(cycle), so the fleet takes aDurableDestroyer(set withSetDurableDestroyer) anddurable.Fleetembeds aDestroyerinterface of the steps. Later ports can each add their own step interface file, which keeps them from conflicting.ent generatewithout the repo's feature flags deletes the upsert builders.DELETEfails with a 500 instead of falling back to the in-process path; background teardowns staydestroyingand reconcile re-sends them.Testing
gofmt -l .,go vet ./...,go build ./...,go test ./... -race -count=1: pass.internal/orchestrator/destroy_durable_test.go(8): durableDELETE, 404 for an unknown vm, task completion and create cleanup send, host 404 as success, dead letter then retry, idempotent finish, reconcile re-send.internal/durable/destroy_test.goagainst 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.-race), typescript (lint, typecheck, 91 tests, build), python (ruff, mypy, 96 tests), rust (fmt, clippy-D warnings, tests).bun run buildpasses.bun run lintfails 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.FUSE_RESTATE_MODE=self:kill -9mid-destroy:fuse environment destroy <id> & sleep 1; sudo pkill -9 -f fuse-orchestrator, restart, check the vm is gone fromfuse environment listandcurl -s localhost:<agent>/v1/vm.sudo systemctl stop fc-agent, destroy, watch retries inhttp://127.0.0.1:9070/ui,sudo systemctl start fc-agent, check it completes.destroy_failedon the environment, thedestroyrow inorchestrator_dead_lettersandorchestrator_destroy_dead_letters_totalon/metrics.Not in scope
durable.Runstep; this PR only callsdurable.Run, no file overlap.Fix replacement-environment destruction, missing completion events, and skipped key revocation before merging.
Summary
Routes environment teardown through journaled Restate steps and adds the retryable
destroy_failedstate across storage, CLI, SDKs, and docs.Acknowledged choices from andrewn6: synchronous
DELETEremains 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]Reviews (1) · Last reviewed commit: "style: wrap destroy_failed sdk test fixt..." · Reviewed by Greptile