Repository navigation
Suspend and pause preserve concurrent actor crashes and report failure - #2707
Merged
Zoe Zhao (zoez7) merged 2 commits intoOct 16, 2026
Merged
Conversation
ensureSuspendedFinalized and ensurePausedFinalized re-read the actor so that an out-of-band transition is not overwritten, but only the version precondition guarded that, and it covers a crash landing after the read, not before it. A worker delete or the syncer crashes an actor without taking the actor lease, and clears its worker assignment doing so. When that landed between the atelet call and finalize's read, suspend committed SUSPENDED over the crash and left the stale crash on the record, and pause found no assignment to free and reported success for a crashed actor. Both steps now require SUSPENDING and otherwise return FailedPrecondition without touching the record, as the mark steps do, so each workflow ends CRASHED and the caller is told. Cover it through the full workflows: flip the test that pinned the old outcome, and give the already-crashed pause test the shape a real crash leaves (assignment cleared) instead of one that only reached FailedPrecondition through the node-unknown branch.
Zoe Zhao (zoez7)
previously approved these changes
Oct 15, 2026
Zoe Zhao (zoez7)
enabled auto-merge
October 15, 2026 18:28
Collaborator
|
Luomu (@cloudonly) Please fix the e2e test failures below |
Worker deletion can crash the actor before suspend finalization. Check that the RPC result matches the persisted state and that a reported crash retains its completed snapshot and reason.
auto-merge was automatically disabled
October 15, 2026 22:33
Head branch was pushed to by a user without write access
Contributor
Author
|
Zoe Zhao (@zoez7) Fixed the E2E test failures, please review again. |
Zoe Zhao (zoez7)
enabled auto-merge
October 16, 2026 00:06
Zoe Zhao (zoez7)
approved these changes
Oct 16, 2026
1 of 2 tasks
Merged
via the queue into
agent-substrate:main
with commit Oct 16, 2026
a1471f0
23 of 27 checks passed
This was referenced Oct 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2676.
If worker deletion or another crash writer marks an actor CRASHED after checkpoint succeeds but before finalize re-reads it, SuspendActor currently overwrites that crash with SUSPENDED, while PauseActor reports success for the CRASHED actor.
Both finalize steps now require the re-read actor to be SUSPENDING before validating snapshots, releasing the worker, or writing the final state. Otherwise they return FailedPrecondition and preserve the existing state, crash reason, and snapshot records. The existing version precondition still rejects a concurrent crash after the read with Aborted. Completed suspend and pause calls retain their existing success paths at workflow entry.
The full-workflow regression now checks FailedPrecondition and preservation of the original crash in both modes, updating the behavior pinned by #2688. The already-crashed pause fixture also clears WorkerAssignment, matching a real crash; it previously reached FailedPrecondition only through the missing-node branch.
Validation of the production change on Linux with CI=true and real PostgreSQL test containers:
go test -race -count=1 -json ./...passes: 158 test-bearing packages and 9,670 test entries, including all 763 controlapi entries and 169 API functional entries.go build ./cmd/ateapi ./cmd/atelet ./cmd/kubectl-ate,hack/verify-all.sh, and the tools/apitool race tests pass. The verifier leaves the source tree unchanged.E2E follow-up:
The original CI passed both gVisor suites but failed both microVM suites in
TestGracefulWorkerTerminationSuspend. That test unconditionally expected SuspendActor to succeed after worker deletion, even when deletion had already crashed the actor.The test now ties the RPC result to the stored state. OK requires SUSPENDED with no crash record. FailedPrecondition or Aborted requires CRASHED, a crash reason, and the previous completed durable snapshot. Both outcomes require the worker and its assignment to be removed. All other errors fail the test.
Follow-up validation on Linux with CI=true and real PostgreSQL and envtest:
-race -count=3.go test -race -c ./internal/e2e/suites/demoandgo vet ./internal/e2e/suites/demopass. Native compilation, gofmt, and whitespace checks also pass.action_requiredand needs maintainer approval.Draft #2694 refactors these same workflows and currently preserves an out-of-band state while returning success. This focused fix also reports FailedPrecondition, as required by #2676; that behavior should carry forward into the refactor.