Skip to content

fix: use group-level concurrency in fully-async rollout - #2367

Open
JayPritchet wants to merge 1 commit into
THUDM:mainfrom
JayPritchet:fix/fully-async-group-concurrency
Open

fix: use group-level concurrency in fully-async rollout#2367
JayPritchet wants to merge 1 commit into
THUDM:mainfrom
JayPritchet:fix/fully-async-group-concurrency

Conversation

@JayPritchet

Copy link
Copy Markdown

Problem

The fully-async worker receives a concurrency value derived from sglang_server_concurrency, which represents per-sample concurrency:

_global_worker = AsyncRolloutWorker(
args, data_buffer, concurrency=args.sglang_server_concurrency * get_rollout_num_engines(args)
)

However, the worker applies this value to the number of active generate_and_rm_group tasks and the number of completed groups in output_queue which are measured in groups rather than samples.

# Top up. The qsize gate is the queue's backpressure: once a
# full pool of completed groups is waiting, stop pulling new
# prompts until the training side drains some.
while (
len(active_tasks) < max_concurrent and self.output_queue.qsize() < max_concurrent and self.running
):
groups = self.data_buffer.get_samples(1)

When n_samples_per_prompt > 1, this makes the worker's scheduling and backpressure window larger than intended.

For example, with a sample concurrency of 512 and 8 samples per prompt, the worker previously allowed up to 512 active groups instead of roughly 64 groups.

The inner semaphore still limits actual SGLang sample concurrency, so this does not exceed the server request concurrency limit. However, it can cause excessive task creation, prefetching, and completed-group buffering before backpressure takes effect.

Change

Derive the worker's group concurrency from the total sample concurrency:

group_concurrency = ceil(sample_concurrency / n_samples_per_prompt)

Rename the worker fields and variables to make their group-level semantics explicit.

This change only affects the outer fully-async scheduling and backpressure window. The existing per-sample SGLang semaphore remains unchanged.

Testing

  • Added a unit test covering sample-to-group concurrency conversion.
  • Updated existing fully-async backpressure tests to use group-level terminology.
  • tests/test_fully_async_rollout.py: 5 passed.

Derive the worker concurrency from the total sample concurrency and
n_samples_per_prompt. This keeps the worker's active-task and queue
backpressure limits in group units.

Update the fully-async documentation and unit tests accordingly.
@JayPritchet
JayPritchet force-pushed the fix/fully-async-group-concurrency branch from 4b03119 to c950027 Compare September 7, 2026 06:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant