findings(p2p): request-finalization crash fix, capacity provenance, series 3 (missing affinity+P2P arm)

Three related investigations from the guide-testing/blog-review session:

- p2p-request-finalization-crash.md: vllm#49635 (bisected to #46284),
  both fix variants verified on this findings set's own crash
  reproducer plus fresh unit runs.
- capacity-number-provenance.md: traces why the guide report and blog
  disagreed on GPU KV capacity (0.48M / 1.38M / a fresh 1.22M and
  1.61M) - each number was genuinely measured, for a different config
  or vLLM nightly than the one it ended up describing, not a
  copy-paste error.
- RESULTS-3.md: the previously-untested epp-affinity-p2p arm, run on
  both the guide's flagship scenarios. It loses the concurrency/
  ownership-contention regime (document Q&A) to load-aware+P2P, and
  wins the capacity/replication regime (uniform pool) over it - no
  single arm wins both, extending series 1's own conclusion with the
  specific missing data point.

Signed-off-by: nilig <nili.ifergan@gmail.com>

Author: nilig

test/p2p-findings/RESULTS-3.md

@@ -0,0 +1,120 @@
+# P2P benchmarks, series 3: the missing affinity+P2P arm, and where it wins
+
+Series 1 ([RESULTS.md](RESULTS.md)) and the llm-d guide's own benchmark
+report both leave a gap: `epp-affinity-p2p` — precise prefix-cache
+affinity *plus* the pull, the guide's shipped default — was never
+isolated on the guide's own two flagship scenarios. Every published table
+compares affinity-alone or load-alone against load+P2P; the arm actually
+recommended by default was untested. This series fills that gap on both
+scenarios and finds they disagree with each other.
+
+Run on the guide's own manifests (`guides/p2p-kv-cache-sharing`, commit
+`d4a5fb91`), the pinned nightly
+(`nightly-4080263bb2c5d10deac17aaeb88e0823bc35bca9`) with the
+`generic_p2p` overlay (`145a460c`) plus the request-finalization crash
+fix (`orozery/vllm@fa07027d` — see
+[p2p-request-finalization-crash.md](p2p-request-finalization-crash.md))
+overlaid via ConfigMap. Fix overlay:
+[configs/2026-07-capacity-and-scenarios/patch-p2p-fix-scaffolding.yaml](configs/2026-07-capacity-and-scenarios/patch-p2p-fix-scaffolding.yaml),
+fix source:
+[configs/2026-07-capacity-and-scenarios/vllm-fix-orozery-scheduler.py](configs/2026-07-capacity-and-scenarios/vllm-fix-orozery-scheduler.py).
+Every deployment verified in-pod before running load: pinned image
+digest, `finished_signaled` present in the mounted `scheduler.py` (fix
+live), `remote_kv_source` present in the mounted `manager.py` (branch
+overlay live).
+
+## Scenario D: document Q&A (14 pods, session-ownership regime)
+
+Guide's own headline workload
+([guide_p2p-kv-cache-sharing_1.yaml.in](configs/2026-07-capacity-and-scenarios/guide_p2p-kv-cache-sharing_1.yaml.in) —
+also lives at `llm-d-benchmark#1656`): 192 conversations, private 48K-token
+document prefix, 6 turns of 256-token questions/answers, 128 concurrent.
+Two independent runs of the new arm (order-alternation doesn't apply with
+only one non-baseline arm running).
+
+| run | affinity (no pull) | **affinity + P2P** | load + P2P |
+|---|---|---|---|
+| 1 | 4.1 / 41.0 / 80.5s; 5.98 turns/s | 4.0 / 27.7 / 48.8s; 5.6 turns/s | 4.5 / 13.0 / 20.9s; 7.02 turns/s |
+| 2 | 4.2 / 17.3 / 37.2s; 7.66 turns/s | 2.6 / 33.6 / 67.2s; 5.7 turns/s | 3.9 / 12.5 / 26.7s; 7.76 turns/s |
+
+(TTFT p50/p95/p99; throughput. affinity+P2P's throughput is slightly
+undercounted by a sub-1% tail of gpt-oss requests that generated
+unusually long reasoning output under `ignore_eos:true` — median/p90/p95
+output length all land on the intended 256-token target, only `p99.9`
+blows out. TTFT is unaffected, measured at first token before that
+generation happens.)
+
+**affinity+P2P sits between the other two arms, in both runs, not one.**
+It improves on affinity-alone's worst case (48.8-67.2s p99 vs 80.5s) but
+never reaches load+P2P's range (20.9-26.7s p99). Likely mechanism:
+affinity+P2P still queues a request behind a busy owner pod whenever the
+scorer's affinity term outweighs load, and only pulls once a peer already
+out-caches the scheduled pod — it recovers the *cache-locality* cost of a
+cross-pod placement but not the *queueing* cost of a placement decision
+that still prefers a busy owner. load+P2P never makes that tradeoff:
+placement always goes to the least-loaded pod.
+
+Zero restarts, zero failures, 1,152/1,152 turns completed on both runs of
+the new arm (2,304 turns total).
+
+## Scenario A: uniform shared-prefix pool (16 pods, capacity regime)
+
+128 shared 48K-token prefixes, 256-token questions, 64-token outputs,
+constant-rate stages 4-24 req/s. Profile built for this series:
+[guide_p2p-kv-cache-sharing_2.yaml.in](configs/2026-07-capacity-and-scenarios/guide_p2p-kv-cache-sharing_2.yaml.in)
+(now also `llm-d-benchmark#1656`). Raw per-stage results:
+[configs/2026-07-capacity-and-scenarios/scenario-a-raw/](configs/2026-07-capacity-and-scenarios/scenario-a-raw/).
+
+| offered | affinity | **affinity + P2P** | load, no P2P | load + P2P |
+|---|---|---|---|---|
+| 4 req/s | 3.8 / 2.4s | 3.9 / 2.4s | 3.8 / 4.2s | 3.9 / 2.4s |
+| 8 req/s | 7.9 / 0.7s | 7.9 / 0.73s | 6.7 / 6.5s | 7.7 / 1.6s |
+| 12 req/s | 11.9 / 0.7s | 11.9 / 0.75s | 9.0 / 24.9s | 11.4 / 2.3s |
+| 16 req/s | 15.8 / 0.7s | 15.8 / 0.76s | 8.8 / 37.3s | 15.1 / 3.6s |
+| 20 req/s | 19.8 / 0.7s | 19.8 / 0.77s | 9.4 / 48.8s | 15.4 / 15.6s |
+| 24 req/s | 23.7 / 0.8s | 23.6 / 0.82s | 9.4 / 63.5s | 16.7 / 30.0s |
+
+(achieved req/s / request latency p50)
+
+**Here the result reverses.** affinity+P2P matches affinity's near-ideal
+ceiling within noise at every offered rate, tracking offered to
+saturation at 24 req/s. load+P2P — the arm that won Scenario D — degrades
+sharply at the top of the ladder: p50 30.0s at 24 req/s, achieving only
+16.7 of 24 offered. Likely mechanism: load-aware placement scatters each
+prefix's traffic across more pods than affinity does; at saturation that
+costs more in pull/transfer contention than affinity+P2P ever pays, since
+affinity+P2P never scatters a prefix's traffic in the first place — the
+pull mostly sits idle here (placement rarely diverges from cache), so it
+costs nothing to have it available for the rare miss.
+
+Zero restarts, zero failures, 5,040/5,040 requests.
+
+## No single arm wins both regimes
+
+Consistent with series 1's own cross-regime conclusion ("no single
+configuration wins both regimes") — this series supplies the specific
+data point that was missing: **affinity+P2P**, not just affinity-alone,
+loses the concurrency/ownership-contention regime (Scenario D) and wins
+the capacity/replication regime (Scenario A). The two regimes:
+
+- **Capacity-driven** (working set spread wider than any pod's cache,
+  traffic uniformly distributed across owners — Scenario A here, series
+  1's uniform pool, series 1's small-scale Llama pool): affinity-based
+  placement concentrates each prefix's traffic tightly enough that pulls
+  stay cheap and local; load-aware placement's scattering costs more at
+  saturation than it saves in balance.
+- **Concurrency/ownership-driven** (many concurrent sessions each pinned
+  to a fixed owner pod by a composite scorer, contention builds
+  independent of aggregate capacity — Scenario D here): load-aware
+  placement avoids the owner-pod queue by never preferring a busy owner;
+  affinity-based placement still pays that queueing cost even with the
+  pull covering the cache-locality half of it.
+
+The guide ships `epp-affinity-p2p` as the default (reverted back to it
+after this series' Scenario A result, having briefly flipped to
+`epp-load-p2p` on Scenario D's evidence alone) — the safer
+general-purpose choice: close to affinity's ceiling in the capacity-driven
+regime, and still a clear improvement over affinity-alone in the
+concurrency-driven one, even though it does not match load+P2P there.
+`epp-load-p2p` remains the documented recommendation specifically for
+workloads shaped like Scenario D.

test/p2p-findings/capacity-number-provenance.md

@@ -0,0 +1,90 @@
+# P2P findings — GPU KV capacity number provenance (guide report + blog)
+
+The published gpt-oss-120b capacity figures in the guide's benchmark
+report and the blog draft disagreed with each other and, in the P/D case,
+with a fresh measurement. Neither was a copy-paste error or careless
+mistake — each number was genuinely measured, but for a *different*
+config or vLLM nightly than the one it ended up describing. Tracing the
+actual provenance:
+
+## The three numbers and where each one is really from
+
+**~0.48M tokens/pod** — genuinely measured, but for a deliberately
+different config: `--gpu-memory-utilization=0.60` on the series-2
+"sessions exceed fleet GPU cache" scenario (`RESULTS.md` line ~235,
+`RESULTS-2.md` line 18), where the whole point was to force eviction
+under a smaller GPU budget. It was never the number for the guide's
+actual shipped aggregated config (`--gpu-memory-utilization=0.85`) — it
+got carried into `benchmark-results/gpt-oss-120b-h200.md` and the blog's
+Use Case 1 table as if it were.
+
+**~1.38M tokens/pod** — also genuinely measured, at `0.85`/`65536` (the
+guide's real aggregated flags), but on an earlier vLLM nightly than the
+one now pinned (`RESULTS.md` line 48, no specific SHA recorded). A fresh
+read on the current pin (`nightly-4080263bb2c5d10deac17aaeb88e0823bc35bca9`,
+same `0.85`/`65536` flags, two independent pod starts) gives
+**1,218,315 tokens** instead — nightly-to-nightly drift in the memory
+profiler/CUDA-graph accounting, not a config difference. The 1.38M figure
+was then reused *again*, by reference rather than measurement, for the
+P/D topology's CPU-tier sizing rationale in Run L
+(`RESULTS-2.md` line 290: "128 GiB CPU tier both roles (~2.3x the
+~1.38M-token GPU KV at TP=1)") — the P/D topology's GPU KV was never
+independently measured. The blog's Use Case 3 table copied Run L's
+numbers directly, inheriting the un-verified-for-P/D figure.
+
+**1,607,392 tokens/pod (P/D, fresh measurement, this session)** — a
+minimal 2-pod probe (1 prefill + 1 decode, TP=1 both legs, gpt-oss-120b,
+`MultiConnector[NixlConnector + OffloadingConnector]`, current pinned
+nightly, "as the pd-disaggregation guide ships" — no explicit
+`--gpu-memory-utilization` or `--max-model-len` override, since neither
+`guides/pd-disaggregation/modelserver/gpu/vllm/base/patch-{prefill,decode}.yaml`
+nor the recipes base sets them). vLLM's own defaults apply instead:
+`--gpu-memory-utilization=0.92` (logged explicitly) and
+`--max-model-len=131072` (gpt-oss-120b's native context, vs the
+aggregated config's explicit 65536 cap). Both legs identical, as
+expected for matched TP=1/config. Manifest:
+[configs/2026-07-capacity-and-scenarios/pd-capacity-probe.yaml](configs/2026-07-capacity-and-scenarios/pd-capacity-probe.yaml).
+First attempt OOMKilled at `memory: 32Gi` (too small for the 88 GiB
+`cpu_bytes_to_use` shm-backed tier); the guide's own proven value is
+`memory: 160Gi` (`modelserver/gpu/vllm/patch-vllm.yaml`) — used that on
+retry, 0 restarts.
+
+## What this means for the published numbers
+
+Neither the guide's report nor the blog can currently cite a P/D GPU-KV
+figure with real provenance — the true number depends on which
+`--gpu-memory-utilization` the P/D deployment actually runs, which
+neither document states. Two ways to close this:
+
+1. Explicitly set `--gpu-memory-utilization=0.85` on the P/D legs (for
+   consistency with the aggregated config) and re-measure — expect
+   something below 1.61M, roughly `1.61M * 0.85/0.92 ≈ 1.49M` if the
+   relationship is close to linear, but this needs a real measurement,
+   not an extrapolation, before publishing.
+2. Leave the P/D legs at vLLM's defaults (as the guide currently ships
+   them) and publish 1.61M as the actual figure, documenting the
+   `--gpu-memory-utilization`/`--max-model-len` used so it's
+   reproducible.
+
+Either way, the aggregated topology's report and blog entries should be
+corrected to 1.22M (this session's twice-confirmed measurement on the
+current pin), and the P/D entry should stop being derived by reference
+to a number that was never independently checked for that topology.
+
+## Downstream corrections already made (guide only, not yet the blog)
+
+- `benchmark-results/gpt-oss-120b-h200.md` and `benchmarking/README.md`:
+  aggregated GPU KV corrected to ~1.22M, CPU:GPU ratio corrected to
+  ~1.8x (was reported as 4.4x using the wrong 0.48M denominator), and the
+  document-Q&A headline's "oversubscribes the fleet's aggregate GPU KV"
+  claim removed — at the corrected capacity the 9.2M-token corpus fits
+  inside the fleet's aggregate cache with room to spare, so the
+  mechanism driving the headline's displaced-request pattern is
+  per-pod queueing under concurrency, not capacity scarcity. See guide
+  commits `97c45027`, `d4a5fb91` on `guides/p2p-kv-cache-sharing`.
+- Blog (`llm-d.github.io`, `blog/2026-08-15_p2p-kv-cache-sharing-llm-d.md`):
+  Use Case 1's capacity figure and oversubscription framing were
+  corrected the same way (commit `cc1dd76`). Use Case 3's P/D figure and
+  the guide's placement-rule closing section were **not yet corrected**
+  as of this writing — pending a decision on how to handle the
+  unresolved P/D provenance above.

test/p2p-findings/configs/2026-07-capacity-and-scenarios/guide_p2p-kv-cache-sharing_1.yaml.in

@@ -0,0 +1,70 @@
+load:
+  type: concurrent
+  num_workers: 8
+  worker_max_concurrency: 1000
+  request_timeout: 180
+  stages:
+  # 192 conversations x 6 turns; 128 concurrent conversations in flight.
+  - num_requests: 1152
+    concurrency_level: 128
+api:
+  type: completion
+  streaming: true
+server:
+  type: vllm
+  model_name: REPLACE_ENV_LLMDBENCH_DEPLOY_CURRENT_MODEL
+  base_url: REPLACE_ENV_LLMDBENCH_HARNESS_STACK_ENDPOINT_URL
+  ignore_eos: true
+tokenizer:
+  pretrained_model_name_or_path: REPLACE_ENV_LLMDBENCH_DEPLOY_CURRENT_MODEL
+data:
+  type: conversation_replay
+  conversation_replay:
+    seed: 37
+    max_model_len: 65536
+    num_conversations: 192
+    shared_system_prompt_len: 256
+    # Each conversation carries a private ~48K-token "document" prefix;
+    # 6 short questions with short answers make TTFT dominate. What drives
+    # hit-vs-miss here is request placement under concurrency (128
+    # sessions against a limited set of owner pods), not corpus size
+    # versus aggregate GPU KV - scale num_conversations and concurrency_level
+    # relative to your fleet's pod count, not its aggregate cache.
+    dynamic_system_prompt_len:
+      type: lognormal
+      min: 49152
+      max: 49152
+      mean: 49152
+      std_dev: 1
+    turns_per_conversation:
+      type: lognormal
+      min: 6
+      max: 6
+      mean: 6
+      std_dev: 1
+    input_tokens_per_turn:
+      type: lognormal
+      min: 256
+      max: 256
+      mean: 256
+      std_dev: 1
+    output_tokens_per_turn:
+      type: lognormal
+      min: 256
+      max: 256
+      mean: 256
+      std_dev: 1
+    tool_call_latency_sec:
+      type: lognormal
+      min: 1
+      max: 2
+      mean: 1
+      std_dev: 1
+report:
+  request_lifecycle:
+    summary: true
+    per_stage: true
+    per_request: true
+storage:
+  local_storage:
+    path: /workspace

test/p2p-findings/configs/2026-07-capacity-and-scenarios/guide_p2p-kv-cache-sharing_2.yaml.in

@@ -0,0 +1,41 @@
+load:
+  type: constant
+  stages:
+  - rate: 4
+    duration: 60
+  - rate: 8
+    duration: 60
+  - rate: 12
+    duration: 60
+  - rate: 16
+    duration: 60
+  - rate: 20
+    duration: 60
+  - rate: 24
+    duration: 60
+api:
+  type: completion
+  streaming: true
+server:
+  type: vllm
+  model_name: REPLACE_ENV_LLMDBENCH_DEPLOY_CURRENT_MODEL
+  base_url: REPLACE_ENV_LLMDBENCH_HARNESS_STACK_ENDPOINT_URL
+  ignore_eos: true
+tokenizer:
+  pretrained_model_name_or_path: REPLACE_ENV_LLMDBENCH_DEPLOY_CURRENT_MODEL
+data:
+  type: shared_prefix
+  shared_prefix:
+    num_groups: 128                # 128 shared prefixes (Scenario A: uniform pool)
+    num_prompts_per_group: 32      # unique questions per shared prefix
+    system_prompt_len: 48000       # ~48K-token shared prefix per group
+    question_len: 256              # unique question length (tokens)
+    output_len: 64                 # target output length (tokens)
+report:
+  request_lifecycle:
+    summary: true
+    per_stage: true
+    per_request: true
+storage:
+  local_storage:
+    path: /workspace

test/p2p-findings/configs/2026-07-capacity-and-scenarios/patch-p2p-fix-scaffolding.yaml

@@ -0,0 +1,20 @@
+# TEST SCAFFOLDING (not for commit): overlays the request-finalization fix
+# (offloading connector scheduler) onto the engine. Currently Or's variant,
+# orozery/vllm@fa07027d, from the p2p-fix-or ConfigMap.
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: decode
+spec:
+  template:
+    spec:
+      containers:
+        - name: modelserver
+          volumeMounts:
+            - mountPath: /usr/local/lib/python3.12/dist-packages/vllm/distributed/kv_transfer/kv_connector/v1/offloading/scheduler.py
+              name: p2pfix
+              subPath: offloading_scheduler.py
+      volumes:
+        - name: p2pfix
+          configMap:
+            name: p2p-fix-or