[tp] Consolidate branches and validate vLLM/SGLang#6
Draft
rchalamala wants to merge 168 commits into
Draft
Conversation
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Signed-off-by: xenshinu <liuxs@umich.edu>
Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Signed-off-by: xenshinu <liuxs@umich.edu>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
…markers
Replace the on-volume READY-last/rollback/renameat2 protocol with a dedicated
persistent publication_index (modal.Dict). Run ids get a cryptographically
random suffix; publication fails closed if the run id is already indexed or the
canonical path exists. Flow: copy validated archive+manifest to the unique
canonical path, pre-serialize the payload, bounded-retry archive_volume.commit(),
then as the single final side effect write publication_index[run_id] =
{snapshot_digest, published_path, foundry_ref}. The Dict is never written before
a successful commit; nothing fallible runs after it except returning the payload.
Consumers require an exact Dict entry (digest/path/ref) plus a matching on-volume
manifest and archive child before copying; auto-committed-but-unindexed archives
are inert. Commit failure => no index; Dict failure => inert unindexed archive;
neither rolls back, so ExceptionGroup and the Python 3.11 dependency are gone.
Snapshot digest is a sha256 over canonical snapshot JSON.
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
…LOAD reload
Publish the vLLM TP archive via an atomic modal.Dict marker instead of a plain
Dict assignment:
- run_save_transaction's single final side effect is _write_publication_marker,
which uses publication_index.put(run_id, record, skip_if_exists=True). A False
return reconciles idempotently against the stored record (exact match = success,
difference = collision); an ambiguous put exception is reconciled by fetching the
record, else the original error is preserved.
- The stored record carries {run_id, snapshot_digest, published_path, foundry_ref,
payload}; payload holds the snapshot plus reduced per-phase primitives only. Its
serialized size (max of JSON/pickle) is preflighted against
PUBLICATION_RECORD_MAX_BYTES before any publish.
- main obtains the transaction via _obtain_transaction/_reconcile_transaction: if
the remote raises but a self-consistent marker exists, recover its payload;
otherwise re-raise. Genuine failures without a marker stay failures.
- LOAD calls archive_volume.reload() before reading the Dict/manifest/bytes.
- Add delete_published_run cleanup hook (Task 7 contract) using the record's
run_id/published_path.
CPU contracts cover atomic put/skip_if_exists idempotency and collisions, ambiguous
put reconciliation, main recovery, bounded/oversized records, warm-container reload
ordering and stale-record visibility, and the cleanup hook.
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
…conciliation Harden the publication authority contract: - delete_published_run derives the deletion target exclusively from run_id (never from the record's published_path), refuses any record failing full schema/consistency validation, validates the on-volume manifest/archive, then strictly rmtrees (no ignore_errors) + verifies absence, commits with bounded retries, and pops the Dict marker last. Commit failure keeps the marker; pop failure leaves the bytes gone (consumers reject via the missing archive). - _record_is_consistent now derives the canonical path from run_id and requires record+payload to name it, plus run_id/foundry_ref/digest and the full required payload schema. Consumers (_require_indexed_published_run) and cleanup reuse it; run_save_transaction's idempotent replay reloads and re-validates on-volume. - _reconcile_transaction validates the full record schema and returns the payload only; on failure it raises a new Python-3.10-compatible PublicationReconciliationError with the original remote error preserved as __cause__. Documented that local main cannot inspect the volume and the first LOAD performs on-volume validation before use. - Corrected the record size/fingerprint and volume-layout comments. Tests cover malicious/foreign record paths, missing payload fields, mismatched run_id/path/ref/digest, stale record with missing archive, strict cleanup remove/refuse/commit-failure/pop-failure and reload->commit->pop ordering, and original-error __cause__ preservation. Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
…dempotent cleanup retry
Deepen the publication-record contract and consumer/cleanup validation:
- _record_is_consistent now deep-validates the payload: results must be exactly
{seed,save,save2} with typed reduced fields (save2's query outputs included,
bool rejected where int is required), and the snapshot must satisfy the full
schema (rank keys, exact per-rank counts, positive offsets, inventory/semantic/
byte maps present and typed) with a digest recomputed from the payload snapshot.
It never raises on malformed nested data (guarded digest) so reconciliation
never masks the original error.
- _require_indexed_published_run collects a fresh collect_archive_snapshot of the
on-volume archive bytes and compares inventory/counts/offsets/semantic/byte maps
exactly to the expected snapshot (verify_bytes, default on). Consumers and the
idempotent replay guard use it, so a byte-corrupted/drifted published archive is
rejected despite a present manifest/dir. Cleanup passes verify_bytes=False.
- delete_published_run adds an idempotent retry: if the canonical dir is already
absent (prior deletion committed, pop failed), pop the marker and succeed; if the
dir exists but manifest/archive are missing, fail.
- Corrected record size/fingerprint and volume-layout comments.
Tests cover deep results/snapshot rejection, malformed-nested-data reconciliation
preserving __cause__, consumer and idempotent-replay byte-corruption rejection, and
the cleanup partial-success retry.
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
…d orphan delete_published_run now reloads first, derives the canonical path, then fetches the marker and branches on marker presence: - marker absent + canonical absent -> return idempotent success (covers a pop that committed but whose ack was lost, and repeated cleanup of an already-deleted run); - marker absent + canonical present -> fail closed as an unindexed orphan (never delete an unvalidated tree); - marker present + inconsistent -> refuse; - marker present + canonical absent -> pop the marker and succeed (prior deletion committed the bytes but its pop failed); - marker present + canonical present -> validate presence, strict rmtree + verify, bounded commit, pop last. Tests add a pop-write-then-ack-lost first attempt followed by an idempotent retry, and a fully-absent repeated cleanup; the missing-record test now asserts the unindexed-orphan fail-closed path. Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Add an optional expected_stems callable to validate_archive_inventory and collect_archive_snapshot. When supplied, each graph index's paired stem must equal the profile's exact descriptor, rejecting an extra or malformed-but-paired descriptor even when the count, indices, and pairing are all correct. The parameter is optional so callers with a different profile (e.g. SGLang) keep the count/index/pairing contract unchanged. Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
… set
Pinned seed profiling legitimately captures duplicate batch sizes (e.g. 64/63
before the normal 64, yielding 66 pairs). Seed's archive is diagnostic only
and overwritten by the next SAVE, so it must not be held to the exact-64
canonical contract.
- _run_transaction_phase: seed records bounded diagnostic graph counts/file
list (_collect_seed_diagnostics) instead of the exact-64 validator; it still
requires a healthy process, clean teardown, fatal-free logs, exact P2P/IPC,
and full provenance via _run_server_phase.
- save/save2 now validate the exact canonical descriptor set
(graph_{i}_FULL_t{i+1}_r{i+1}_UX_pcN) via _expected_graph_stem, rejecting
extra/malformed descriptors even at count 64.
- Payload schema accounts for seed's diagnostic shape separately from the
save/save2 canonical reduced fields (_COMMON/_SAVE/_SEED reduced types).
- _collect_save_evidence appends bounded graph filename/count diagnostics to a
PhaseFailure for future debugging.
_validate_transaction already compares only save/save2 full inventory/repro
while still checking seed transport/errors, and provenance is enforced per
phase.
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
…duced schema
Three review findings, source-confirmed from recipe/vllm/serve_qwen3-8b_tp.sh
(CUDAGRAPH_CAPTURE_SIZES=seq 1 64) + vLLM largest-first capture + graph_ops
_build_graph_filename (tokens=batch_descriptor.num_tokens, capture_index
increments per capture):
1. Canonical save/save2 stems are descending capture order: capture_index i ->
batch size 64-i, so graph_{i}_FULL_t{64-i}_r{64-i}_UX_pcN (index 0 = t64,
index 63 = t1). _expected_graph_stem and every fixture/hardcoded stem updated.
2. Seed66 fixture models the real capture order: profiling sizes 64/63 at
indices 0/1, then the normal descending 64..1 at indices 2..65. The bounded
diagnostic now exposes duplicate batch sizes (duplicate_sizes -> {64:2,63:2})
while the phase still accepts seed.
3. _reduced_results_are_wellformed enforces the exact allowed schema per phase:
common required + phase-specific required (+ query extras only on save2) and
nothing else — rejecting seed carrying save fields, save/save2 carrying
seed_diagnostics, stray query extras, and any unknown field.
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
mainLineage and branch disposition
6c77e9ba8e43442421a80fea7991ee26f97f1003b170e29origin/main...HEAD:0 168main:0Late generalized SGLang graph/catalog/speculative profiles and CUDA dependency-edge metadata remain explicitly quarantined; they are not silently claimed by these validated profiles.
Verification
Local
320 passed21 passedpython3 -m compileall -q python testsgit diff --check origin/main...HEADNative H100
Run: https://modal.com/apps/modal-labs/rahul-dev/ap-0aeSCJNb6eHQhqdMkpyfXk
a8e4344, exit 0DeepEP 2×H100
Run: https://modal.com/apps/modal-labs/rahul-dev/ap-Q8mb8U4EYuE5Jo6Zb0xwrH
fabric,nvl_ipc, andnvl_ipc_preallocall passed SAVE→LOADa8e4344, exit 0SGLang main TP=2 — VERIFIED
Pinned SGLang
a25164bda355250015c0ba6d8821f0d4b37abe7d, modelb968826d9c46dd6066d109eabc6255188de91218.Focused: https://modal.com/apps/modal-labs/rahul-dev/ap-5wO9cFhRZ7ITsLDDv27Xfh
Full matrix: https://modal.com/apps/modal-labs/rahul-dev/ap-6fYcSUFLOxocBSjNxakNGZ
68908220416in every Foundry phasea8e4344, all 20 checks passedvLLM TP=2 — VERIFIED
Run: https://modal.com/apps/modal-labs/rahul-dev/ap-YfKALdMwnyJQmzQoVvZSMU
Pinned vLLM
4309c257d3f639e5490d3811293c890c61c76f29, wheel parent6cbe448eed751824d608faf9078ef84724d621c1, model revisionb968826d9c46dd6066d109eabc6255188de91218.64/6470724354048for both ranks across SAVE/SAVE2/LOAD/LOAD2a8e4344, all 17 checks passedSupported scope
Experimental pinned profiles only:
TP>2, multi-host, unpinned engine revisions, alternate collective/graph modes, SGLang EP, MoE TP, and hybrid/speculative profiles remain unsupported or unvalidated.