fix: don't record an empty node name in NodeVmsWithLocalSnapshots#399
Open
Grant McCloskey (MushuEE) wants to merge 1 commit into
Open
fix: don't record an empty node name in NodeVmsWithLocalSnapshots#399Grant McCloskey (MushuEE) wants to merge 1 commit into
Grant McCloskey (MushuEE) wants to merge 1 commit into
Conversation
When FinalizePausedStep runs after the worker record is already gone, the node holding the local checkpoint is unknown. Previously the step wrote [""] into NodeVmsWithLocalSnapshots, which findFreeWorker treats as a hard restriction no worker can satisfy - every subsequent resume fails with 'no free workers available' and the actor is permanently unschedulable short of boot=true. Write side: only record the node when it is known; an empty list means no locality restriction, so the scheduler can still place the actor (local checkpoints live on the node, so a shuffled retry can land on the node that holds it). Read side: findFreeWorker ignores empty entries in the restriction list, healing actor records already poisoned by the old write path without a data migration. Fixes agent-substrate#397
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
1 participant
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.
Fixes #397
When
FinalizePausedStepruns after the assigned worker record has already been deleted, the node holding the local checkpoint is unknown and the step wroteNodeVmsWithLocalSnapshots: [""].findFreeWorkertreats any non-empty restriction list as a hard node filter, and no worker has an empty node name — so every subsequent resume of the affected actor fails withno free workers available, permanently, unless the caller discards state withboot=true.Two guards:
FinalizePausedStep): only record the node name when it is known. An empty list means "no locality restriction", which keeps the actor schedulable — local checkpoints live on the node itself, so a shuffled retry can still land on the node that holds the data.findFreeWorker): ignore empty entries in the restriction list, healing actor records already poisoned by the old write path without a data migration.Includes unit tests for
findFreeWorker's restriction handling (none existed). Existing pause/resume functional tests pass unchanged — the normal path (node known) still records the single-element list.