[Fix] Preserve concurrent trace sessions in disaggregated RL - #2021
[Fix] Preserve concurrent trace sessions in disaggregated RL#2021matrix72c wants to merge 2 commits into
Conversation
|
@claude review |
|
Claude: Summary本 PR 将 disaggregated RL 的 trace session 清理从 ProduceBatchResult impact: 字段语义未改变;但 Main Flowchart after this PRflowchart TD
A[disagg _fit: get_batch] --> B[_train_one_batch]
B --> C{release_only_consumed_trace_sessions}
C -->|True| D[_release_trace_store train_batch<br/>release_sessions 已消费 session]
D --> E{need_sync}
E -->|Yes| F[pause_produce<br/>in-flight rollout 转为 ABORTED 入 replay buffer]
F --> G[_sync_weights_and_save]
G --> H{enable_evaluate}
H -->|Yes| I[_run_evaluation<br/>finally: _release_trace_store 无参数 → release_all]
I --> J[replay buffer 中 ABORTED/leftover session 被误释放<br/>routed_experts refs 失效]
H -->|No| K[continue_produce]
J --> K
style D fill:#cce5ff,stroke:#0366d6
style I fill:#f9c0c0,stroke:#d73a49
style J fill:#f9c0c0,stroke:#d73a49
核心原理实现与单测核心实现是三处所有权划分: 单测方面,三个新增/修改测试全部替换了项目内部 seam,导致本 PR 的核心新代码没有被真实执行: 抽象与信息隐藏评估
单测建议
其他 Issues
VerdictREQUEST_CHANGES |
ab0035c to
78d5594
Compare
Summary
This PR fixes the ownership and cleanup of agentic rollout trace sessions in disaggregated RL training.
Root Cause
RLDisaggregatedTrainerintentionally keeps its background producer running while the learner trains the current batch. During that overlap, theRolloutTraceStorecontains sessions with different owners:The inherited post-batch cleanup called
RolloutTraceStore.release_all(). That operation assumed a batch-synchronous lifecycle in which every live session belonged to the completed learner batch. The assumption is not valid for disaggregated training, so cleanup could delete producer-owned sessions and force-free their Ray-backedrouted_expertswhile future rollout states still referenced them.Ray preserves ordering for calls from one caller, but it does not provide a global ordering across the learner and producer callers. Consequently, the same race could surface in more than one form:
ObjectReffailures when the data is consumed or checkpointed.A short one-step asynchronous smoke test can miss the problem when cleanup happens before the next producer insertion, or when the prefetched batch is immediately aborted at shutdown and never consumed. The failure becomes reproducible when producer progress overlaps the learner's post-batch cleanup.
This is therefore an XTuner lifecycle bug in the combination of disaggregated training, agentic trace storage, and concurrent prefetch; it is not caused by a sandbox configuration.
Fix
Selective trainer cleanup
Add an atomic, idempotent
RolloutTraceStore.release_sessions(session_ids)actor method and use it after a disaggregated learner batch. The trainer extracts the sessions represented by the consumed batch and releases only those sessions. Colocated and evaluation cleanup retain their existing full-store behavior.The actor method ignores already-absent IDs and returns the IDs it actually released. Keeping the lookup and release in one actor RPC avoids a client-side list/intersection/release time-of-check/time-of-use window.
Terminal discard cleanup
Selective post-batch cleanup means a rollout that will never reach
train_batch()needs an explicit terminal cleanup path. This PR therefore releases trace sessions before discarding:Retryable expired groups retain their sessions because they may re-enter the training lifecycle. When trace cleanup has already freed the routed-expert objects, the corresponding local field is cleared before generic rollout-state disposal to avoid a second explicit free.
Impact
ProduceBatchResult: no status, reward, timing, rollout-state, or accounting semantics are changed.RoutedExperts: references for consumed or terminally discarded sessions are freed exactly once; references owned by concurrent or retryable rollouts remain valid until their owning lifecycle ends.Reproduction
The issue was observed in a three-node disaggregated run with 8 learner GPUs and 16 rollout GPUs:
The trainer regression test models this deterministically with one consumed session and one concurrently produced session, then verifies that only the consumed session is released.
Test Plan
RLDisaggregatedTrainer;5 passed.git diff --check: passed.The full repository matrix is left to upstream CI.
Out of Scope
This PR does not change replay-checkpoint serialization. The stale replay
ObjectRefsymptom can be caused by the same premature trace release, but serialization policy and timeout diagnostics are separate concerns.