Repository navigation
Require snapshot generation to be passed to atelet - #2690
Merged
Zoe Zhao (zoez7) merged 1 commit intoOct 15, 2026
Merged
Conversation
2 tasks done
Michelle Au (msau42)
force-pushed
the
snapshot-generation
branch
from
October 15, 2026 02:58
10704e7 to
e0d5efd
Compare
ateapi numbers each actor's snapshots with a generation, but its Checkpoint, PromoteSnapshot, and Restore requests to atelet left Snapshot.generation unset, so atelet saw every snapshot as generation 0. Set it from the snapshot each request names, and have atelet reject a request that leaves it unset. Atelet does not yet compare the generation with the snapshots it holds. Once it does, it can reject a Checkpoint or PromoteSnapshot that carries a stale view of the actor's snapshots, as Snapshot.generation describes.
Michelle Au (msau42)
force-pushed
the
snapshot-generation
branch
from
October 15, 2026 03:50
e0d5efd to
863b39b
Compare
Michelle Au (msau42)
marked this pull request as ready for review
October 15, 2026 03:52
| // TODO: Temporarily, atelet does not compare it with the snapshots it holds. | ||
| // | ||
| // +k8s:optional | ||
| // +k8s:required |
Collaborator
There was a problem hiding this comment.
🤖 question 🟢 – This also makes atelet reject every Checkpoint, Restore, and PromoteSnapshot that carries no generation, so an atelet upgraded ahead of ateapi fails all three until ateapi rolls. The commit title says "and require them" but the PR title and description only say "pass". Worth stating the new requirement there so the rollout order is visible to whoever deploys it.
Collaborator
Author
There was a problem hiding this comment.
are we supporting upgrades pre-1.0? I've updated the title
Collaborator
There was a problem hiding this comment.
backwards incompatible changes are fine pre-1.0
Michelle Au (msau42)
changed the title
Michelle Au (msau42)
changed the title
Zoe Zhao (zoez7)
approved these changes
Oct 15, 2026
| // TODO: Temporarily, atelet does not compare it with the snapshots it holds. | ||
| // | ||
| // +k8s:optional | ||
| // +k8s:required |
Collaborator
There was a problem hiding this comment.
backwards incompatible changes are fine pre-1.0
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
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.
Part of #2069
ateapi assigns each actor snapshot a generation, but never passed it to atelet: every
ateletpb.Snapshotit builds leftgenerationunset. That covers the pause and suspendCheckpoint, the paused-actorPromoteSnapshot, and bothRestorepaths. This sets it from the snapshot each request names, and narrows the TODO onSnapshot.generationto say only that atelet does not check it yet.Atelet requires generation to be set but doesn't use the value yet. Once #2677 lands, atelet can reject a
CheckpointorPromoteSnapshotwhose generation is stale, as theSnapshot.generationdoc describes. That check came up in review on #2677.