Crash a paused actor on resume when its node is gone by igooch · Pull Request #3010 · agent-substrate/substrate

igooch · 2026-10-20T21:24:59Z

#2883 lets resume forget an unreachable node when a paused actor's latest snapshot is durable. When that upload is still PENDING, the actor stays pinned. If the node was deleted, every resume returns Unavailable, and nothing says the snapshot is lost.

Now, when resume finds no atelet on the pinned node and NodeGone reports the Node object deleted, it reads the actor again. If the upload was recorded during the lookup, it detaches as usual. Otherwise it crashes the actor. The crash keeps assigned_node and the PENDING snapshot. RevertActor then skips the terminate, deletes the pending objects, and returns the actor to its last completed durable snapshot. A node that still exists, or a failed Node lookup, keeps the current retry.

There are two known limits. The check runs only after resume finds room on another node, so with no room anywhere resume still returns RESOURCE_EXHAUSTED. And a Node object deleted while the machine keeps running crashes the actor even though its checkpoint is still on that machine. That's the same trade #2486 makes.

A PENDING copy may already be fully uploaded if atelet died before reporting it. Resume still crashes the actor, and revert deletes that copy. Recovering it needs ate-api-server to check for the manifest, which is a follow-up.

No proto changes. ResumeActor can now crash the actor and return FAILED_PRECONDITION in this case, and docs/api-guide.md says so.

Part of #2470

Supersedes #3005. Its "upload completes during lookup" regression case is adapted here.

AI use: Claude Code helped write the change and tests. I reviewed them.

Tests pass
Docs updated

Zoe Zhao (zoez7) · 2026-10-21T13:52:16Z


+// latestLocalNotDurable reports whether the actor's latest local snapshot has
+// no completed durable copy.
+func latestLocalNotDurable(actor *ateapipb.Actor) bool {


One edge case is: although the latest snapshot was not recorded as durable, atelet has uploaded the snapshots already (and failed to call ateapi to ReportActorSnapshotStatus)
Wdyt of optimistically try to resume the actor on a different node, atelet can try to download the snapshot object, if atelet gets a not found error, we then crash the actor.

substrate/cmd/atelet/main.go

Lines 1615 to 1639 in d04d280

var sandboxRec *sandboxAssetsRecord

localRec, localErr := readLocalSnapshot(localDir)

durable := req.GetSnapshot().GetObject()

if localErr == nil {

directLocal = true

checkpointDir = localDir

sandboxRec = localRec

} else {

if errors.Is(localErr, errUnusableLocalSnapshot) {

// The next checkpoint or terminate prunes the unusable copy.

slog.WarnContext(ctx, "Local snapshot is unusable; restoring from object storage",

slog.String("local_dir", localDir), slog.Any("err", localErr))

}

manifest, err := s.fetchManifestCached(ctx, actor, durable.GetSnapshotUri(), snapshotCacheMode(durable, cacheModeLink))

if status.Code(err) == codes.NotFound {

return nil, apierror.NotFound("snapshot %q has no usable local copy and no uploaded copy: %w",

req.GetSnapshot().GetSnapshotUuid(), localErr)

}

if err != nil {

return nil, fmt.Errorf("while fetching snapshot manifest: %w", err)

}

if sandboxRec, err = unmarshalSandboxRecord(manifest); err != nil {

return nil, fmt.Errorf("while unmarshalling sandbox record: %w", err)

}

}

Agreed, this case is real. The manifest goes last, so a PENDING copy with a manifest is complete, and revert would delete it as abandoned. I'd like to do it as a follow-up PR, though. ate-api-server can't check for the manifest today, because ControlProvider only has CleanupSnapshot and CopySnapshot. So it needs either a new RPC there, or a resume that restores the PENDING copy and turns atelet's NotFound into this crash. Which would you prefer? Until then, this PR still beats an actor that's stuck forever. The crash keeps the PENDING record, so nothing is deleted until someone reverts.

I think "a resume that restores the PENDING copy and turns atelet's NotFound into this crash" is simpler

Davanum Srinivas (dims) · 2026-10-21T14:44:07Z

some overlap with #3005?

Eren Akbulut (eren23) · 2026-10-21T16:06:47Z

Thanks again igooch for reviewing #2486 and picking up the resume follow-up; I'm closing #3005 in favor of this one. One case from that work may be useful here: the latest upload can become COMPLETED while NodeGone is in flight. Could we reload the actor and recheck that the latest snapshot is still incomplete before committing the crash, taking the existing detach path if it completed? crashActor reloads the actor but doesn't recheck snapshot durability, so that timing can still crash an actor with a completed snapshot. The "upload completes during lookup" regression case in #3005 covers this and is available to reuse.

igooch · 2026-10-21T17:16:29Z

Thanks, good catch. Folded into ada3183. Resume now reads the actor again after NodeGone. If the latest snapshot became durable, it takes the detach path. Otherwise it crashes only the version it checked, so a concurrent write aborts the resume instead. I adapted your "upload completes during lookup" case as a row in TestDetachFromFullNode.

Zoe Zhao (zoez7) · 2026-10-21T17:38:06Z

LGTM. Please fix the CI failures.

A paused actor whose node was deleted before atelet uploaded its latest snapshot stayed pinned to that node. Every resume returned Unavailable, and nothing told the caller the snapshot was lost. When resume finds no atelet on the node and the Node object is gone, it now crashes the actor. Revert then returns it to its last completed durable snapshot. A node that still exists, or a failed Node lookup, keeps the current retry.

Resume crashed the actor on a deleted node whenever it could not detach without atelet. That also covered an actor with a crash record whose latest snapshot was durable, and recorded a false crash. Resume now checks that the latest local snapshot has no completed durable copy. Move the doc sentence to a ResumeActor errors bullet, note the capacity precondition, and qualify the detach error rule.

When the caller went away or the lease was lost while resume checked whether the node still exists, it returned Unavailable. It now returns the context error, as the DetachActor call below it already does. Note in the test that a SUSPENDED actor never carries a crash record.

An upload recorded while resume checked the Node could still crash the actor, since crashActor reloads it without rechecking durability. Resume now reads the actor again after the lookup. If its latest snapshot became durable, it detaches. Otherwise it crashes only the version it checked, and a concurrent write aborts the resume for a retry. The regression case is adapted from eren23's closed alternative.

crashActorVersion assigned to an err declared by the GetActor call that moved to crashActor. That only compiled because releaseWorker declared err as well. Declare it on the UpdateActor call instead.

The docs no longer speak of paused actors. Say a suspended actor loses its local snapshot instead.

…strate#3094) Part of agent-substrate#2470, follow-up from [3010](agent-substrate#3010 (comment)). atelet can upload a suspended actor's latest local snapshot and die before it reports the upload. If the node is then deleted, the upload is complete but still recorded `PENDING`. Resume crashed the actor, and a revert then deleted the upload. Resume now releases the deleted node and restores the snapshot on another node. atelet there has no local copy, so it reads the upload. The manifest is written last, so a missing or partial upload returns `NOT_FOUND`. - `recordGoneNode` clears `assigned_node`. The snapshot keeps its old locality, so `snapshotToResume` still picks it. - A `NOT_FOUND` from that restore crashes the actor with the existing gone-node message. - `finalizeRunning` records the restored snapshot `COMPLETED` and clears its locality. - `canRelaxRuntime` lets the snapshot fall back to another runtime, as a durable snapshot can. No proto changes. Resume claims a worker before it knows the upload exists, so a missing upload costs one placement. A revert still deletes an unreported upload, so the snapshot is lost if its restore fails for another reason or the actor is reverted before it resumes. AI use: Claude Code helped write the change and tests. I reviewed them. - [ ] Tests pass - [x] Docs updated

igooch force-pushed the resume-node-gone branch from 087562b to dd76dd0 Compare October 20, 2026 23:46

igooch marked this pull request as ready for review October 20, 2026 23:46

igooch requested review from Michelle Au (msau42) and Zoe Zhao (zoez7) October 21, 2026 01:24

Michelle Au (msau42) reviewed Oct 21, 2026

View reviewed changes

Comment thread cmd/ateapi/internal/controlapi/workflow_resume_detach.go Resolved

Comment thread cmd/ateapi/internal/controlapi/workflow_resume_detach_test.go Resolved

igooch requested a review from Michelle Au (msau42) October 21, 2026 04:42

Zoe Zhao (zoez7) reviewed Oct 21, 2026

View reviewed changes

Davanum Srinivas (dims) mentioned this pull request Oct 21, 2026

Crash suspended actors when their latest snapshot is lost with the node #3005

Closed

igooch requested a review from Zoe Zhao (zoez7) October 21, 2026 17:18

igooch added 5 commits October 21, 2026 11:06

Scope the update error in crashActorVersion

44ac323

crashActorVersion assigned to an err declared by the GetActor call that moved to crashActor. That only compiled because releaseWorker declared err as well. Declare it on the UpdateActor call instead.

igooch force-pushed the resume-node-gone branch from 07014ce to 44ac323 Compare October 21, 2026 18:10

Describe the lost snapshot in current resume terms

d431264

The docs no longer speak of paused actors. Say a suspended actor loses its local snapshot instead.

Zoe Zhao (zoez7) approved these changes Oct 21, 2026

View reviewed changes

Zoe Zhao (zoez7) enabled auto-merge October 21, 2026 18:17

Zoe Zhao (zoez7) added this pull request to the merge queue Oct 21, 2026

Merged via the queue into agent-substrate:main with commit dff068d Oct 21, 2026
15 checks passed

igooch mentioned this pull request Oct 21, 2026

Resume restores an uploaded snapshot when its node is gone #3094

Merged

1 of 2 tasks

Eren Akbulut (eren23) mentioned this pull request Oct 25, 2026

[Bug]: Since #2222, revert and delete --any-state cannot clear an actor whose node is gone #2470

Closed

2 tasks done

igooch deleted the resume-node-gone branch October 27, 2026 18:26

Navigation Menu

Repository navigation

Crash a paused actor on resume when its node is gone - #3010

Crash a paused actor on resume when its node is gone#3010
Zoe Zhao (zoez7) merged 6 commits into
agent-substrate:mainfrom
igooch:resume-node-gone

igooch commented Oct 20, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Zoe Zhao (zoez7) Oct 21, 2026

Uh oh!

igooch Oct 21, 2026

Uh oh!

Zoe Zhao (zoez7) Oct 21, 2026

Uh oh!

Davanum Srinivas (dims) commented Oct 21, 2026

Uh oh!

Eren Akbulut (eren23) commented Oct 21, 2026

Uh oh!

igooch commented Oct 21, 2026

Uh oh!

Zoe Zhao (zoez7) commented Oct 21, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	var sandboxRec *sandboxAssetsRecord
	localRec, localErr := readLocalSnapshot(localDir)
	durable := req.GetSnapshot().GetObject()
	if localErr == nil {
	directLocal = true
	checkpointDir = localDir
	sandboxRec = localRec
	} else {
	if errors.Is(localErr, errUnusableLocalSnapshot) {
	// The next checkpoint or terminate prunes the unusable copy.
	slog.WarnContext(ctx, "Local snapshot is unusable; restoring from object storage",
	slog.String("local_dir", localDir), slog.Any("err", localErr))
	}
	manifest, err := s.fetchManifestCached(ctx, actor, durable.GetSnapshotUri(), snapshotCacheMode(durable, cacheModeLink))
	if status.Code(err) == codes.NotFound {
	return nil, apierror.NotFound("snapshot %q has no usable local copy and no uploaded copy: %w",
	req.GetSnapshot().GetSnapshotUuid(), localErr)
	}
	if err != nil {
	return nil, fmt.Errorf("while fetching snapshot manifest: %w", err)
	}
	if sandboxRec, err = unmarshalSandboxRecord(manifest); err != nil {
	return nil, fmt.Errorf("while unmarshalling sandbox record: %w", err)
	}
	}

Conversation

igooch commented Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Zoe Zhao (zoez7) Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

igooch Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

Zoe Zhao (zoez7) Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

Davanum Srinivas (dims) commented Oct 21, 2026

Uh oh!

Eren Akbulut (eren23) commented Oct 21, 2026

Uh oh!

igooch commented Oct 21, 2026

Uh oh!

Zoe Zhao (zoez7) commented Oct 21, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

igooch commented Oct 20, 2026 •

edited

Loading