Repository navigation
feat(token-capture): retire and delete complete-record captures - #3939
Merged
Merged
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
This was referenced Oct 2, 2026
Merged
Merged
ananthsub
force-pushed
the
ananthsub/token-capture-store-retire
branch
from
October 6, 2026 19:28
079db1a to
673025f
Compare
ananthsub
force-pushed
the
ananthsub/token-capture-store-retire
branch
from
October 6, 2026 20:54
673025f to
de83018
Compare
ananthsub
force-pushed
the
ananthsub/token-capture-store-retire
branch
from
October 6, 2026 22:42
de83018 to
6faa172
Compare
ananthsub
force-pushed
the
ananthsub/token-capture-store-retire
branch
from
October 8, 2026 15:08
6faa172 to
d755c67
Compare
ananthsub
force-pushed
the
ananthsub/token-capture-store-retire
branch
from
October 8, 2026 21:30
d755c67 to
61fd7cd
Compare
ananthsub
force-pushed
the
ananthsub/token-capture-store-retire
branch
from
October 8, 2026 21:32
61fd7cd to
bd354a1
Compare
macandro96
reviewed
Oct 10, 2026
macandro96
left a comment
Contributor
There was a problem hiding this comment.
Both cases below are race conditions for very unlikely cases but fixes seem cheap
ananthsub
force-pushed
the
ananthsub/token-capture-store-retire
branch
from
October 10, 2026 01:42
2b04094 to
61e6945
Compare
With external staging the model server keeps a capture ledger per rollout and never removes it, because only the training framework knows when a rollout's manifest is no longer needed. Give the ledger the two operations the complete-record token store already has. CaptureLedger.retire removes a rollout's ledger and leaves an empty <id>.lineage.retired fence, like the fence TokenSource.drop leaves: the framework retires every rollout it is done with, finished or abandoned, and later calls for it are discarded instead of starting a new ledger. That covers an abandoned agent that keeps running and a client retry that ran a finished rollout twice. CaptureLedger.delete removes the ledger and the fence, like TokenCaptureStore.delete: the framework deletes retired rollouts once nothing of them can still write, and deletes a rollout ID before reusing it. Both are batched, validate the whole batch first, reject a bare string, and are safe to retry, behind bearer-protected POST .../rollouts/retire and .../rollouts/delete routes with matching RolloutControlClient methods. Writes look for the fence only when they would create the ledger, so a captured call costs no extra metadata lookup. Staged token data is untouched; the framework that staged it removes it. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
…yncs RolloutControlClient now validates and deduplicates the whole list of rollout IDs before sending the first batch. Before, a bare string was split into one-character IDs, an invalid ID in a later batch failed only after earlier batches had been retired, and an ID repeated across batches was reported both removed and absent. Retire no longer touches fences that already exist, and retire and delete sync the directory only when they changed something. A retire with a ledger left to delete still syncs first, so a fence left by a crashed earlier retire is durable before its ledger goes. The manifest of a retired rollout now raises RolloutRetiredError, which the manifest route returns as HTTP 410, instead of an empty manifest that reads like a rollout with no calls. The warning for a discarded late row names the call's staging key, since its staged tokens are in no manifest. retire and delete declare their result as the RolloutRemovalPayload TypedDict instead of dict. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
…client's manifest errors A call that started after its rollout was retired was admitted as a new text root: the retired rollout had no ledger rows, so admission saw a fresh rollout. The worker then staged the call's tokens and the ledger discarded its row, leaving staged tokens that no manifest names. The same happened to the first call of a duplicate run, which resolves as a root. has_rows now raises RolloutRetiredError for a retired rollout, and admission skips capture for it. RolloutControlClient.manifest now validates the rollout ID before building the URL (dot segments are normalized, so "../rollouts/r1" fetched r1's manifest), raises RolloutRetiredError for HTTP 410 instead of a plain RuntimeError, and rejects a manifest for a different rollout. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
Retire and delete removed cached ledger rows with a direct dictionary pop. A cache that also tracks its size, as the bounded ledger cache in #4268 does, would then keep counting the removed rows and gradually evict everything else. Removals now go through _ledger_cache_pop, the helper that cache defines, and a test checks that removing ledgers leaves no tracked size behind. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
A delete that found nothing to remove skipped the directory sync. When it overlapped another delete of the same rollout that had removed the files but not yet synced, it returned success while the removal was not durable, so a crash could bring back the old fence or ledger after the caller had started reusing the rollout ID. Every non-empty delete batch now syncs; a retire with nothing left to remove still syncs nothing, because its fences are already durable. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
…edger A retire that found every fence already in place and no ledger to delete returned without syncing. If an overlapping retire had created those fences without syncing yet, a crash could lose them after this call had reported success, and a late write could start a new ledger. Every non-empty retire batch now syncs its fences before removing anything and before returning; a retried batch still writes nothing. A retire removes ledgers after releasing and retaking each rollout's lock. In between, a delete could clear the fence and a reused rollout ID record a new ledger, which the retire then deleted. Removal now re-checks the fence under the lock and leaves an unfenced rollout's ledger alone. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
The complete-record token store could only retire a consumed snapshot through the conditional TokenSource.drop. Masked, failed, and abandoned captures had no removal path, and fences were only removable by sweep_retired, an age-based sweep that nothing called. Add two batched operations, the same ones the external-staging capture ledger gets. TokenSource.retire removes records and keeps the fence whatever the rollout's state (drop without the snapshot check): for masked or failed captures once their evidence is no longer needed, and for abandoned attempts. The fence makes later writes fail, including from a duplicate execution of the same rollout. TokenSource.delete removes records and fences: for purging once nothing can still write, and for resetting a rollout ID before reuse. TokenCaptureStore implements both, with retire_now and delete_now as synchronous forms, and drop shares the retire step. delete is now batched (it was single-ID and synchronous). The conformance suite checks both operations. Writes to a retired rollout fail with TokenCaptureRetiredError, a TokenCaptureFrozenError, instead of a plain RuntimeError. The sink already drops a late write to a frozen rollout quietly; before this, a late call of an abandoned attempt was logged and counted as a capture failure and the sink tried to mark the retired rollout incomplete. retire fsyncs the directory once per batch, and drop once instead of twice: the directory fsync after the state replace already covers the unlinks before it. The age-based sweeps are removed: sweep_retired, and sweep_stale, which reclaimed unretired captures once every file of them was older than a cutoff. Nothing in Gym called either. A capture's end of life is the training framework's explicit retire, for every attempt it finishes or abandons, including one whose request was cancelled mid-call, and delete once nothing can still write. A time cutoff can remove a capture that is still in use by a long session, and leaves the rest until the cutoff. rollout_collection still keeps masked and failed captures; callers can now remove them. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
TokenCaptureStore.delete_now synced the capture directory for every batch, even when none of the rollouts had files left, so a retried or empty delete still paid a directory fsync. It now syncs only when it removed a file, matching the capture ledger's delete. TokenSource.retire and delete, and the store's implementations, declare their result as the RolloutRemovalPayload TypedDict the capture ledger uses, instead of dict. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
…ches A retire that failed after removing a rollout's records but before writing its fence left the state file pointing past records that no longer existed, so every later freeze or write failed with a corrupt index error. The fence is now written first; a retire that then fails leaves the rollout fenced, and a retry finishes removing its records. retire_now and delete_now sync the directory once per batch, but an error part way through skipped that sync, and a retry found nothing left to change and returned without syncing. Both now sync what they changed before the error propagates. A late call of an abandoned attempt that produced no token ids still tried to mark the retired rollout incomplete and logged the refusal with a traceback. The sink now treats that refusal like a late write. The conformance check for retirement now requires late writes, mark_incomplete, and begin_call to fail with TokenCaptureFrozenError, the error the sink depends on, instead of accepting any exception. The in-memory reference backend in the tests is updated to match. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
…uiet Two retires of one rollout could overlap so that the second returned before the fence was durable: the first had written the fence and removed the records, the second then found nothing to change and returned without syncing, and a crash before the first's batch sync undid the fence. retire_now and drop now work in two phases, each ending in one directory sync: write the fences, then remove the records. The first sync also runs when only records are left to remove, because their fences may not be synced yet. A sync after a failed batch is logged instead of raised, so it never hides the error that failed the batch. A late call of a frozen or retired rollout that failed to build, produced no token ids, or arrived under a mismatched identity still counted a capture failure or logged a warning before finding out the rollout was gone. The sink now tries the mark first and stays quiet when the call is late. The conformance retirement check requires TokenCaptureRetiredError, as TokenSource.retire documents, instead of any TokenCaptureFrozenError. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
…ovals delete_now skipped its directory sync when it found nothing to remove. Overlapping another delete of the same rollout that had removed the files but not yet synced, it returned success while the removal was not durable, so a crash could bring back the old fence or records after the caller had started reusing the rollout ID. Every non-empty delete batch now syncs. _remove_record_files reported what it removed only when it returned, so when a later unlink failed, retire_now and delete_now did not know that earlier files were already gone and skipped the error-path sync. Both now treat a removal step as changing the directory before it starts. The conformance retirement check passed one string to ConformanceError, which takes a check name and a detail, so it raised TypeError and was reported as an unexpected error instead of naming the late write. It now passes both, and tests check the reported message. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
…cords A retire that found its rollouts already retired, with nothing left to remove, returned without syncing. If an overlapping retire had written those fences without syncing yet, a crash could lose them after this call had reported success, and a late write could land unfenced. Every non-empty retire batch now syncs its fences; a retried batch still writes nothing. retire_now removes records in a second phase, after releasing and retaking each rollout's lock. In between, delete_now could clear the fence and a reused rollout ID write new records, which the retire then removed, leaving the new attempt's state pointing past missing records. The second phase now re-checks the fence under the lock and leaves an unfenced rollout's records alone. drop holds the lock across both phases, so it was not affected. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
ananthsub
force-pushed
the
ananthsub/token-capture-store-retire
branch
from
October 10, 2026 02:11
61e6945 to
7615cb2
Compare
macandro96
approved these changes
Oct 10, 2026
Contributor
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/38026783272 |
This was referenced Oct 10, 2026
3 of 4 tasks
This branch was previously deployed
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.
Branch
ananthsub/token-capture-store-retire, on #3938 in the partial-rollout checkpoint stack, so this diff shows only the complete-record store change. It doesn't need #3938: the same commit applied tomainalone is branchananthsub/token-capture-store-retire-independent, which also carries identical copies of the two helpers #3938 adds (validate_rollout_idsandRolloutRemoval), and passes the unit suite on its own. Related and independent: #3937 (striped locks; a small conflict around the removed sweeps) and #4268 (bounded ledger cache).What changed and why
The complete-record token store (
TokenCaptureStore, behindTokenSource) could retire a rollout only through the conditionalTokenSource.drop, which needs the consumed snapshot. That leaves three gaps:rollout_collectionkeeps them for diagnosis, and a trainer had no operation to remove them later.dropcan't retire them at all.retiredstatedropleaves behind, which the store's docs called a tombstone) were only removable bysweep_retired, an age-based sweep that nothing called, which is a timer in disguise.sweep_stale(feat(token-capture): sweep abandoned captures safely #3969), the same kind of age-based sweep. It guesses that a capture is abandoned once all its files are older than a cutoff, which the caller must keep above the longest possible session. Nothing in Gym called it either.This PR gives the store the same two batched operations #3938 gives the capture ledger, with the same names and meaning:
CaptureLedger.delete(rollout_ids)TokenSource.delete(rollout_ids)CaptureLedger.retire(rollout_ids)TokenSource.retire(rollout_ids):dropwithout the snapshot check; keeps theretiredfenceTokenCaptureStore.retireanddeleteare async like the rest ofTokenSource, withretire_nowanddelete_nowas synchronous forms, followingfreezeandfreeze_now.dropandretireshare one retire step, so both leave the same fence, which refuses later writes,begin_call,mark_incomplete, andfreeze.TokenCaptureRetiredError, a new subclass ofTokenCaptureFrozenError, instead of a plainRuntimeError. The sink already drops a late write to a frozen rollout quietly. Before this, a late call of an abandoned attempt, which is exactly whatretireis for, was logged with a traceback, counted as a capture failure, and the sink then tried to mark the retired rollout incomplete. Now it is dropped like a late write after a freeze. A late call that failed to build, produced no token ids, or arrived under a mismatched identity is dropped quietly the same way: the sink tries to mark the rollout first, and when the rollout turns out to be frozen or retired it skips its warning and the capture-failure counter.retiresyncs the directory once per batch instead of once per rollout, anddropsyncs it once instead of twice: the directory fsync after the state file is replaced already covers the deletions before it. Retiring takes one file fsync per rollout instead of three.deletesyncs every non-empty batch, even when it finds nothing to remove, for the same reason as the capture ledger'sdelete: an overlapping delete may not have synced yet, and a reused rollout ID must not see the old fence or records come back after a crash.retire_nowanddropfirst write and sync the fences, then remove the records and sync again. A rollout is therefore never left without records and without a fence, and an overlapping retire of the same rollout cannot return before its fence is durable: the first sync also runs when only records are left to remove. A retry removes records left behind by a retire that failed part way or by a crash. A batch that fails part way still syncs what it changed; a failing sync there is logged, not raised, so it never hides the original error. Every non-empty batch syncs its fences before returning, even when it wrote none, so an overlapping retire can never report success before the fence is durable. The second phase re-checks each fence under the rollout's lock, so a rollout ID that a delete cleared and a new attempt reused between the phases keeps its new records.retiredstate a fence, the capture ledger's word for it, instead of a tombstone.deleteis now batched; it was single-ID and synchronous. Its one caller,clear_token_captures_for_rollouts, now makes one batched call.sweep_retiredandsweep_staleare removed. A capture's end of life is an explicit call from its owner, not its age:retirewhen the training framework is done with a rollout, including an abandoned attempt, anddeleteonce nothing can still write.removedandabsentlike the ledger's, typed as the sameRolloutRemovalPayloadTypedDict.TokenSourcehas to implement them. Its retirement check requires a lateput,mark_incomplete, andbegin_callto fail withTokenCaptureRetiredError, asTokenSource.retiredocuments, rather than accepting any exception, and lets any otherfreezeerror fail the check.rollout_collectionbehavior is unchanged: it still keeps masked and failed captures. Callers can now remove them.How it works
dropstays the way to retire a consumed snapshot, because it refuses when the rollout changed after the snapshot.retireis for everything else the consumer is done with;deleteremoves fences once nothing can still write, and resets an ID before reuse.What the caller must do
retire_rollout_token_capture(unchanged: conditionaldrop)retire([ids])retire([ids]); its agent may still be running, and the fence makes its later writes faildelete([ids])to remove the fencesdelete([ids])first (rollout_collectionalready does this before re-dispatch)Always pass a list; both operations are idempotent, so a failed batch can be retried as is.
Compatibility
TokenSourcegainsretireanddelete. Gym never checks aTokenSourceagainst the protocol at runtime, but the conformance suite now requires both, so an external source must add them to keep passing.TokenCaptureStore.delete(rollout_id)was synchronous and single-ID; it is nowasync delete(rollout_ids), withdelete_now(rollout_ids)as the synchronous form. A caller still writingstore.delete("id")gets an un-awaited coroutine and deletes nothing, so update such calls todelete_now(["id"]). Gym and NeMo RL have no such callers outside the one updated here.begin_call,mark_incomplete, andfreezeon a retired rollout raiseTokenCaptureRetiredError. It subclassesTokenCaptureFrozenErrorand soRuntimeError, so code that caught either still catches it.TokenCaptureStore.sweep_retiredandTokenCaptureStore.sweep_staleare removed. Nothing in Gym or NeMo RL called either. A caller that swept abandoned captures by age now retires the rollouts it abandoned.Issue
No tracking issue. This aligns the complete-record store's retirement with the capture ledger's and closes its gaps for masked, failed, and abandoned captures and for fences.
Validation
tests/unit_tests/test_token_id_capture.pycover retiring live, frozen, and never-captured rollouts (records removed; later writes,begin_call,mark_incomplete, andfreezeall raiseTokenCaptureRetiredError; a repeat is a no-op), a late call after retire dropped by the sink without a capture failure or an incomplete marker, one directory fsync per retire batch and perdrop, delete removing the fence so the ID can be reused, batch validation with bare-string rejection, every non-empty delete batch syncing, files removed before a later removal failed still being synced, a batch failing part way still syncing what it changed, a retire failing part way leaving the rollout fenced and a retry finishing it, an overlapping retire returning only after the fence is durable, a sync failure after an error not hiding that error, two directory syncs per retire batch and perdrop, a retire that cannot write its fence changing nothing, and late calls without token ids or with a failed build not reported as failures. Existing tests that usedsweep_retiredor single-IDdeletenow usedelete_now, and thesweep_staletests are removed with it.unconditional_retirementanddelete_clears_retirementpass for the file store and the in-memory reference backend, and reject a backend that lets a latemark_incompletethrough after retire, rejects late writes with a plain or merely frozen error, or whosefreezefails after retire.retire_now, the fence removal indelete_now, the retired error type, or the per-batch fsync is reverted.capture_failed{write}and 50marked_incompletebefore the error-type change, and as 50late_write_droppedafter it. Retiring a rollout takes about 330 µs on a local disk, down from about 420 µs, and its system calls drop from three fsyncs to one.mainwith this PR, fix(token-capture): use striped lock files that are never deleted #3937, feat(token-capture): retire and delete external-staging capture ledgers #3938, feat(token-capture): low-overhead metrics for capture operations, outcomes, digests, and caches #4218, and fix(token-capture): bound the capture-ledger cache #4268):pytest tests/unit_testspassed.pyteston every token-capture, staging, trajectory-builder, rollout-collection, and model-server unit test file: passed.pytest responses_api_models/vllm_model/tests/test_token_capture_integration.py responses_api_models/vllm_model/tests/test_external_capture_streaming.py: passed.pre-commit run --files <changed files>: all hooks passed.Rollout evidence
N/A: the delivery path still retires through the same
drop, and the new operations run only when a caller invokes them, so rollouts behave as before.