[nataliekung]

What does this PR do?

Fixes #7398. transfer_queue.backend.SimpleStorage.num_data_storage_units defaulted to a fixed 8. Each SimpleStorageUnit and the TransferQueueController reserve 1 CPU, and TaskRunnerV1.run always sets transfer_queue.enable = True, so every default PPO run reserves 9 CPUs before the trainer starts. On a node where Ray has fewer than 9 CPUs, one unit stays PENDING_CREATION and tq.init blocks with no error.

This PR defaults the count to null and resolves it to 2 * trainer.nnodes right before tq.init, which matches the recommendation in the config comment. An explicit value is kept as is.

Checklist Before Starting

• Search for similar PRs: https://github.com/verl-project/verl/pulls?q=is%3Apr+num_data_storage_units
  • [BREAKING][trainer, cfg] fix: derive TransferQueue storage units from node count #7525 proposed the same approach and was closed by its author without review. This PR picks it up on current main, with a repro in [transfer_queue] num_data_storage_units default (8) is not cluster-size aware and can exhaust CPU on small single-node runs #7398.
  • [fix] Fail fast when SimpleStorage lacks Ray CPU capacity Ascend/TransferQueue#165 and fix: typo #166 (fail fast and bounded startup wait in TransferQueue) were closed without merge, so TransferQueue at 5fe4e49 still blocks.
• Format the PR title as [{modules}] {type}: {description}

Test

• New tests/trainer/test_tq_storage_units_on_cpu.py composes ppo_trainer with Hydra and checks: nnodes=1 gives 2, nnodes=4 gives 8, an explicit num_data_storage_units=3 stays 3. Local run: 3 passed.
• scripts/generate_trainer_config.sh regenerated the four _generated_*.yaml files and reports clean.
• Behavior before this change, from the repro in [transfer_queue] num_data_storage_units default (8) is not cluster-size aware and can exhaust CPU on small single-node runs #7398 (one local Ray node, TransferQueue 5fe4e49, Ray 2.59.0): 8 CPUs with 8 units leaves one unit PENDING_CREATION and tq.init blocked for 120 s; 8 CPUs with 2 units returns with 5 CPUs left.

API and Usage Example

No new option. Users who relied on 8 units on a 1 to 3 node run can set it explicitly:

transfer_queue.backend.SimpleStorage.num_data_storage_units=8

Design & Code Changes

• verl/trainer/config/transfer_queue/transfer_queue.yaml: default null, comment states the 1 CPU per unit cost.
• verl/trainer/main_ppo.py: set_default_tq_storage_units(config) fills the count from trainer.nnodes when it is null, called before the config is printed and passed to tq.init. This is the only place in verl that creates TransferQueue from config; the other tq.init() calls attach to the existing controller.
• Regenerated _generated_ppo_*.yaml.

Checklist Before Submitting

• Read the Contribute Guide.
• Ruff check and format on changed files.
• Add unit test(s).

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 3c878807-181f-4512-aab2-211795061982

📥 Commits

Reviewing files that changed from the base of the PR and between dc709af and 881fb96.

📒 Files selected for processing: 7

• tests/trainer/test_tq_storage_units_on_cpu.py
• verl/trainer/config/_generated_ppo_megatron_trainer.yaml
• verl/trainer/config/_generated_ppo_torchtitan_trainer.yaml
• verl/trainer/config/_generated_ppo_trainer.yaml
• verl/trainer/config/_generated_ppo_veomni_trainer.yaml
• verl/trainer/config/transfer_queue/transfer_queue.yaml
• verl/trainer/main_ppo.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Summary

Summary by CodeRabbit

• New Features
  • Transfer queue storage now defaults to two storage units per trainer node, scaling automatically with the configured node count.
  • An explicitly configured storage-unit count is preserved instead of being replaced by the automatic default. Each storage unit reserves one CPU.

The SimpleStorage storage-unit default is now unset in the trainer configurations. Trainer setup assigns twice the node count when the value is unset and preserves explicitly configured values.

Changes: Transfer queue storage units

Layer / File(s)	Summary
Nullable setting and node-scaled default verl/trainer/config/transfer_queue/transfer_queue.yaml, verl/trainer/config/_generated_ppo_*_trainer.yaml, verl/trainer/main_ppo.py	The configuration defaults num_data_storage_units to null. set_default_tq_storage_units sets it to twice trainer.nnodes when unset and leaves explicit values unchanged.
Trainer application and regression tests verl/trainer/main_ppo.py, tests/trainer/test_tq_storage_units_on_cpu.py	TaskRunnerV1.run applies the default before displaying and resolving the configuration. Tests cover defaults of 2 units for 1 node and 8 for 4 nodes, and preservation of an explicit value of 3.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 881fb

No actionable merge-blocking risk was established for the changed transfer-queue sizing behavior.

Security architecture risk: 🔵 Low · up to 881fb

The change is bounded to startup storage sizing and preserves explicit settings. No expanded privileges or new externally reachable interface were identified. Resource feasibility and partial-startup behavior remain dependent on TransferQueue and the deployed cluster.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The demonstrated effect is on storage-unit allocation in the Ray cluster used by the trainer. Broader tenant or environment exposure cannot be established without deployment topology and the external TransferQueue implementation.

Trust Boundaries and Controls

• inferred — The new policy uses trainer configuration already supplied by the caller. Explicit storage counts remain configurable, so the change does not establish a new ability to control allocation or cross an identity boundary.

Resilience and Maintainability Implications

• observed — Normalization completes before TransferQueue initialization and is idempotent for an unchanged configuration. The existing initialization call remains outside the training cleanup block, while tq.close remains in its finally path. Partial-initialization cleanup is therefore an inherited uncertainty, not an established new failure-containment defect.

Pre-merge checks: 3 · 2

Failed checks: 2 warnings

Check name	Status	Explanation	Resolution
Linked Issues check		Issue [#7398] requires a cluster-aware default with the proposed formula max(2, 2 * trainer.nnodes) and an explicit override. The PR preserves explicit values and resolves normal 1-node and 4-node d…	Set the null-value default to max(2, 2 * config.trainer.nnodes) and add an automated test for the minimum case.
Docstring Coverage		Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (5 skipped: 5…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 3 passed

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: deriving TransferQueue storage units from trainer.nnodes.
Description check		The description is directly related to the changeset and explains the new default, override behavior, implementation, tests, and motivation.
Out of Scope Changes check		The changed configuration defaults, generated configuration files, resolver function, and CPU-side tests all support issue [#7398]. No unrelated product behavior or unrelated files are changed.

Full details: Linked Issues check

Explanation

Issue [#7398] requires a cluster-aware default with the proposed formula max(2, 2 * trainer.nnodes) and an explicit override. The PR preserves explicit values and resolves normal 1-node and 4-node defaults correctly, but set_default_tq_storage_units assigns only 2 * config.trainer.nnodes. The implementation and tests do not establish the required minimum of two units for a node count that yields fewer than two units.

Resolution

Set the null-value default to max(2, 2 * config.trainer.nnodes) and add an automated test for the minimum case.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (5 skipped: 5 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[nataliekung]

@wuxibin89 could you approve the CI workflow runs for this PR and the ones below when you have a moment? They come from an external contributor, so each shows 35-36 workflows waiting for approval, and none of them has run CI yet. All are mergeable against current main.

• [tool, rollout] fix: create BaseTool instances once per trajectory and release them when it ends #8156 [tool, rollout] create BaseTool instances once per trajectory
• [reward, cfg] fix: remove invalid super().__post_init__() in RewardManagerConfig #7399 [reward, cfg] remove invalid super().post_init() in RewardManagerConfig
• [rollout, vllm, sglang] fix: fail fast when a rollout server actor is unschedulable #7382 [rollout] fail fast when a rollout server actor is unschedulable
• [vllm] fix: surface bucketed receiver failures #7366 [vllm] surface bucketed receiver failures
• [rollout] fix: keep loop workers on the job's node group #7343 [rollout] keep loop workers on the job's node group
• [single_controller, trainer] fix: finalize nsys capture range in continuous profiling #7330 [single_controller, trainer] finalize nsys capture range in continuous profiling

Thanks.

[0oshowero0]

Well, on a node with fewer than 9 CPUs is not even able to perform the training 😂