Repository navigation
[Bug]: Retrying a Checkpoint whose snapshot already committed crashes the actor #2836
Description
Bug
If ate-api-server retries Atelet.Checkpoint after atelet has already committed the snapshot, the actor is crashed even though its snapshot is intact.
Expected behaviour
Atelet.Checkpoint should be idempotent on (actor_uid, snapshot_uuid).
- If the tracker has already committed this UUID with the same generation, fidelity and URI, Checkpoint returns the current ActorSnapshotState without calling the worker.
- On the ate-api side: FinalizeSuspended then completes and the actor ends SUSPENDED.
Suggested fix
// snapshottracker/lifecycle.go
func (t *Tracker) BeginCheckpoint(ctx, actorUID, snap) (committed *ActorSnapshotState, end func(), err error) {
lock(actorUID)
a := waitForDetachLocked(ctx, actorUID, checkGeneration(snap))
if a != nil {
// same uuid as LatestLocal / LatestDurable / Uploading ?
if ref := a.committedAs(snap.uuid); ref != nil {
if ref.generation != snap.generation || ref.uri != snap.uri || ref.fidelity != snap.fidelity {
return nil, nil, FailedPrecondition("snapshot %s committed with different params")
}
return a.state(), noop, nil
}
}
return nil, holdLocked(actorUID, a, snap, false), nil
}
// atelet main.go Checkpoint
committed, end, err := s.snapshots.BeginCheckpoint(ctx, uid, req.Snapshot)
if err != nil { return nil, err }
defer end()
if committed != nil {
if req.UploadMode == IMMEDIATE && committed.LatestDurable.uuid != req.uuid {
// first attempt was DEFERRED, retry is sync (mode can change on retry)
return nil, FailedPrecondition(...) // or: upload from the local copy, see below
}
return &CheckpointResponse{State: committed}, nil
}
... // existing path unchanged
Activity
chw120 commentedon Oct 19, 2026
I was planning to fold this suggested fix into #2602, but noticed that #2849 has already implemented it (along with the
BeginStartwait andIMMEDIATEpromotion on retry). The only remaining piece in #2602 is detaching the in-flightAtelet.Checkpointfrom caller cancellation (context.WithoutCancel(ctx)afterBeginCheckpoint), so that when a caller times out mid-checkpoint, the first attempt continues to completion and commits the snapshot instead of being canceled after the worker has already deleted the workload. We can either let #2849 merge first and narrow #2602 down to just thatWithoutCancelchange, or include it directly in #2849.Separately, #2623 covers the
ateworker-side cleanup when an earlier checkpoint attempt fails mid-flight beforeateletcommits the snapshot — without it, the next retry intoateworkerstill fails on leftover partial files in gVisor'scheckpoint-state/(runsc checkpointexit status 128) or a partially moved base in microVM'srestore-state/.msau42 commentedon Oct 19, 2026
I'm exploring if there's ways to simplify #2849. It's getting a bit complex trying to keep tracking of ongoing RPCs and I'm exploring a few things:
chw120 commentedon Oct 19, 2026
Michelle Au (@msau42) Sounds good! In case it's useful for what you're exploring: earlier revisions of #2602 up until f42b97b actually used *actorlock.Locks directly on Atelet in cmd/atelet/main.go to serialize Checkpoint per actor (before we rebased onto #2778 and moved the wait into BeginCheckpoint).
Feel free to take a look at that commit to see if it aligns with what you have in mind — if the actorlock.Locks approach makes more sense, I can also revert #2602 back to that version (with context.WithoutCancel). Please let me know which way you think is better to approach this.
atelet: Serialize lifecycle RPCs per actor (agent-substrate#2890)