Prepared-metadata reuse for collective-free async checkpoint saves#359
Prepared-metadata reuse for collective-free async checkpoint saves#359asolergi-nv wants to merge 5 commits into
Conversation
Add an optional prepared/complete Metadata reuse path to the async save. reuse_metadata_obj skips both the planning gather_object(local_plan) and the finalize gather_object(write_results), detecting write failures via a single all_reduce(MIN) and writing the prepared metadata verbatim. create_metadata_path persists the complete metadata on the first full save for later reuse. Backs Megatron's --ckpt-metadata. Adds a unit test covering the create+reuse round-trip and the gather-skip. Signed-off-by: asolergi-nv <asolergibert@nvidia.com>
|
🔗 Paired PR (must land together): Megatron |
| raise CheckpointException("write", node_failures) | ||
| if dist_wrapper.is_coordinator: | ||
| write_start = time() | ||
| _write_metadata_only(storage_writer, reuse_metadata_obj) |
There was a problem hiding this comment.
How are you ensuring that the length/offset are correctly collected from WriteResult reported by every rank here?
There was a problem hiding this comment.
Good question — this is the core invariant the reuse path relies on, so let me be explicit. The per-chunk offset/length in storage_data are a function of the checkpoint structure — item shapes/dtypes and the write-bucketing (which depends on world size and --dist-ckpt-workers) — not of the tensor values. For a fixed structure the byte layout is therefore identical save-to-save, which is exactly what makes a prepared metadata reusable.
I've made the unit test demonstrate this directly (also addressing your other comment): it now re-randomizes every parameter between the create save and the reuse save, then asserts the reuse checkpoint — whose .metadata is the verbatim prepared metadata built from the old weights — loads back to the new weights. If any offset/length depended on the data, that round-trip would fail. The validity constraint (same structure / world size / --dist-ckpt-workers / --keep-redundant-extra-state) is documented in the docstring and PR body and is the caller's responsibility.
If you'd like a stronger guarantee we could add a purely-local save-time check that compares this rank's actual WriteResult offsets/lengths against the prepared metadata (no collective needed) and fails fast on a mismatch — happy to add that as a follow-up if you think it's worth it. Addressed in 7ce1029.
There was a problem hiding this comment.
One potential idea, instead of a fail-fast would be to hijack the mechanism that records whether ranks are done writing. Currently, that's an all-reduce/all-gather of a single value tensor, which can be 0 or 1. We could add a value 2 that indicates the rank was done writing, but its offsets/lengths/something else about distcp file has changed and the metadata cache can't be reused.
Greptile SummaryThis PR adds a prepared-metadata reuse path to NVRx's async checkpoint saver, allowing both
Confidence Score: 5/5Safe to merge; all previous reviewer concerns are addressed and no new correctness bugs were found. The implementation is carefully constructed: the bidirectional layout guard catches all documented drift scenarios, the mutual exclusion of reuse and create is enforced with an explicit ValueError, the temp+rename write is atomic on the POSIX path, and f.flush() precedes os.fsync. The only finding is that _write_metadata_only writes the prepared metadata verbatim, so the new checkpoint's .metadata carries storage_meta.checkpoint_id from the create-time checkpoint directory rather than the current one — a provenance staleness issue for tooling, not a loading bug. No files require special attention beyond the _write_metadata_only call site noted in the inline comment. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant R as All Ranks
participant C as Coordinator (rank 0)
participant FS as Filesystem
rect rgb(200, 230, 200)
Note over R,FS: CREATE mode (first save — collective path)
R->>R: save_state_dict_async_plan (gather_object local_plan)
R->>C: gather_object(write_results) in finalize
C->>FS: storage_writer.finish() → writes .metadata
C->>FS: _read_written_metadata() → reads back complete Metadata
C-->>R: broadcast_object_list(complete_metadata)
C->>FS: _atomic_pickle_write(complete_metadata, create_metadata_path)
Note over R: Returns complete_metadata to caller
end
rect rgb(200, 200, 240)
Note over R,FS: REUSE mode (subsequent saves — collective-free)
R->>R: save_state_dict_async_plan (skips gather_object, purely local)
R->>R: _find_layout_mismatch(write_results, reuse_metadata_obj)
R->>R: all_reduce(MIN, local_status) — single 1-element collective
alt "global_status == 2 (all match)"
C->>FS: _write_metadata_only(reuse_metadata_obj) — atomic tmp+rename
else "global_status == 1 (layout drifted)"
R->>R: raise CheckpointException(metadata_reuse)
else "global_status == 0 (write failure)"
R->>R: raise CheckpointException(write)
end
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant R as All Ranks
participant C as Coordinator (rank 0)
participant FS as Filesystem
rect rgb(200, 230, 200)
Note over R,FS: CREATE mode (first save — collective path)
R->>R: save_state_dict_async_plan (gather_object local_plan)
R->>C: gather_object(write_results) in finalize
C->>FS: storage_writer.finish() → writes .metadata
C->>FS: _read_written_metadata() → reads back complete Metadata
C-->>R: broadcast_object_list(complete_metadata)
C->>FS: _atomic_pickle_write(complete_metadata, create_metadata_path)
Note over R: Returns complete_metadata to caller
end
rect rgb(200, 200, 240)
Note over R,FS: REUSE mode (subsequent saves — collective-free)
R->>R: save_state_dict_async_plan (skips gather_object, purely local)
R->>R: _find_layout_mismatch(write_results, reuse_metadata_obj)
R->>R: all_reduce(MIN, local_status) — single 1-element collective
alt "global_status == 2 (all match)"
C->>FS: _write_metadata_only(reuse_metadata_obj) — atomic tmp+rename
else "global_status == 1 (layout drifted)"
R->>R: raise CheckpointException(metadata_reuse)
else "global_status == 0 (write failure)"
R->>R: raise CheckpointException(write)
end
end
Reviews (5): Last reviewed commit: "Make reuse layout guard bidirectional" | Re-trigger Greptile |
| async_queue.maybe_finalize_async_calls(blocking=True, no_dist=False) | ||
| return finalize_result.get('metadata') | ||
|
|
||
| def test_prepared_metadata_reuse_roundtrip(self, tmp_path_dist_ckpt): |
There was a problem hiding this comment.
You should replace model weights by random weights between the metadata reuse. That will check that the metadata reuse holds for multiple training iterations.
There was a problem hiding this comment.
Done in 7ce1029. The test now re-randomizes every parameter between the create and reuse saves (emulating a later training iteration — same structure, new data) and asserts the reuse checkpoint loads back to the new weights. This exercises reuse across changed data and, as noted on your other comment, demonstrates that the reused offsets/lengths depend only on structure, not on the values.
- _write_metadata_only: flush the file object's buffer before fsync and drop the unlink-then-rename step (POSIX rename atomically replaces the destination), so the `.metadata` write is genuinely atomic and durable as the docstring claims. - save_state_dict_async_finalize: fix the `Returns` docstring (the reuse path returns Optional[Metadata]); document that on the reuse path peer ranks raise a detail-less CheckpointException, and log an actionable pointer to the failing rank's logs before doing so. - test_prepared_metadata_reuse_roundtrip: re-randomize the model weights between the create and reuse saves and assert the reuse checkpoint loads back to the new weights, proving the reused per-chunk offsets/lengths depend only on the checkpoint structure, not the tensor values. Signed-off-by: asolergi-nv <asolergibert@nvidia.com>
These two finalize parameters are mutually exclusive: reuse skips the collective save that create needs to build the complete metadata. Without a guard, passing both silently took the reuse branch, dropped the create/persist step and returned None. Raise ValueError instead so the contract holds even outside the Megatron wiring (which already gates the two on prepared_metadata being None/not-None). Signed-off-by: asolergi-nv <asolergibert@nvidia.com>
_find_layout_mismatch previously checked only the items this rank wrote against the prepared metadata, so an item present in the prepared metadata but not written this save -- a removed or resharded item at the end of a file, which shifts nothing -- slipped through and would publish a stale .metadata with a dangling entry that loads the wrong bytes. Add the reverse check, scoped to the files this rank actually wrote, and warn when the non-decentralized planning path cannot skip the gather. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
Reuse a prepared
.metadataacross checkpoint saves (collective-free save)This is a linked pair of PRs — both must land for the feature to work:
asolergi-nv/nvidia-resiliency-ext, branchfeat/ckpt-metadata-reuse): adds the backend reuse path to the async saver.asolergi-nv/Megatron-LM, branchfeat/ckpt-metadata-reuse): adds the--ckpt-metadata/--ckpt-metadata-createknobs that drive it.The Megatron PR depends on the NVRx PR (it passes the new
reuse_metadata_obj/create_metadata_patharguments). Merge/release NVRx first.Problem
On the NVRx async save path, every distributed checkpoint save performs two
cross-rank collectives that scale with world size and are pure coordination
(no model data):
gather_object(local_plan)to the coordinator to build theglobal
Metadata.gather_object(write_results)to assemble and write.metadata.For large jobs these gathers are a non-trivial, recurring tax on every save,
even though the checkpoint structure is constant across an entire run
(same model, parallelism, and world size). The global
Metadatais thereforeidentical save-to-save and across resumes — it is wasteful to rebuild it every
time via collectives.
Solution
Allow a prepared, complete
Metadataobject to be supplied to the save, soboth gathers are skipped entirely:
NVRx side (
async_ckpt/state_dict_saver.py)save_state_dict_async_plan(..., reuse_metadata_obj=None): when a preparedmetadata is supplied, the decentralized planning branch skips both the
verify_global_md_reuseall-reduce and thegather_object(local_plan)—planning becomes purely local.
save_state_dict_async_finalize(..., reuse_metadata_obj=None, create_metadata_path=None):the reuse path skips
gather_object(write_results)and instead detects writefailures with a single
all_reduce(MIN)of a 1-element flag, then writesthe prepared metadata verbatim as
.metadata.create_metadata_path): the first save runs the normal(collective) path, then the coordinator reads back the just-written complete
metadata, broadcasts it, and persists it to a container-local file so later
saves in the job/run can reuse it. Returns the complete metadata.
A single, tiny
all_reduce(MIN)is intentionally retained on the reuse path forwrite-failure safety (so a failed rank still aborts the whole job).
Megatron side
--ckpt-metadata <FILE>: reuse mode — load the prepared.metadata(pickle)and hand it to the save strategy as
prepared_metadata(skips both gathers).--ckpt-metadata-create: first-job mode — the first save creates the<FILE>via the collective path, then subsequent saves reuse it.TorchDistSaveShardedStrategygainsprepared_metadata/ckpt_metadata_create_pathand passes
reuse_metadata_obj/create_metadata_paththrough to NVRx; theasync
finalize_fncaptures the returned complete metadata in create mode.Save-side collectives in NVRx (and which disappear)
Each NVRx async save (decentralized plan path) triggers these cross-rank
collectives — none of which move model/optimizer data; they only coordinate
the checkpoint's
Metadata:save_state_dict_async_plan)all_reduceinsideverify_global_md_reusegather_object(local_plan)→ coordinatorMetadatasave_state_dict_async_finalize)gather_object(write_results)→ coordinator.metadatabroadcast(failures_occurred)(1 int)Which disappear after the first save(s), and why
all_reduce+gather_object(local_plan)— eliminated by NVRx'sown
CheckpointMetadataCache. With--ckpt-assume-constant-structure, the cachecaches the central plan and, once it has observed an identical structure on
two consecutive saves, sets
validated_cache_reuse=Trueand reuses thecached central plan. So these run on save Add GitHub Actions workflow for building docs #1 and Lint code (black, isort, ruff checks) #2 (the cache needs two
matching observations to validate) and then vanish from save add linting code actions #3 onward.
gather_object(write_results)+broadcast— not cached bystock NVRx: the per-rank write results are re-gathered to rebuild
.metadataon every save, so this collective never disappears on its own.
What
--ckpt-metadatachangesSupplying a prepared, complete
Metadatalets the save skip both gathers:reuse_metadata_objis set → the planning branch skips theverify_global_md_reuseall_reduceandgather_object(local_plan)(planning is purely local), and the finalize skips
gather_object(write_results),replacing it with a single
all_reduce(MIN)of a 1-element flag forwrite-failure detection. This holds from the very first reuse save — no
warm-up saves needed — and is independent of
CheckpointMetadataCache.metadata, plus a one-time
broadcastof the complete metadata to all ranks,then persists it; every later save reuses it collective-free.
Net steady-state: the only remaining save-side collective is the 1-element
all_reduce(MIN)failure check (kept by design). (The separate mcore-levelcollectives — the FullyParallel save-distribution
all_gather_objectand thesharding-integrity
determine_global_metadatagather — are addressed by thesibling Megatron PRs M4 and M5 and are out of scope here.)
Correctness constraints
Reuse is only valid when the run that reuses a
.metadatahas the samecheckpoint structure, world size, and
--dist-ckpt-workersas the run thatcreated it (the caller owns this guarantee). In particular the prepared
metadata must come from a save done with the same
--keep-redundant-extra-statesetting, since that toggle changes the saved item set and per-file byte
offsets. A stale/mismatched prepared metadata produces a wrong shard map and a
load-time unpickling error.
Usage
First job (create once):
Subsequent jobs (reuse, collective-free saves):
Result
With
--ckpt-metadatareuse active (and constant structure), the only remainingsave-side collective is the 1-element
all_reduce(MIN)failure check. Bothgather_objects are eliminated.