Skip to content

fix(storage#782): exclude write-side teardown from §4.7.1 failover-callout gap#787

Merged
FileSystemGuy merged 2 commits into
mainfrom
fix/issue-782-failover-callout-write-teardown
Jul 13, 2026
Merged

fix(storage#782): exclude write-side teardown from §4.7.1 failover-callout gap#787
FileSystemGuy merged 2 commits into
mainfrom
fix/issue-782-failover-callout-write-teardown

Conversation

@FileSystemGuy

Copy link
Copy Markdown
Contributor

Summary

Fixes #782. The split-mode failover-callout gap check charged the write phase's post-benchmark work — final multi-node cluster collection, metadata serialization, interpreter shutdown — against the 30-second budget by measuring from write.summary.end_time (end of the timed section). On large topologies that teardown can take tens of seconds, causing false failures on legitimate submissions whose real inter-invocation handoff is within budget.

Adds a symmetric write-side bookend to mirror the read-side fix from #714:

  • _invocation.py — new INVOCATION_END + mark_invocation_end(), symmetric with the existing INVOCATION_START.
  • benchmarks/base.py — calls mark_invocation_end() at the top of write_metadata(), the latest moment where the value can still land in the persisted file. Emits it as metadata['invocation_end_time'].
  • Submission checker cache_flush_validation — prefers write.metadata.invocation_end_time as the write-side origin, falling back to write.summary.end_time for older results dirs (pre-fix behavior). Emits an INFO line per pair with the chosen origin so the measurement is transparent regardless of pass / warn / fail.

Now the two bookends are genuinely symmetric: whatever falls outside INVOCATION_START / INVOCATION_END is excluded from the gap on both sides — framework startup on the read side, and post-benchmark cluster collection, rank-0 teardown, JSON serialization, and interpreter shutdown on the write side.

Worked example (from #782)

Quantity Value
write.summary.end_time (timed section ends) 2026-07-13T06:36:25
write.metadata.invocation_end_time (nodes released, rank 0 exiting) 2026-07-13T06:37:03
read.metadata.invocation_start_time 2026-07-13T06:37:29
gap charged before fix (read_start − summary_end) 64.4s → FAIL
gap charged after fix (read_start − invocation_end) ~27s → PASS

Test plan

  • mlpstorage_py/tests — 850 passed (includes 3 new §4.7.1 tests)
  • tests/ — 2907 passed, 1 skipped, 13 deselected
  • vdb_benchmark/tests — 174 passed
  • kv_cache_benchmark/tests — 238 passed
  • Reviewer sanity-check on a real split-mode submission whose failure motivated the issue

New tests

…llout gap

The split-mode failover-callout gap check charged the write phase's
post-benchmark work — final multi-node cluster collection, metadata
serialization, interpreter shutdown — against the 30-second budget by
measuring from write.summary.end_time (end of the timed section). On
large topologies that teardown can take tens of seconds, causing false
failures on legitimate submissions whose real inter-invocation handoff
is within budget.

Adds a symmetric write-side bookend to mirror the read-side fix from
storage#714. _invocation.INVOCATION_END is captured at the top of
write_metadata() — the latest moment where the value can still land in
the persisted file — and emitted as metadata.invocation_end_time. The
submission checker prefers it over summary.end_time, so post-benchmark
teardown is now excluded from the gap on both sides of the handoff.
Falls back to summary.end_time for older results dirs (pre-fix
behavior). Emits an INFO line per pair with the chosen origin so the
measurement is transparent regardless of pass/warn/fail.
@FileSystemGuy
FileSystemGuy requested a review from a team July 13, 2026 16:55
@github-actions

Copy link
Copy Markdown

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

@FileSystemGuy
FileSystemGuy merged commit 7b44f61 into main Jul 13, 2026
4 checks passed
@FileSystemGuy
FileSystemGuy deleted the fix/issue-782-failover-callout-write-teardown branch July 13, 2026 17:01
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.

Validation §4.7.1 checkpointCacheFlushValidation unfairly charges the write phase's final multi-node collection to the failover-callout budget

1 participant