Limit forward_backward queue drain size#1874
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new configuration option forward_backward_max_request_count to limit the number of pending forward_backward requests coalesced into a single backend call, preventing potential CUDA OOM issues. It includes a synthetic reproduction script, engine changes to chunk the requests, and corresponding unit tests. The reviewer suggests adding Pydantic validation (gt=0) to the new configuration field to prevent non-positive values from being silently accepted.
| forward_backward_max_request_count: int | None = Field( | ||
| default=None, | ||
| description=( | ||
| "Optional cap on how many pending forward_backward requests the engine " | ||
| "coalesces into one backend call. Default `None` preserves the current " | ||
| "unlimited batching behavior." | ||
| ), | ||
| json_schema_extra={"argparse_type": lambda v: None if v == "None" else int(v)}, | ||
| ) |
There was a problem hiding this comment.
To prevent silent configuration errors where a user might accidentally configure a non-positive value (e.g., 0 or a negative integer) and experience unexpected CUDA OOMs due to the engine falling back to unlimited batching, we should enforce that forward_backward_max_request_count is strictly greater than zero using Pydantic's gt=0 validation.
forward_backward_max_request_count: int | None = Field(
default=None,
gt=0,
description=(
"Optional cap on how many pending forward_backward requests the engine "
"coalesces into one backend call. Default 'None' preserves the current "
"unlimited batching behavior."
),
json_schema_extra={"argparse_type": lambda v: None if v == "None" else int(v)},
)
Summary
Closes #1873.
A client can submit many pending
FORWARD_BACKWARDrequests with uneven sequence lengths. Today the engine can drain every batchable pending backward request before a destructive barrier into one backend call, so a long pending queue can become one oversized training step.This PR bounds that path in two places:
forward_backward_max_request_countsplits pending backward requests into ordered chunks before backend processing.min(actual_batch_size, configured_micro_batch_size)so a capped one-request chunk is not padded back up to the full configured microbatch size.The default remains unlimited unless operators configure the cap.
Public Repro
This PR includes a public real-service repro. The payloads are synthetic token IDs to avoid any private data, but the script sends actual
forward_backwardcalls through a SkyRL/Tinker-compatible service URL and waits for real service results.python examples/repro_forward_backward_queue_drain.py summarize python examples/repro_forward_backward_queue_drain.py run-forward-backward \ --base-url http://$SERVICE_HOST:8000 \ --future-timeout-s 7200 \ --skip-leading-blocker \ --max-pending-requests 17Shape summary:
The live OOM repro uses the first 17 pending requests from that shape: eight short requests, one 35,000-token request, then eight more short requests.
Endpoint shape used for before/after validation:
Before / Cap-Only / After
n=1completes, thenprocess_batch_requests(forward_backward, n=16)fails withtorch.OutOfMemoryError: CUDA out of memory. Tried to allocate 9.08 GiBwhile running backward through the LoRA MLP path.Padded batch from 1 to 16 (alignment=16), so the one 35,000-token request is expanded back to the configured microbatch size before backward.forward_backward request batch split 16, processes sixteenn=1backend calls, the 35,000-token request returns metrics, and there is no OOM.Code Changes
EngineConfig.forward_backward_max_request_count.FORWARD_BACKWARDrequests into ordered request-count chunks before callingprocess_batch_requests.effective_padding_micro_batch_size(...)and applies it before_pad_batch(...)for forward and forward-backward SkyRL-Train paths.examples/.Validation
Focused test results:
5 passedfor the queue repro tests and3 passedfor the padding tests.Real endpoint validation: