Skip to content

Suspend and pause finalize don't check the actor is still SUSPENDING after an out-of-band crash #2676

Closed

Description

@zoez7

Neither finalize step checks that the actor it re-reads is still SUSPENDING. An actor that was crashed out-of-band before finalize re-read it comes out wrong in both workflows:

  • ensureSuspendedFinalized (cmd/ateapi/internal/controlapi/workflow_suspend.go) commits SUSPENDED over the crash.
  • ensurePausedFinalized (cmd/ateapi/internal/controlapi/workflow_pause.go) keeps the crash, but PauseActor reports success.

Scenario

  1. SuspendActor or PauseActor marks the actor SUSPENDING, and the atelet call succeeds.
  2. Before finalize runs GetActor, the DeleteWorker workflow (which takes no actor lease) crashes the actor: state CRASHED, Crash set, WorkerAssignment cleared in the same update. crashActor clears the assignment the same way.
  3. Finalize reads the CRASHED record.
    • Suspend: the latest snapshot still has its PENDING durable snapshot, so the DurableSnapshotMissing check passes. The release is skipped because there is no assignment. The UpdateActor precondition is pinned to the CRASHED version it just read, so the update succeeds. The actor ends up SUSPENDED, with the durable snapshot COMPLETED and AssignedNode cleared, but still carrying the stale Crash from step 2.
    • Pause: with no assignment, finalize returns the record unchanged. The actor stays CRASHED, which is right, but PauseActor returns OK, so the caller is told the pause succeeded.

Expected

Both finalize steps say they re-read the actor "so an out-of-band transition (e.g. the syncer crashing the actor after its worker died) is not overwritten". When the re-read actor is not SUSPENDING, finalize should leave the record alone and fail with FailedPrecondition, as the mark steps do for a CRASHED actor. Both workflows would then end CRASHED with FailedPrecondition.

The precondition only protects against a crash landing between finalize's read and its write: that case fails Aborted (covered by the KeepsCrashFromWorkerDelete tests). It does not protect against a crash landing before the read.

TestPauseActor_Finalized_AlreadyCrashed gets FailedPrecondition only because its crash keeps the worker assignment and has no AssignedNode, so pause finalize takes the node-unknown crash branch. A real crash clears the assignment, and then pause returns OK.

Notes

#2688 pins the current behavior of both workflows in TestSuspendAndPause_FinalizeWithoutAssignment, so that the upcoming refactor that merges the pause and suspend workflows stays behavior-neutral. The fix should update that pin: sync state SUSPENDED → CRASHED, and both codes OK → FailedPrecondition.

Activity

  1. changed the title [-]SuspendActor finalize overwrites an out-of-band CRASHED with SUSPENDED[/-] [+]Suspend and pause finalize don't check the actor is still SUSPENDING after an out-of-band crash[/+] on Oct 15, 2026
  2. added a commit that references this issue on Oct 16, 2026
    a1471f0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions