Skip to content

Validate bounded vLLM cache token sources on H100 - #2796

Open
cquil11 wants to merge 29 commits into
mainfrom
codex/h100-cache-source-validation
Open

Validate bounded vLLM cache token sources on H100#2796
cquil11 wants to merge 29 commits into
mainfrom
codex/h100-cache-source-validation

Conversation

@cquil11

@cquil11 cquil11 commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • reduce the MiniMax-M3 H100 AgentX validation to one Mooncake DRAM point at c8
  • run one simple NVMe point at c30
  • run one native DRAM+NVMe tiered point at c20
  • collect the patched vLLM cached-token source metrics in official sweep artifacts

Validation

Local matrix generation emits exactly the three intended points. The official full sweep will be linked here after the GitHub-visible diff and image are verified.


Note

Medium Risk
Changes cluster job allocation, container mounts, and KV offload connectors for long-running AgentX runs; mistakes could waste GPU hours or produce non-comparable metrics, but scope is limited to a narrow H100 validation config.

Overview
Adds NVMe-only and DRAM+NVMe tiered KV offload modes for H100 MiniMax-M3 AgentX, alongside the existing Mooncake DRAM path, and narrows the official sweep to three validation points on a patched vLLM image that exports bounded cached-token-by-source metrics.

Config and matrix plumbing now accept kv-offloading values nvme and declarative ['dram', 'nvme'] (runtime dram+nvme), with schema/sweep generation enforcing DRAM utilization for tiered DRAM legs, single-node-only NVMe rules, and distinct experiment-name suffixes. Shared benchmark validation in benchmark_lib.sh mirrors those modes (NVMe skips TOTAL_CPU_DRAM_GB).

H100 launcher provisions a job-scoped native NVMe directory, mounts it into the container as /kv-offload, and extends Slurm allocation to 420 minutes for MiniMax-M3 agentic nvme / dram+nvme (override via SALLOC_TIME_LIMIT). The AgentX recipe wires SimpleCPUOffloadConnector (disk) vs OffloadingConnector tiering and requires AIPerf artifacts to include vllm:prompt_tokens_cached_by_source.

Sweep config minimaxm3-fp8-h100-vllm-agentic-mtp drops the prior wide cliff sweep in favor of Mooncake DRAM c8, Simple NVMe c30, and native tiered c20; docs and perf changelog record the longer wall time and validation intent.

Reviewed by Cursor Bugbot for commit 992df5d. Bugbot is set up for automated code reviews on this repo. Configure here.

@cquil11 cquil11 closed this Sep 1, 2026
@cquil11 cquil11 reopened this Sep 1, 2026
…ce-validation

# Conflicts:
#	perf-changelog.yaml

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit fd88716. Configure here.

"
NVME_CONTAINER_MOUNT=",$NVME_HOST_DIR:/kv-offload"
export NVME_OFFLOAD_DIR=/kv-offload
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NVMe job cache is never cleaned up

High Severity

The launcher creates a job-scoped NVMe directory under the node's enroot cache and mounts it for nvme and dram+nvme runs, but cleanup_allocation only cancels the Slurm job. Up to 8 TB of KV files stay on the shared NVMe after every run, so later jobs and enroot imports can fail with no space.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit fd88716. Configure here.

echo "Error: CPU DRAM budget is too small for checkpoint cache, model, and DRAM+NVMe KV offload" >&2
exit 1
fi
CPU_OFFLOAD_TOTAL_BYTES=$((PER_RANK_GIB * TP * 1073741824))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Native tier budget subtracts Mooncake buffer

Low Severity

The dram+nvme branch reuses the Mooncake DRAM formula and still subtracts MOONCAKE_LOCAL_BUFFER_GIB even though this path uses OffloadingConnector and never starts Mooncake. cpu_bytes_to_use is therefore undersized by 4 GiB per rank relative to the configured host DRAM budget.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit fd88716. Configure here.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings marked 🟡 are optional suggestions and need no follow-up push.

Comment thread perf-changelog.yaml Outdated
Comment on lines +6580 to +6590
- config-keys:
- glm5.2-fp4-mi355x-sglang-agentic-mtp
- minimaxm3-fp8-h100-vllm-agentic-mtp
scenario-type:
- agentic-coding
description:
- "Switch the TP8 arm from EP=8 to EP=1: removing the MoE all-to-all collective reduces ITL p50 by ~5% at conc 4 (6.95 ms vs 7.3 ms baseline) and raises interactivity P90 from 105 to 110.5 tok/s/user. Extend the TP8 concurrency sweep to [4, 6, 8, 10] to cover the mid-range crossover region."
- "Raise HiCache ratio from 1.5 to 2.5 and switch write policy to write_through_selective (per PR #2679): the agentic-coding corpus saturates any fixed DRAM pool at conc ≥ 10; the larger host tier sustains throughput through conc 12 (+12% tok/s/GPU vs conc 10). Trim the TP4 concurrency list to [1, 2, 4, 8, 10, 12] removing the untested conc 16 point."
- "Double MAX_RUNNING_REQUESTS to 2×CONC: MTP draft+verify transiently batches more tokens than CONC sessions; the extra headroom prevents scheduler stalls under burst. Cap CUDA_GRAPH_MAX_BS at 64 (SGLang auto-interpolates a batch-size list up to the cap)."
- "Trim TP8/EP=1 concurrency list from [4, 6, 8, 10] to [4, 10]: sweep validation shows c6 and c8 dominated by the TP4/EP4 HiCache arm at those concurrency points."
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2777
- "Run a narrow H100 MiniMax-M3 AgentX validation on the patched vLLM image that exports the bounded cached-token sources device, cpu, disk, p2p, and external and includes vLLM PR #53087's bounded fallback for stalled tier-primary writes."
- "Validate one Mooncake DRAM point at TP8 concurrency 8 and one NVMe-only point at TP8 concurrency 30 using SimpleCPUOffloadConnector's disk backend with 8 TB aggregate capacity."
- "Validate one declarative kv-offloading [dram, nvme] point at TP8 concurrency 20, mapped by the vLLM recipe to OffloadingConnector's TieringOffloadingSpec with an LRU DRAM primary tier and node-local filesystem secondary tier."
- "Mount a job-scoped directory from the H100 node's native NVMe filesystem into the Pyxis container."
- "Collect the vLLM Prometheus endpoint through AIPerf so artifacts include vllm:prompt_tokens_cached_by_source alongside the native KV-offload tiering counters."
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2775

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 The new PR #2775 entry is inserted mid-file (right after the #2730 entry, not at the true tail), and the diff also strips trailing whitespace from several unrelated pre-existing separator lines, both violating AGENTS.md's explicit 'append-only and byte-sensitive; preserve all existing bytes/separator whitespace; append only at the tail' rule for this file. Fix: move the new entry to the actual end of perf-changelog.yaml and revert the incidental whitespace edits on unrelated entries so only the tail changes.

Extended reasoning...

AGENTS.md line 22 states perf-changelog.yaml is append-only and byte-sensitive, with new entries appended only at the tail and all existing bytes preserved. This diff inserts the #2775 block between the pre-existing #2730 and #2781 entries (lines 6580-6591) instead of after the final #2763 entry, and also removes trailing whitespace on blank separator lines under PRs #1195, #1209, #1387, #2781, #2737, #2640 that were untouched otherwise. Any tooling or reviewers relying on the append-only/byte-stability guarantee (e.g. diffing or tailing the file for the latest change) will see unexpected mid-file mutation and stripped bytes that base branch never had.

Verification: normal — violates AGENTS.md line 22's non-negotiable invariant ("perf-changelog.yaml is append-only and byte-sensitive. Preserve all existing bytes and separator whitespace, and append only at the tail"), a rule the base branch satisfied and this change breaks. Two distinct violations, both visible in the diff: 1. Mid-file insertion (not tail). Hunk @@ -6577,6 +6577,18 @@ inserts the new…

Comment thread configs/nvidia-master.yaml Outdated

minimaxm3-fp8-h100-vllm-agentic-mtp:
image: vllm/vllm-openai:v0.27.1
image: ttl.sh/cquil11-vllm-tier-d67b417bca-pr53087-20260901:24h

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 The h100 MiniMax-M3 AgentX entry now points at an ephemeral ttl.sh tag (ttl.sh/cquil11-vllm-tier-d67b417bca-pr53087-20260901:24h) with an explicit 24h TTL, inside nvidia-master.yaml which is meant to describe a durable, re-runnable benchmark config, unlike the released vllm/vllm-openai:vX.Y.Z tags used elsewhere. Fix: only merge master-config image bumps that reference a persisted, non-expiring registry tag; keep PR-branch/ttl.sh images out of nvidia-master.yaml until a released image exists, or repoint to the final tag before merge.

Extended reasoning...

ttl.sh images expire after their TTL (here 24h) and become unpullable 404s afterward. Because this is committed into the canonical nvidia-master.yaml (not a scratch/test file), any CI re-run of the sweep, a matrix regeneration, or another engineer re-triggering this exact benchmark config more than a day after merge will fail at container pull with no indication in the yaml that the image is temporary, unlike every other entry in the file which references a stable released tag.

Verification: normal. At configs/nvidia-master.yaml:7344 the diff changes the minimaxm3-fp8-h100-vllm-agentic-mtp entry from a durable released tag to an ephemeral ttl.sh tag: - image: vllm/vllm-openai:v0.27.1 / + image: ttl.sh/cquil11-vllm-tier-d67b417bca-pr53087-20260901:24h. ttl.sh is an anonymous ephemeral registry that deletes images after the TTL encoded in the tag; :24h means the image is…

Comment on lines +309 to +320
NVME_CONTAINER_MOUNT=""
if [[ "${KV_OFFLOADING:-none}" == "nvme" || "${KV_OFFLOADING:-none}" == "dram+nvme" ]]; then
NVME_HOST_ROOT="/mnt/numa0/enroot/cache/group-$(id -g)"
NVME_HOST_DIR="$NVME_HOST_ROOT/inferencex-kv-$JOB_ID"
srun --jobid="$JOB_ID" bash -c "
set -e
test -w '$NVME_HOST_ROOT'
mkdir -m 700 '$NVME_HOST_DIR'
findmnt -T '$NVME_HOST_DIR'
"
NVME_CONTAINER_MOUNT=",$NVME_HOST_DIR:/kv-offload"
export NVME_OFFLOAD_DIR=/kv-offload

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 The new nvme/dram+nvme branch creates a job-scoped NVMe cache directory (/mnt/numa0/enroot/cache/group-$(id -g)/inferencex-kv-$JOB_ID) and mounts it into the container, but nothing ever removes it — the cleanup_allocation trap only cancels the Slurm job, it never rm -rf's NVME_HOST_DIR. Fix: have cleanup_allocation (or an added trap step) delete NVME_HOST_DIR when it was created, so it also runs after cache.bin writes for both new tiers, not just on the happy path.

Extended reasoning...

Every sweep run using kv-offloading=nvme or dram+nvme allocates a unique per-JOB_ID directory on the shared host NVMe and writes disk_capacity_bytes-sized cache.bin/tiering files into it (up to ~1TB per TP rank), but cleanup_allocation() at lines 296-303 only calls scancel on EXIT/INT/TERM and never touches NVME_HOST_DIR. Unlike the pre-existing dram/none modes which never wrote to local NVMe, repeated CI sweep invocations now leave orphaned multi-hundred-GB directories under /mnt/numa0/enroot/cache/group-*/ that accumulate indefinitely across runs, eventually exhausting shared NVMe capacity on the H100 cluster.

Verification: Severity: normal — a resource leak this change introduces that the base branch does not have. The nvme/dram+nvme path creates a per-job directory on the compute node's local NVMe and mounts it into the container as the KV-offload target, but nothing ever deletes it: - runners/launch_h100-dgxc-slurm.sh:311-318 (in the diff) creates it via srun --jobid:… | normal (resource leak on shared host…

Comment on lines +113 to +114
TOTAL_CPU_DRAM_GIB=$((TOTAL_CPU_DRAM_GB * 1000000000 / 1073741824))
PER_RANK_GIB=$(((TOTAL_CPU_DRAM_GIB - MODEL_CHECKPOINT_PAGE_CACHE_GIB) / TP - MODEL_CPU_OFFLOAD_GB - MOONCAKE_LOCAL_BUFFER_GIB))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 (optional) The new dram+nvme branch copy-pastes the Mooncake DRAM budget formula and still subtracts MOONCAKE_LOCAL_BUFFER_GIB (4 GiB/rank) even though this path uses OffloadingConnector/TieringOffloadingSpec, which never starts Mooncake or allocates that buffer, so the derived per-rank DRAM tier is needlessly smaller than the base branch would give a Mooncake-only run, and can trip the 'CPU DRAM budget is too small' exit at configs the actual budget could otherwise support. Fix: only subtract MOONCAKE_LOCAL_BUFFER_GIB when the connector actually uses Mooncake; for the dram+nvme/OffloadingConnector path compute PER_RANK_GIB from TOTAL_CPU_DRAM_GIB, MODEL_CHECKPOINT_PAGE_CACHE_GIB, and MODEL_CPU_OFFLOAD_GB only.

Extended reasoning...

Line 114 reuses the exact expression from the dram/Mooncake branch (line 65) verbatim, including '- MOONCAKE_LOCAL_BUFFER_GIB', inside the elif KV_OFFLOADING=dram+nvme block that instead builds an OffloadingConnector kv-transfer-config (lines 120-123) which has no Mooncake component and never sets MOONCAKE_CONFIG_PATH or starts mooncake_master. The 4 GiB/rank/TP is therefore withheld from the DRAM tier for no reason tied to this connector, and at the lower end of the DRAM budget this extra deduction can push PER_RANK_GIB to <=0, aborting the run (line 115-118) at a TOTAL_CPU_DRAM_GB value that would otherwise succeed.

Verification: nit. Factually real but negligible impact. Line 114 (PER_RANK_GIB=$(((TOTAL_CPU_DRAM_GIB - MODEL_CHECKPOINT_PAGE_CACHE_GIB) / TP - MODEL_CPU_OFFLOAD_GB - MOONCAKE_LOCAL_BUFFER_GIB))) in the new dram+nvme branch copies line 65's Mooncake formula verbatim, still subtracting MOONCAKE_LOCAL_BUFFER_GIB=4 (set line 59). That branch builds an OffloadingConnector/TieringOffloadingSpec config…

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

1 similar comment
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

1 participant