fix(deepseek): fence resident decode-metadata refreshes with a verified readback - #196
fix(deepseek): fence resident decode-metadata refreshes with a verified readback#196ndleslx wants to merge 1 commit into
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ed readback hw-native-sys#144 made the per-(ping-pong slot, rank) decode metadata device-resident behind a dirty key, uploading changed shards with copy_to and ordering those uploads only against the *previous* dispatch via the predecessor protocol. Nothing ordered an enqueued H2D copy against the *next* dispatch that reads the shards, so when the copy lost that race the fused kernel computed one step against the previous content of the slot's page mappings: a plausible-but-wrong token with no error signal. The prefix-cache accuracy guard caught it intermittently -- its second request forces a key change on both slots at once (copy-on-write detaches the shared rolling page), and it compares the two runs' completions token-for-token (main run 32468071303: ' a leading global provider of information ...' vs ' a leading global information ... (ICT)'). Two edges closed: - Verified readback fence: after uploading a rank's shards, read them back (copy_from blocks on data arrival) and compare bytes against the staged source; mark the device key clean only on a match. Correct regardless of how the device orders control copies against program execution. A mismatch re-uploads; three failures stop the job. The steady-state path (clean keys) is untouched -- the fence runs only on dirty ranks, so the decode-loop win hw-native-sys#144 measured is kept. - Per-slot predecessor: the refreshed shards are read by this slot's own previous dispatch, which under concurrent dispatch frames may outlive the newest dispatch the global predecessor tracks. wait() is idempotent, so waiting the slot's dispatch too is free. The verify scratch is a pre-fork shared buffer sized to the largest per-rank metadata shard, allocated with the sources it verifies. Unit tests model an async copy worker whose H2D lands only when a D2H drains it, including a readback that loses the race (retry then clean) and one that never lands (fail loudly, key stays dirty).
6f7516f to
e49b81e
Compare
|
Status: this does not fix the failure — hypothesis falsified by this run. The guard failed again with this fence active
Together with Recommendation: merge #186 (the revert, green) to unbreak main and treat this |
1 participant
Fixes the intermittent
test_deepseek_v4_http_completion_matches_expected_text[k1-prefix-cache]failureintroduced by #144 — an alternative to reverting #144 (#186): keep the residency
win, close the ordering hole.
Root cause
#144 keeps the per-(ping-pong slot, rank) decode metadata device-resident behind
a dirty key, uploading changed shards with
copy_toordered against theprevious dispatch (the predecessor protocol). Nothing orders an enqueued H2D
copy against the next dispatch that reads those shards — the consuming
dispatch is submitted microseconds later with no device-side dependency on the
copy. When the copy loses that race, the fused kernel computes one step against
the previous content of the slot's page mappings: a plausible-but-wrong token
with no error signal.
Evidence — main run 32468071303
(the failing #144 merge),
k1-prefix-cachecase, which runs the same prompttwice and compares the completions:
a leading global information and communications technology (ICT)a leading global **provider of** information and communications technology (A single wrong token at step ~4 of run 1, then a coherent continuation — one
step computed on stale mappings, not cascading corruption. The prefix-cache case
is the exposed one because its second request forces a key change on both slots
at once (copy-on-write detaches the shared rolling page, so the tail block ID
changes exactly where a fresh dispatch follows the re-upload), and because it
compares two identical generations token-for-token. The same race exists in
k1-fusedat slot 1's first use; it just usually wins. Intermittency isqueue-timing: #144's own PR run, #174, and #191's merge run all passed with the
same code.
The fix
Two edges closed in
_sync_decode_device_metadata_rank:(
copy_fromblocks on data arrival) and compare bytes against the stagedsource; the device key is marked clean only on a match. This is correct
regardless of how the device orders control copies against program execution
— no assumption about copy/run channel topology. A mismatch re-uploads (up to
3 attempts); a device that keeps contradicting its own readback fails the job
loudly instead of computing on stale page tables.
previous dispatch, which under concurrent dispatch frames may outlive the
newest dispatch the global predecessor tracks. The submit path now records
the per-slot dispatch;
wait()is idempotent so waiting it too is free.The verify scratch is one pre-fork shared buffer sized to the largest per-rank
metadata shard, allocated next to the sources it verifies (a few KB; syncs are
serialized by the control lock).
Steady state unchanged. The fence runs only on dirty ranks — key changes
(request boundaries, page crossings) — so #144's measured decode-loop win
(38.8 → 38.0 ms trimmed mean) is kept where it was measured; dirty steps pay a
KB-scale readback.
Tests
The metadata tests' fake worker now models the actual hazard:
copy_toenqueueswithout landing, and only a
copy_fromdrains the queue (per worker, FIFO), witha knob for a readback that loses the race:
fence per dirty rank, including the per-slot predecessor wait;
re-uploaded, and the key marked clean only after a matching readback — a
repeated sync with the same key stays a no-op;
dirty.
Full local suite: 208/208 unit tests, ruff, headers, english-only clean.
If this lands and the guards stay green, #186 (the revert) should be closed as
superseded.