Skip to content

fix(megatron): finalize checkpoints before pruning - #2145

Draft
bvolpato wants to merge 1 commit into
NovaSky-AI:mainfrom
bvolpato:bvolpato/fix-async-checkpoint-publication
Draft

fix(megatron): finalize checkpoints before pruning#2145
bvolpato wants to merge 1 commit into
NovaSky-AI:mainfrom
bvolpato:bvolpato/fix-async-checkpoint-publication

Conversation

@bvolpato

@bvolpato bvolpato commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

With Megatron async checkpointing, the trainer can advance latest_ckpt_global_step.txt and delete the previous checkpoint while the new save is still pending. Megatron writes its completion metadata during finalization, so completed background tensor writes alone do not make the new checkpoint loadable. With max_ckpts_to_keep=1, an interrupted run can lose its last resumable checkpoint.

This waits for policy and critic checkpoint finalization before publishing the latest step or running retention. It covers PPO, SFT, and fully async PPO. The fully async trainer also writes its required UID/epoch state before the base trainer publishes the checkpoint.

Related to #1838. This fixes publication ordering; it does not change the checkpoint format or make marker-file writes atomic.

Reproduction and validation

The new tests hold model writes pending until finalization, exercise policy and critic failures, and verify that retention keeps the previous checkpoint until the new one completes:

uv run --isolated --extra skyrl-train --extra dev pytest --noconftest \
  tests/train/test_checkpoint_publication.py -q
  • 13 regression cases passed with this change; the same 13 failed against parent commit 345ce86 using normal imports.
  • 6 existing checkpoint-retention, consistency, and SFT callback tests passed.
  • Tests ran in an isolated Python 3.12 environment with PyTorch 2.11 CPU and the trainer dependencies. --noconftest avoids the unrelated session-wide Ray startup for these filesystem tests.
  • Ruff, Black, and gitleaks pre-commit hooks passed.
  • No GPU or distributed checkpoint run was available on this host. Keeping this draft pending that validation.

Downsides

Trainer checkpoint saves now wait for the background disk write and finalization. Async checkpoint writes no longer overlap subsequent training steps. Direct worker saves retain their asynchronous behavior. This keeps the fix small and avoids introducing concurrent finalization collectives or a separate pending-publication lifecycle.

Risk and rollback

The change adds a completion barrier to each trainer checkpoint save; synchronous backends use their existing no-op finalization. If reverting, disable async_dist_ckpt_save first to avoid restoring the publication/retention race.

Signed-off-by: bvolpato <brunocvcunha@gmail.com>
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