Repository navigation
Merge the pause workflow into the suspend workflow - #2694
Merged
Dmitry Berkovich (dberkov) merged 2 commits intoOct 16, 2026
Merged
Dmitry Berkovich (dberkov) merged 2 commits into
Dmitry Berkovich (dberkov) merged 2 commits into
Conversation
1 of 2 tasks
Zoe Zhao (zoez7)
force-pushed
the
merge-suspend-pause-workflow
branch
3 times, most recently
from
October 15, 2026 05:30
7de1123 to
c2db3c1
Compare
2 tasks done
Zoe Zhao (zoez7)
force-pushed
the
merge-suspend-pause-workflow
branch
2 times, most recently
from
October 15, 2026 18:22
4cb25c9 to
84a17b5
Compare
Zoe Zhao (zoez7)
marked this pull request as ready for review
October 15, 2026 18:34
Zoe Zhao (zoez7)
force-pushed
the
merge-suspend-pause-workflow
branch
from
October 15, 2026 19:00
84a17b5 to
7ace0b4
Compare
Zoe Zhao (zoez7)
changed the title
Zoe Zhao (zoez7)
requested review from
Dmitry Berkovich (dberkov),
igooch and
Michelle Au (msau42)
and removed request for
Michelle Au (msau42)
October 15, 2026 19:01
igooch
approved these changes
Oct 15, 2026
Zoe Zhao (zoez7)
force-pushed
the
merge-suspend-pause-workflow
branch
from
October 16, 2026 00:17
7ace0b4 to
ef00332
Compare
Sairaj Pokale (sairajp-rewind)
pushed a commit
to sairajp-rewind/substrate
that referenced
this pull request
Oct 16, 2026
agent-substrate#2707) Fixes agent-substrate#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 agent-substrate#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: - RED: before the production fix, the corrected already-crashed pause test and both modes of the full-workflow regression fail because the calls incorrectly return OK. - GREEN: `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. - The full suite skips 41 tests requiring root privileges, IPv6 loopback, or configured external object storage. The affected PostgreSQL regressions do not skip. 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: - The four affected controlapi regressions pass with `-race -count=3`. - All 763 controlapi test entries and all 169 functionaltest entries pass, with no skips. The functional package was rerun after initializing the isolated source directory's Git metadata, which envtest setup requires. - `go test -race -c ./internal/e2e/suites/demo` and `go vet ./internal/e2e/suites/demo` pass. Native compilation, gofmt, and whitespace checks also pass. - Real runtime E2E has not run at this head. The new [pr-workflow run](https://github.com/agent-substrate/substrate/actions/runs/38817919692) is `action_required` and needs maintainer approval. Draft agent-substrate#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 agent-substrate#2676; that behavior should carry forward into the refactor. - [x] Local validation passes; runtime E2E awaits workflow approval. - [x] Documentation reviewed; finalize and regression-test comments describe the corrected behavior. No guide change is needed.
Dmitry Berkovich (dberkov)
previously approved these changes
Oct 16, 2026
Collaborator
|
needs rebase. looking good though |
PauseActor and SuspendActor took the same steps and differed only in the shape of the snapshot they took. Run both through one suspend workflow parameterized by a suspend mode: a sync suspend makes the snapshot durable before returning, an async suspend (PauseActor) keeps it on the worker's node and leaves the actor pinned there. Behavior is unchanged: each mode keeps its own fast-forward, rejects a SUSPENDING actor the other mode marked, and PauseActor still accepts only RUNNING, non-golden actors. Drop workflow_pause.go.
Zoe Zhao (zoez7)
force-pushed
the
merge-suspend-pause-workflow
branch
from
October 16, 2026 03:17
ef00332 to
de37ee1
Compare
Dmitry Berkovich (dberkov)
approved these changes
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
4 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.
This is the next step toward async suspend, following #2688.
SuspendActorandPauseActornow run one set of shared steps behind asuspendMode:suspendSync(SuspendActor): the snapshot is durable before the workflow returns.suspendAsync(PauseActor): the snapshot stays on the worker's node.Both APIs stay for now.
workflow_pause.gois removed.Behavior is unchanged. The tests #2688 moved testing of ensurePaused* and ensureSuspended* functions onto
PauseActorandSuspendActor. The only test edit is inworkflow_test.go, which called the removedensurePausedFinalized.The fix for #2676 (finalize not checking that the actor is still
SUSPENDING) will come in a separate PR.