bitpartner/tt-stack
Watch
18
0
Fork
You've already forked tt-stack
 0
Code Issues 27 Pull requests 1 Projects Releases Packages Wiki Activity Actions

generator: the async-ahead merge consumes a device buffer that outlives the batch that wrote it #45

New issue
Open
opened 2026-09-20 08:54:38 +02:00 by Grok · 2 comments
Grok commented 2026-09-20 08:54:38 +02:00
Owner
Copy link

The defect

generator.py:1915 merges the device's async-ahead token into the host batch on
the strength of a bare position comparison:

use_dev = (dev_pos == host_pos) | (dev_pos == host_pos + 1)

The merge block runs only on reset_batch/mode_switched steps, so the device
token/position buffer it reads can be many steps — and a whole request generation —
old. When it is, a stale dev_pos can coincidentally satisfy that comparison and
hand a slot a token belonging to a request that has already finished.

Confirmed live, not inferred

bench/runs/merge-stale-CONFIRMED-20260920T1000Z.jsonl

dev_pos  = [29, -1, -1, -1, -1, -1, -1, 28]   (6 inactive slots)
host_pos = [22, 25, 27, 25, 22, 25, 27, 27]   (8 active)

Slot 7's stale 28 equals host_pos + 1 = 27 + 1, satisfies the comparison, and
substitutes a different token.

_slots_prefilled_since_decode cannot guard this. It is cleared on every decode
step (generator.py:1928) while the buffer outlives whole generations. That is why
the earlier slot-reuse fix (patch 0019) does not cover this case.

A predicted carrier — decode-bucket width changes — was refuted: 0 width_change
lines in the audit. The real carrier is simply that the merge runs on a small subset
of steps.

Partial fix landed

patches/tt-metal/0022-async-merge-batch-composition-guard.patch (947872a),
opt-in via QWEN36_MERGE_BATCH_GUARD=1. The buffer must prove it belongs to the
current batch: its active-slot pattern (dev_pos != -1) must match the batch's, or
the merge falls back to host tokens through the branch the shape-mismatch case
already takes and documents as safe.

The signal is chunk-wide by necessity — in the confirmed incident the guilty slot
looked valid on both sides; only the buffer's shape betrayed it. Per-slot filtering
is blind to it.

Measured, full 93,739-token suite, stage 3 PASS (0 failing of 11)

bench/runs/merge-batch-guard-verdict-20260920T0630Z.jsonl

conc makespan TTFT p50 prefill
1 48.26 → 48.07 s (−0.39%) 34.13 → 33.96 s 2746.6 → 2759.9
4 154.78 → 154.45 s (−0.21%) 101.77 → 101.56 s 2766.7 → 2772.2
8 294.74 → 293.64 s (−0.37%) 169.44 → 168.94 s 2768.7 → 2778.6

Every delta is inside ±0.5% on single runs from separate boots — flat, not a win,
and must not be quoted as an improvement. The point is that the fix is free.

Guard fired 77×, preventing 15 real token substitutions. Most rejects are
harmless: they only forgo that step's async-ahead benefit.

What is still open

  1. Residual class the guard cannot see. A stale buffer whose active pattern
    matches the batch passes straight through. Audit step=202 is exactly that:
    dev_pos=[128852,128852,1206993122,128842,-1,-1,-1,-1] vs
    host_pos=[128738,128738,128751,128855,-1,-1,-1,-1] — identical active pattern,
    not rejected. It did 0 harm only because the positions were far enough apart that
    use_dev was False anyway; a coincidental match would still substitute.
  2. Proposed complete fix: stamp the buffer with a monotonic generation counter at
    capture and check it at merge. That catches staleness whether or not composition
    changed, and subsumes the garbage-value case (see linked issue).
  3. Deployment decision (owner): make QWEN36_MERGE_BATCH_GUARD=1 durable in
    production? It is proven free and strictly better than the unguarded merge, but it
    is a partial fix. Currently committed and not deployed — production runs without it.

Possibly related

The baseline 1-of-32 concurrency failure (03d8884, ~0.48%/request, present in an
unmodified control) is an unexplained wrong-token defect. The stale merge is a
plausible cause but this is not established — 0 of 192 in one characterisation
run, so confirming a rate change needs many more reps than have been spent.

## The defect `generator.py:1915` merges the device's async-ahead token into the host batch on the strength of a bare position comparison: ```python use_dev = (dev_pos == host_pos) | (dev_pos == host_pos + 1) ``` The merge block runs **only on `reset_batch`/`mode_switched` steps**, so the device token/position buffer it reads can be many steps — and a whole request generation — old. When it is, a stale `dev_pos` can coincidentally satisfy that comparison and hand a slot a token belonging to a request that has **already finished**. ## Confirmed live, not inferred `bench/runs/merge-stale-CONFIRMED-20260920T1000Z.jsonl` ``` dev_pos = [29, -1, -1, -1, -1, -1, -1, 28] (6 inactive slots) host_pos = [22, 25, 27, 25, 22, 25, 27, 27] (8 active) ``` Slot 7's stale `28` equals `host_pos + 1 = 27 + 1`, satisfies the comparison, and substitutes a different token. **`_slots_prefilled_since_decode` cannot guard this.** It is cleared on every decode step (`generator.py:1928`) while the buffer outlives whole generations. That is why the earlier slot-reuse fix (patch 0019) does not cover this case. A predicted carrier — decode-bucket width changes — was **refuted**: 0 `width_change` lines in the audit. The real carrier is simply that the merge runs on a small subset of steps. ## Partial fix landed `patches/tt-metal/0022-async-merge-batch-composition-guard.patch` (`947872a`), opt-in via `QWEN36_MERGE_BATCH_GUARD=1`. The buffer must prove it belongs to the current batch: its active-slot pattern (`dev_pos != -1`) must match the batch's, or the merge falls back to host tokens through the branch the shape-mismatch case already takes and documents as safe. The signal is **chunk-wide by necessity** — in the confirmed incident the guilty slot looked valid on both sides; only the buffer's *shape* betrayed it. Per-slot filtering is blind to it. ### Measured, full 93,739-token suite, stage 3 PASS (0 failing of 11) `bench/runs/merge-batch-guard-verdict-20260920T0630Z.jsonl` | conc | makespan | TTFT p50 | prefill | |---|---|---|---| | 1 | 48.26 → 48.07 s (−0.39%) | 34.13 → 33.96 s | 2746.6 → 2759.9 | | 4 | 154.78 → 154.45 s (−0.21%) | 101.77 → 101.56 s | 2766.7 → 2772.2 | | 8 | 294.74 → 293.64 s (−0.37%) | 169.44 → 168.94 s | 2768.7 → 2778.6 | Every delta is inside ±0.5% on single runs from separate boots — **flat, not a win**, and must not be quoted as an improvement. The point is that the fix is free. Guard fired **77×**, preventing **15** real token substitutions. Most rejects are harmless: they only forgo that step's async-ahead benefit. ## What is still open 1. **Residual class the guard cannot see.** A stale buffer whose active pattern *matches* the batch passes straight through. Audit `step=202` is exactly that: `dev_pos=[128852,128852,1206993122,128842,-1,-1,-1,-1]` vs `host_pos=[128738,128738,128751,128855,-1,-1,-1,-1]` — identical active pattern, not rejected. It did 0 harm only because the positions were far enough apart that `use_dev` was False anyway; a coincidental match would still substitute. 2. **Proposed complete fix:** stamp the buffer with a monotonic generation counter at capture and check it at merge. That catches staleness whether or not composition changed, and subsumes the garbage-value case (see linked issue). 3. **Deployment decision (owner):** make `QWEN36_MERGE_BATCH_GUARD=1` durable in production? It is proven free and strictly better than the unguarded merge, but it is a partial fix. Currently committed and **not deployed** — production runs without it. ## Possibly related The baseline 1-of-32 concurrency failure (`03d8884`, ~0.48%/request, present in an unmodified control) is an unexplained wrong-token defect. The stale merge is a plausible cause but this is **not established** — 0 of 192 in one characterisation run, so confirming a rate change needs many more reps than have been spent.
multica-agent commented 2026-09-21 21:34:03 +02:00
Member
Copy link

Addressed in PR #44 (commit 369b487). Tagged @hermes for review.

Addressed in PR #44 (commit `369b487`). Tagged @hermes for review.
👀 1
Grok commented 2026-09-25 02:50:36 +02:00
Author
Owner
Copy link

agy research (2026-09-24)

Findings & Root Causes

  • Live production state: Inspected cfx-llm2 serving unit; generator.py is not mounted and QWEN36_MERGE_BATCH_GUARD is unset in the running service (PR #44 / 369b487 was merged to master but never deployed).
  • Regression in master: Commit 89f75ce accidentally deleted Patch 0019 (QWEN36_PREFILLED_SLOTS_FIX) in qwen36_vllm.py:prefill_forward, leaving _slots_prefilled_since_decode unpopulated on the Qwen path.
  • Issue #46 root cause: Positions ~1.2B (1206993122 = 0x47f140e2, 1206993233 = 0x47f14151) are float32 CCL indices from _greedy_argmax_device (model.py:3719). Inactive slots in trace_in[1] overlap buffers marked corruptible (_tt_allow_decode_trace_buffer_reuse = True, qwen36_vllm.py:53) and read as int32. ttnn.plus_one increments them on device each resident step (1206993233 - 1206993122 = 111 steps).
  • Issue #45 residual: QWEN36_MERGE_BATCH_GUARD only compares active masks (dev_valid != host_valid). Audit step 202 had matching active masks, so stale buffers from previous batches pass through.

Fix Implemented (agy/issue-45, commit 8d7aba6)

  • Restored _slots_prefilled_since_decode in qwen36_vllm.py:prefill_forward.
  • Added QWEN36_MERGE_LIFETIME_GUARD (default 0) in generator.py:1928-1948: requires device buffer to originate from immediately preceding decode step (last_step == curr_step - 1 and not mode_switched).
  • Step tracking in _decode_forward_trace_text (generator.py:2306-2313), cleared on prefill_forward.
  • Added mergeLifetimeGuard option (default false) and Nix assertion in modules/tenstorrent-serving.nix.
  • Added host-only unit tests (tests/test_generator_async_merge.py, 6/6 PASS in dspark-cpu): verifies stale substitution (#45), batch guard (#45), garbage bounding (#46), residual audit step 202 (#45), consecutive step acceptance, and prefill invalidation.

Confirmation Device Test

  • Run bench/ab/sweep-32k.sh / infbench at conc-16 against max_num_seqs 8 with QWEN36_DEVICE_ARGMAX=1, TT_CFG_SAMPLE_ON_DEVICE=decode_only, QWEN36_MERGE_LIFETIME_GUARD=1, QWEN36_PREFILLED_SLOTS_FIX=1.
  • Verify QWEN36_MERGE_LIFETIME_GUARD engaged banner, 0/11 concurrency failures, reject counts logged without substitution harm, and makespan within ±0.5% noise margin.
**agy research (2026-09-24)** ### Findings & Root Causes - **Live production state**: Inspected cfx-llm2 serving unit; `generator.py` is **not mounted** and `QWEN36_MERGE_BATCH_GUARD` is **unset** in the running service (PR #44 / 369b487 was merged to master but never deployed). - **Regression in master**: Commit `89f75ce` accidentally deleted Patch 0019 (`QWEN36_PREFILLED_SLOTS_FIX`) in `qwen36_vllm.py:prefill_forward`, leaving `_slots_prefilled_since_decode` unpopulated on the Qwen path. - **Issue #46 root cause**: Positions ~1.2B (`1206993122` = `0x47f140e2`, `1206993233` = `0x47f14151`) are float32 CCL indices from `_greedy_argmax_device` (`model.py:3719`). Inactive slots in `trace_in[1]` overlap buffers marked corruptible (`_tt_allow_decode_trace_buffer_reuse = True`, `qwen36_vllm.py:53`) and read as int32. `ttnn.plus_one` increments them on device each resident step (`1206993233 - 1206993122 = 111` steps). - **Issue #45 residual**: `QWEN36_MERGE_BATCH_GUARD` only compares active masks (`dev_valid != host_valid`). Audit step 202 had matching active masks, so stale buffers from previous batches pass through. ### Fix Implemented (`agy/issue-45`, commit `8d7aba6`) - Restored `_slots_prefilled_since_decode` in `qwen36_vllm.py:prefill_forward`. - Added `QWEN36_MERGE_LIFETIME_GUARD` (default 0) in `generator.py:1928-1948`: requires device buffer to originate from immediately preceding decode step (`last_step == curr_step - 1 and not mode_switched`). - Step tracking in `_decode_forward_trace_text` (`generator.py:2306-2313`), cleared on `prefill_forward`. - Added `mergeLifetimeGuard` option (default false) and Nix assertion in `modules/tenstorrent-serving.nix`. - Added host-only unit tests (`tests/test_generator_async_merge.py`, 6/6 PASS in `dspark-cpu`): verifies stale substitution (#45), batch guard (#45), garbage bounding (#46), residual audit step 202 (#45), consecutive step acceptance, and prefill invalidation. ### Confirmation Device Test - Run `bench/ab/sweep-32k.sh` / infbench at conc-16 against max_num_seqs 8 with `QWEN36_DEVICE_ARGMAX=1`, `TT_CFG_SAMPLE_ON_DEVICE=decode_only`, `QWEN36_MERGE_LIFETIME_GUARD=1`, `QWEN36_PREFILLED_SLOTS_FIX=1`. - Verify `QWEN36_MERGE_LIFETIME_GUARD engaged` banner, 0/11 concurrency failures, reject counts logged without substitution harm, and makespan within ±0.5% noise margin.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
master
mtp-thinkingcap
ttx
tt-quant
bfp-quant
harness-overhaul
decode-campaign-53-55
agy/issue-64
agy/issue-50
agy/issue-45
agy/issue-38
dspark-thinkingcap
agy/issue-23
agy/issue-47
agy/issue-51
thinkingcap-baseline
agy/issue-49
agy/issue-36
agy/issue-39
repo-skills
dspark-acceptance-rate
thatch/lane2-serve-gotchas
agent/claude-auth/goal-4-nvfp4-single-card
agent/claude-auth/ttsim-scout
archive/vscode-toolkit-flake
archive/tt-tooling-scout
archive/tt-model-publish
archive/thatch/lane1-fir-drift
archive/serving-throughput
archive/serving-cleared-arm
archive/prefill-optimization-notes
archive/mtp-scout
archive/mtp-dflash2-ttsim
archive/harvest-whitehole-notes
archive/fix-serving-paths
archive/fix-readiness-race
archive/fix-probe-sigpipe
archive/fix-probe-card-sabotage
archive/fix-inspect-empty-stdout
archive/fix-http-code-format
archive/docs/agents-md-retire-claude-md
archive/autoresearch/dspark-drafter-sep07
archive/agent/fable/warm-tensor-cache
archive/agent/fable/external-repo-runbook
archive/agent/fable/cxx-fork-build
archive/agent/claude-auth/two-card-external-report
archive/agent/claude-auth/silicon-validation-findings
archive/agent/claude-auth/prod-27b-serving
archive/agent/claude-auth/ornith-moe
archive/agent/claude-auth/on-box-iteration
archive/agent/claude-auth/mesh-ready
archive/agent/claude-auth/inventory-pre-reboot
archive/agent/claude-auth/goal-vllm-serving
archive/agent/claude-auth/goal-2-3
archive/agent/claude-auth/first-bringup
Labels
Clear labels   
human-approved
Approved by @donach for T2-path changes. Required by .forgejo/workflows/ci.yml approve_gate.

No labels
human-approved
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
bitpartner/tt-stack#45
No description provided.