[async] async data buffer: unified filters and better observability - #2030
Conversation
… rollout API - worker becomes a long-lived task on the shared rollout event loop (lazy-started on the first train call): no thread, no private loop, no module globals, no atexit; asyncio.Queue with the same backpressure - switch to inference_rollout primitives (instance GenerateState); the cross-loop hazard of the legacy singleton disappears - errors are loud: a failed generation task kills the worker and the next drain raises instead of hanging; recycle paths no longer swallow - report queue depth / staleness / recycle counts via RolloutFnTrainOutput.metrics; assert group size matches n_samples_per_prompt - RolloutManager reuses the rollout fn instance when eval_function_path equals rollout_function_path - requires MILES_EXPERIMENTAL_ROLLOUT_REFACTOR=1; scripts and docs updated to the FullyAsyncRolloutFn path; eval raises with guidance
Reuses the 30B harness: fully_async selects train_async.py plus FullyAsyncRolloutFn, on the disaggregated topology it requires (train_async rejects colocation), with the standard CI metric gates. Three rollouts instead of two, to cover the states that only exist with a persistent worker: cold start, drain from a warm queue, and a drain across a weight update that pauses generation and recycles aborted groups.
Rollout selection belongs in the argument surface, not in an environment variable that rewrites sys.argv before parsing. --rollout-function-path now defaults to None, so "the user chose one" is a plain is-None check instead of a comparison against a computed default, and resolve_rollout_function_path() is the single place that maps arguments to a rollout function. miles_validate_args rejects the configurations that cannot work: no class-based rollout API, a competing --rollout-function-path, or --colocate, which the async driver cannot honor. Evaluation keeps the standard rollout function, since fully async does not serve eval. train.py asserts the flag is off, so picking the wrong driver fails loudly. Co-authored-by: yueming-yuan <yym022502@gmail.com>
--rollout-function-path now defaults to None, so multi-LoRA's "the user did not pick one" test can be a plain is-None check; comparing against the standard paths silently stopped matching and left multi-LoRA runs on the default rollout function. Also reject --fully-async together with multi-LoRA, which resolves its own rollout function before the fully-async block runs.
The fully-async finished-group buffer was an asyncio.Queue with an effectively unbounded cap (1000 groups): when production outran training, groups piled up and FIFO consumption always trained on the oldest, with no bound on how stale the consumed data could get and no visibility into it. Replace the queue with GroupBuffer: - --async-buffer-max-groups caps the buffer in prompt groups. On overflow the producer no longer blocks; the buffer evicts and recycles prompts back to the data source: first every group beyond --max-weight-staleness (when set and the engine version is known), then the group whose stalest sample is oldest, ties broken by the larger summed staleness, then randomly. Default None keeps the legacy blocking behavior. - --async-buffer-order picks fifo (default) or lifo consumption; lifo trains closest to on-policy and pairs with the cap so sunk groups are evicted rather than eventually trained on. - Metrics: evicted_stale_groups / evicted_overflow_groups / evict_rate (evictions over groups entering the buffer since the last drain) and buffer_avg/max_staleness over the queued groups at drain time. The consumed avg/max_staleness metrics now report unconditionally instead of only when --max-weight-staleness is set. Eviction ranks by (min weight version, summed weight versions), which equals ranking by (largest sample staleness, largest summed staleness) without needing the current engine version: with equal group sizes it is a constant offset that cancels.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
7cf1112 to
3068087
Compare
main landed the fully-async migration and its eval support, so the branch's copies of those commits conflicted with the merged versions. Resolution takes main for everything the migration already carries (docs, e2e tests, multi_lora, rollout/eval function resolution) and re-applies the staleness delta on top: - GroupBuffer now buffers (prompt group, finished group) entries, matching main's queue item; eviction recycles the prompt group via _recycle. - Drain reports staleness unconditionally and adds the buffer/eviction metrics alongside main's dynamic-filter metrics. - Tests: main's suite plus the GroupBuffer cases, with the worker-failure test seeding a GroupBuffer instead of a raw asyncio.Queue.
…pBuffer -> DataBuffer
# Conflicts: # tests/fast/rollout/test_fully_async_rollout.py
…r drops from the unused handler
|
|
||
| Dataflow control options: | ||
|
|
||
| (1) max groups: use ``--async-data-buffer-capacity-factor`` to set the max |
There was a problem hiding this comment.
Why the blocking logic removed here?
In the current impl. When the buffer is full, we just evicted stale groups
This design also force data buffer hold weight_version info, which introduce some semantic confusion for weight_version in DataBufferInput class. (it's actually observed engine weight version rather than the group's min weight version.).
What we achieve is just save some cpu memory when kick out staled groups.
|
|
||
| (1) max groups: use ``--async-data-buffer-capacity-factor`` to set the max | ||
| size of the buffer, floor(factor * rollout_batch_size) groups. On | ||
| overflow the most stale groups are evicted. |
There was a problem hiding this comment.
I suggest the behavior change:
When the data buffer is full, block the data_source -- prompt --> generation_pool path to stop too many rollouts.
|
also a tiny fix on this #2241 |
guapisolo
left a comment
There was a problem hiding this comment.
LGTM except for the weight version handling.
…arn when it regresses
…ression under --ci-test
…t on regression under --ci-test" This reverts commit 48c4d78.
…ner' into pr-2030-merge # Conflicts: # miles/rollout/fully_async_rollout.py
# Conflicts: # miles/rollout/fully_async_rollout.py # tests/fast/rollout/test_fully_async_rollout.py
When rollout production outruns training consumption in fully-async mode, finished groups pile up in what is effectively an unbounded queue (
asyncio.Queue(maxsize=1000)= 125 training steps of backlog), and FIFO consumption means training always eats the oldest, stalest data — with no bound and no visibility.Stacked on #1717 (
yueming/fully-async-class-api), wherefully_async_rollout.pylives.Buffer capacity with staleness-ranked eviction
--async-data-buffer-max-batchescaps theDataBufferin multiples ofrollout_batch_size(default2). On overflow the producer does not block; the buffer evicts stalest-first — recycling the evicted prompts into the data source for regeneration (same path as the existing abort/staleness recycling) — until nothing in the buffer is beyond--max-weight-staleness(when set and the engine version is known) and the buffer is back within capacity. Ranking:The ranking needs no engine-version query: ranking by (min weight version, summed weight versions) is identical to ranking by (largest sample staleness, largest summed staleness) — with equal group sizes the current version is a constant offset that cancels. The version is only consulted for the
--max-weight-stalenessthreshold.0disables the cap and restores the legacy behavior: a large bound that blocks the producer when full.Evicting at insertion also relieves the drain-time
--max-weight-stalenessfilter, which otherwise discovers stale groups only at consumption, one drain at a time, while training waits on the recycle-regenerate churn.LIFO option
--async-data-buffer-order lifoconsumes the freshest group first, keeping updates closest to on-policy without throttling production. Old groups then sink and only leave via eviction or a late (stale) drain, so lifo is meant to pair with the cap and/or--max-weight-staleness; the help text says so. Default staysfifo.Metrics
rollout/fully_async/evicted_stale_groups,evicted_overflow_groups,evict_rate(evictions / groups entering the buffer since the previous drain)rollout/fully_async/buffer_avg_staleness,buffer_max_stalenessover the groups sitting in the buffer at drain timeavg_staleness/max_stalenessnow report unconditionally, not only when--max-weight-stalenessis set (previously the feature had to be on before you could see whether you needed it)Sizing note
Measured on the current GLM-5.2 16-node run: steady-state queue is 0–5 groups and the post-eval burst is bounded by the in-flight cap (16 groups at
--async-max-concurrent-samples 128), so ~2 training batches of capacity absorbs every observed burst while bounding queue-induced staleness to ~2 steps — hence the default of 2.Tests
DataBufferunit tests (overflow eviction, threshold-first eviction, summed-staleness tie-break, lifo order, staleness stats) plus a drain-level metrics-wiring test; existing tests updated for the two new args. 19 passed, 3 consecutive runs.🤖 Generated with Claude Code