[ZCC] Fix offload barrier never waiting for the in-flight D2H - #4838
[ZCC] Fix offload barrier never waiting for the in-flight D2H#4838ForFishes wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
| 序号 | 位置 | 优先级 | 状态 |
|---|---|---|---|
| 1 | IPC 地址漂移 | ✅ | |
| 2 | ZCC barrier 回归测试 | 🚧 |
|
|
||
| def maybe_update_zcc_worker(self, args, model, optimizer, global_step): | ||
| inner_opt = _unwrap_opt_for_fused_states(optimizer) | ||
| self.check_ipc_buffer_drift(optimizer, global_step) |
There was a problem hiding this comment.
地址漂移后不能继续使用旧 IPC handle 保存。这里忽略了
check_ipc_buffer_drift 的返回值,下一行又会在 fused_buffer_version == cache_version 时直接返回;worker 的 ParamFusionStorageHelper 只有通过 update_zcc_workers 收到新的 ipc_meta_mappings 才会刷新。因此 clear_param_storage() / reset_param_storage() 一旦换了地址,本次 PREPARE/OFFLOAD 仍会从已释放内存读取,可能静默损坏 checkpoint;同时上面的 _comm_buffer_ptrs = current 还会让后续相同 stale 地址不再告警。请在发出保存任务前让所有 rank 一致刷新 worker IPC metadata,或 fail fast/跳过本次保存,不能只告警后继续。
There was a problem hiding this comment.
当前 head 3b4153d 已移除本 PR 新增的 drift 检测及 warning-only 继续保存路径,增量不再引入该行为;此项按本 PR 范围已解决。
| # configurations keep synchronizing at `on_optimizer_begin` as before. | ||
| return | ||
| logger.info("[ZCC manager] Start syncing checkpoints (step begin)") | ||
| self.sync_offload_status() |
There was a problem hiding this comment.
请把这次 barrier 复现加入仓库回归测试。当前 diff 没有测试,现有
tests/ai_edited_test/trainer/test_ai_zero_cost_ckpt.py 也只覆盖枚举、哈希和 optimizer unwrap;PR 描述中的临时 fake-worker 验证不会在 CI 中执行。这个修复依赖 manager.global_step 刷新和 current_pipeline_hook_step guard 的组合,建议至少覆盖:旧 step 必须持续轮询、单 chunk 在 step begin 同步、多 chunk 未发完时不等待、多 worker 复用不会接受 stale echo,以及 on_step_end 两个分支都写入当前 step。
`ZeroCostCheckpointManager.global_step` is only ever assigned inside `update_zcc_workers()`, whose sole caller `maybe_update_zcc_worker()` returns early once `inner_opt.fused_buffer_version == self.manager.cache_version`. The fused buffer version does not change in steady state, so the manager's step number is frozen at the step of the first save of a run. `sync_offload_status()` compares that frozen value against `current_worker.global_step.value`, which the worker merely echoes back from the OFFLOAD task. Manager sends X, worker writes X back, manager compares X == X: from the second save onwards the barrier is satisfied on the first comparison and never waits. Observed on a 2496-GPU run: over a whole training round `Waiting current worker offloading done` was never logged, while `Current worker offloading done` printed 48 times, every one of them with `worker_step == manager_step ==` the first save's step number. The barrier is the only ordering between the trainer and the ZCC worker: the worker is a separate process that reads the fused GPU buffers over CUDA IPC on its own stream, so there is no CUDA stream ordering with the trainer's compute stream. With it disabled, step N+1 mutates GPU memory that the step-N snapshot is still copying out, which can silently corrupt the saved weights and optimizer moments. Changes: 1. Assign `manager.global_step = state.global_step` in both branches of `ZeroCostCheckpointCallback.on_step_end`, right after `maybe_update_zcc_worker()`. This is what the `# set 'on-step-end'` comment on the declaration always intended. The OFFLOAD task then carries the real step number and the comparison becomes meaningful. 2. Move the synchronization earlier. `on_optimizer_begin` is too late: FP8 expert-weight quantization runs in `on_step_begin` and calls `clear_param_storage()`, which releases and re-allocates the very buffers the worker is reading. `Trainer._inner_training_loop` now calls the new idempotent `manager.maybe_sync_offload_status()` just before `on_step_begin`. It is guarded by `current_pipeline_hook_step != pipeline_hooks_steps` so that it only waits when every offload chunk has already been dispatched; configurations that still slice the offload across this step's pipeline hooks (PP > 1, or gradient_accumulation_steps > 1) keep synchronizing at `on_optimizer_begin` and cannot deadlock. The existing synchronization is left in place as a fallback.
ad91f32 to
3b4153d
Compare
PaddleFormers Log Analysis
日志分析报告
失败的测试case: 根本原因分析: PR #4838 改动了两处: Integration test 失败(直接因果)
unittest 失败(可能相关)
修复建议:
🔄 每次 Re-run 后自动更新 |
Cover the barrier bug fixed in PaddlePaddle#4838: - sync_offload_status keeps polling on a stale worker step and does not accept the worker's echo of a previous step - maybe_sync_offload_status waits at step begin only when the offload is fully dispatched, and declines (no deadlock) while chunks remain - on_step_end refreshes manager.global_step on every offloading step (both the save and EMA branches), and leaves it untouched off-interval Pure-mock tests, no GPU/worker process required.
Cover the barrier bug fixed in PaddlePaddle#4838: - sync_offload_status keeps polling on a stale worker step and does not accept the worker's echo of a previous step - maybe_sync_offload_status waits at step begin only when the offload is fully dispatched, and declines (no deadlock) while chunks remain - on_step_end refreshes manager.global_step on every offloading step (both the save and EMA branches), and leaves it untouched off-interval Pure-mock tests, no GPU/worker process required.
Cover the barrier bug fixed in PaddlePaddle#4838: - sync_offload_status keeps polling on a stale worker step and does not accept the worker's echo of a previous step - maybe_sync_offload_status waits at step begin only when the offload is fully dispatched, and declines (no deadlock) while chunks remain - on_step_end refreshes manager.global_step on every offloading step (both the save and EMA branches), and leaves it untouched off-interval Pure-mock tests, no GPU/worker process required.
Problem
The ZCC offload barrier never waits.
ZeroCostCheckpointManager.sync_offload_status()comparesself.current_worker.global_step.valueagainstself.global_step, but:manager.global_stepis only ever assigned insideupdate_zcc_workers(), whose sole callermaybe_update_zcc_worker()short-circuits oninner_opt.fused_buffer_version == self.manager.cache_version.fused_buffer_versiondoes not change in steady state, so the value is written once per run(the step of the first save) and then frozen.
self.global_step.value = global_step),so the manager ends up comparing
Xagainst its ownX.The barrier therefore passes on the very first comparison from the second save onwards.
This matters because the ZCC worker is a
spawn-ed process that maps the trainer's fused GPUbuffers over CUDA IPC and does the D2H copy on its own CUDA context and stream. There is no
CUDA stream ordering between the trainer's compute stream and the worker's copy stream — this
handshake is the only ordering guarantee. With it broken, nothing stops step
N+1from mutatingGPU memory that the step-
Nsnapshot is still reading, which can silently corrupt savedparameters / optimizer moments.
Evidence from a 2496-GPU run
Waiting current worker offloading done(the branch that actually waits)Current worker offloading donemanager_stepThe run had
Offload chunks: 1(~9 GB per card in a single D2H), and anon_step_begincallback that quantizes MoE expert weights to FP8 and calls
optimizer.clear_param_storage(...),i.e. a GPU mutation ~3s before
on_optimizer_beginwhere the barrier used to sit.Changes
manager.global_stepis refreshed every step it is used. Both branches ofZeroCostCheckpointCallback.on_step_endnow assignself.manager.global_step = state.global_stepright after
maybe_update_zcc_worker()and before requesting a worker. This is what the# set 'on-step-end'comment on the attribute declaration originally intended.state.global_stepis already incremented beforeon_step_end, so the value stays>= 1and the
assert global_step != 0inget_idle_worker_for_savingis preserved.The synchronization point moves to the beginning of the next step. Even a working barrier
at
on_optimizer_beginis too late, becauseon_step_begincallbacks may already mutate thebuffers. New
ZeroCostCheckpointManager.maybe_sync_offload_status()is called fromTrainer._inner_training_loopjust beforeon_step_begin. It is guarded bycurrent_pipeline_hook_step != pipeline_hooks_steps: when the offload is still being slicedacross pipeline hooks (PP > 1, or
pipeline_hooks_steps > 1), the remaining chunks are onlydispatched during this step, so waiting there would deadlock — those topologies keep
synchronizing at
on_optimizer_beginexactly as before. The originalon_optimizer_beginsync is left in place as a backstop.Two files, +45 lines, no deletions.
Known remaining limitations (pre-existing, not addressed here)
pipeline_hooks_steps > 1, only the first chunk is dispatched aton_step_end; the restcome from the next step's
on_substep_end/ PP hooks, i.e. after anyon_step_beginmutation.The step-begin guard correctly declines to wait for those topologies.
on_step_enddoes not callzcc_pipeline_hook, so its chunk is alsodispatched by the next step's
on_substep_end.global_step.valueafter bothwait_all()calls and before
process_dump_task), not for the disk write.get_fused_param_mappings()capturesbuffer.param_buffer_ipc_metaonce per run, butclear_param_storage()/reset_param_storage()free and re-allocateparam_storagefor thecleared colors on every step, so the IPC handles held by the worker stay valid only while the
caching allocator happens to hand back the same block. Out of scope here.
on_step_begincallback mutates the fused buffers, the D2H can nolonger overlap the next step's forward/backward at all. That is inherent to the design, and
the correct trade against saving corrupt state.
Verification
ZeroCostCheckpointManagerwas instantiated viaobject.__new__with a fake worker whoseglobal_step.valuewrite-back is delayed by a configurable number of polls, and module-leveltime.sleepmonkeypatched to count polls. This exercises the realsync_offload_status,maybe_sync_offload_statusandzcc_pipeline_hook:pipeline_hooks_steps > 1and PP > 1;