Skip to content

misc: fix the req_to_token member change - #967

Merged
hnyls2002 merged 1 commit into
mainfrom
fix
Aug 7, 2024
Merged

hnyls2002 merged 1 commit into
mainfrom
fix

Conversation

@hnyls2002

Copy link
Copy Markdown
Collaborator

Thank you for your contribution, we really appreciate it. The following instructions will help improve your pull request and make it easier to receive feedback. If there are any items you don't understand, don't worry. Just submit the pull request and ask the maintainers for help.

Motivation

This may cause some error.

Modification

can_use_mem_size -> free_slots

Checklist

  1. Ensure pre-commit pre-commit run --all-files or other linting tools are used to fix potential lint issues.
  2. Confirm that modifications are covered by complete unit tests. If not, please add more unit tests for correctness.
  3. Modify documentation as needed, such as docstrings or example tutorials.

@hnyls2002
hnyls2002 enabled auto-merge (squash) August 7, 2024 08:51
@hnyls2002
hnyls2002 disabled auto-merge August 7, 2024 08:52
@hnyls2002
hnyls2002 merged commit a01ddd9 into main Aug 7, 2024
@hnyls2002
hnyls2002 deleted the fix branch August 7, 2024 08:52
@hnyls2002 hnyls2002 mentioned this pull request Aug 8, 2024
timethink pushed a commit to timethink/sglang that referenced this pull request Mar 9, 2025
qybnb pushed a commit to qybnb/sglang that referenced this pull request Jul 10, 2026
Adjust parameters of test cases to accommodate failed test cases
efschu pushed a commit to efschu/htsglang that referenced this pull request Aug 28, 2026
…te the group, and let the guard speak

sgl-project#965 -- THE CO-DERIVED MATCH TUPLE. `Req.init_next_round_input` reads the radix
tree ONCE and unpacks that single `match_result` into EIGHT attributes in one
tuple assignment: prefix_indices, last_node, last_host_node, best_match_node,
host_hit_length, swa_host_hit_length, mamba_host_hit_length,
mamba_branching_seqlen. One reading of one geometry wearing eight names.

`truncate_prefix_to` moves the prefix they were derived from. Its own docstring
says why that matters -- "the geometry was DERIVED from the prefix that just
moved, so it is now a stale reading rather than a report" -- and that sentence is
true, word for word, of every field in the tuple. It wrote prefix_indices, and
over two separate windows, each discovered by a boot, cache_protected_len (sgl-project#930)
and extend_range (sgl-project#958). Six were left standing. This is the complete sibling
sweep those two never got.

NOTHING RE-DERIVES IN BETWEEN. scheduler.py calls init_next_round_input (all
eight derived), both arms of the `pp_size > 1` fork call truncate_prefix_to, then
adder.add_one_req reads them. No match_prefix on that path.

WHAT THE STALE READINGS DO, at PrefillAdder.add_one_req:
  * `real_input_tokens = cand_extend_input_len - req.host_hit_length` subtracts a
    host hit no longer part of this prefix, under-counting against the budget;
  * `needs_host_load_back()` is still true, so init_load_back runs on a stale
    best_match_node/host_hit_length and does
    `prefix_indices = torch.cat([prefix_indices, new_indices])` -- leaving
    `[0, told)` then `[L_dev, L_dev+H)` with a HOLE between, while
    prepare_for_extend sizes the cross-stage tensor off len(prefix_indices) as
    though it were contiguous. The silently-wrong-context class;
  * that branch then sets `cache_protected_len = prefix_len`, re-raising the
    exact value the truncation lowered nine lines earlier -- undoing sgl-project#930.

FIXED AS ONE CONTRACT, not a seventh one-field patch, because two of the eight
were already bought one at a time by two boots. The ratchet
(`test_every_co_derived_field_is_accounted_for`) reads the producer tuples from
the AST -- not by grep, since a grep needs a name and the whole point is a field
nobody has thought of yet -- and FAILS when a ninth joins, telling the author to
decide TRUNCATED / CLEARED / HANDLE. Planted-ninth-field proof: it names
`newly_added_ninth_reading`.

`last_node` IS NOT NULLED, and that asymmetry is deliberate: it is not a reading
but a RESOURCE HANDLE holding an outstanding inc_lock_ref, and this attribute is
the only surviving reference to it. Nulling it would leak the ref and make the
node permanently unevictable -- a defect that already exists on the PP void path
(`_release_dynamic_chunk_probe` frees rows without cache_finished_req /
dec_lock_ref, unlike schedule_batch.py:2170-2175 and disaggregation/prefill.py
:1159-1161, which do). Registered, NOT fixed here.

A NO-OP TRUNCATION STILL INVALIDATES NOTHING. My first test asserted the
opposite; the invalidator's own documented rule -- "a no-op truncation leaves a
valid geometry valid, clearing it there would void healthy passes for nothing" --
is right, and clearing there would discard a host hit every rank still holds.
The test was inverted to pin the documented behaviour instead of contradicting it.

sgl-project#967 -- THE sgl-project#959 GUARD WAS UNMEASURABLE. It is closed by two bare
`return AddReqResult.OTHER` statements. A refusal that leaves no trace is
indistinguishable from a scheduler that simply built nothing, which is precisely
the state the sgl-project#963 investigation spent a boot separating by hand out of per-rank
coverage databases. Unconditional per-site counter plus a rate-limited
`[sgl-project#967] SECOND CONTINUATION REFUSED` line, following the neighbour that already
got this right (`_note_seam_chunk_refused`: first three, then every thousandth).
Module-level, because PrefillAdder is rebuilt every pass and an instance counter
would reset before anyone could read it.

THE TWO GUARDS ARE IN DIFFERENT METHODS -- add_one_req_ignore_eos:1692 and
add_one_req:1935 -- and my first wiring pin inspected only add_one_req, so it
measured nothing and failed on the UNMUTATED tree. That is how the mistake was
caught. Rewritten to walk the module AST, and it now names the offending line
number when a guard's counter is stripped (proof: "guard(s) at line(s) [1935]
return without counting"). The site labels are the method names.

CAN-FAIL, read rather than asserted:
  sgl-project#965 clearing withdrawn -> 8 failed, one per field, each naming its own field
  sgl-project#965 ninth field planted -> ratchet names it
  sgl-project#967 one guard's note stripped -> wiring pin names line 1935

NUMBERS, scripts/gate_tier2_partitioned.py, CVD="" :
  BEFORE 2 genuine (test_collective_family_siblings_610.py x2), 685.00 s
  AFTER  2 genuine (the same two), 580.49 s, 358 modules
  count probe 2 named == 2 summary, SUBFAILED and ERRORS included
  truncation family + prefill adder + schedule_policy suites: 54 passed
  ruff: schedule_batch.py 45 before / 45 after, schedule_policy.py 21 / 21,
        both new test files clean

TICKET_961_WINDOW.md gains section 8: the "race" of section 7 is NOT a race
(ServerArgs byte-identical, the LOAD ARM differed -- 8422 chunked vs 2822
sub-chunk, and an over-one-chunk prompt forces a chunked_req onto every rank, so
Arm A masks the defect), and the acceptance now counts flip EPOCHS and phase=tp
BATCHES SEPARATELY: 3+3 "both directions" was 3 RANKS confirming ONE epoch, and
no batch ever ran phase=tp, so every post-flip guard's green was vacuous.

NOT CLAIMED: no boot, no metal, no effect measurement.
efschu pushed a commit to efschu/htsglang that referenced this pull request Aug 28, 2026
…, not at the exits

Boot 8 died 25s into load: prepare_for_extend's sgl-project#965 invariant
(len(prefix_indices) == extend_range.start, asserted at
schedule_batch.py:3262) failed on PP0. R11 rooted the writer pair,
refuting both suspects (neither the sgl-project#944 told=0 terminator -- it took
its report-only branch -- nor the sgl-project#987 adopt, which moves only the
fill): the host load-back in PrefillAdder.add_one_req grows
prefix_indices in place and is followed by budget/align/sgl-project#967 early
returns that never reach set_extend_range; reachable because the
waiting_queue loop carried no membership guard, so a request already
resident in can_run_list (the void family's deliberately preserved
co-residency) was visited a second time, its prefix moved, and the
bail-out left the first visit's extend_range behind it.

Fix, the sgl-project#965 lesson (one junction, never per-branch bail patches):
re-derive Range(prefix, prefix) IMMEDIATELY at the mutation -- every
current and future early return inherits a consistent parked shape
automatically; the success paths overwrite it with the real range as
before. Plus the identity membership guard at the loop head (skip
census 'already_in_batch'; identity not rid -- two objects with one
rid are a different defect and must not be masked). Plus the sgl-project#988
LOADBACK instrument line (module-level, rate-limited).

Sibling class registered as ONE posten, not fifteen: 11 further
prefix_indices reassignments without same-function re-derive across
the cache stash paths (R11 sweep; they run where the next admission
re-derives, so not reachable as this crash).

Execution proof: import smoke green; the load-back ran on every
cache-hit admission in boot 8 (FILL-ADOPT and cached_pos=8 prove the
hit path hot); the guard sits in the loop that produced boot 8's
crash batch. Boot 8's collapse numbers stand: unexec 513 -> 10,
abort net 6x bounded, batches and cached at family bests -- this
closes the last named layer of that boot.
efschu pushed a commit to efschu/htsglang that referenced this pull request Aug 29, 2026
Boot 66's forensics: rid 6f9d8c6e, prefix_lens=0 fill_lens=3 out_lens=0, one
ADMIT, no retraction, no chunk -- a 3-token first-pass prefill, which is
literally the "Say OK." hand probe. It reached the DECODE graph runner with
raw_bs=1, was accepted, and died one frame later as
`input_ids: dst(1,) <- src(3,)`. Every short prompt does this, health checks
included, which is why every boot of this window died on its first real
request.

The predicate is the reason. Every verdict in can_run_graph reasons about
BATCH SIZE -- `cuda_graph_bs = forward_batch.batch_size`, then
`cuda_graph_bs <= self.max_bs` -- and none asks how many tokens each sequence
carries. The captured graph fixes that separately as num_tokens_per_bs, and
load_batch derives every buffer width from it. Right bs, wrong tokens per
sequence: accepted here, fatal there. The same axis confusion the runner has,
one level up.

So the guard rejects on the axis that was never read, and the batch runs eager
instead of failing in a copy.

WHAT THIS DELIBERATELY DOES NOT CLAIM: it is not the whole root. is_cuda_graph()
admits DECODE, TARGET_VERIFY, IDLE and DLLM_EXTEND, and which of those this
3-token prefill claims to be cannot be settled from the boot-66 log -- the mode
is not printed anywhere near the crash. Guessing it would be the half-chain fix
I stopped short of last round. The guard prints forward_mode, so the next boot
names it: DECODE means the classification is wrong upstream; an EXTEND mode
means is_cuda_graph() admits a mode this runner cannot serve. Then the root
gets cut on a reading instead of a hunch.

The line is not optional (sgl-project#967): a silent return False would leave it
unmeasurable whether this ever fires, and this one carries the measurement the
root still needs.

Evidence: desk, executed. py_compile; the guard is the first verdict in the
function, ahead of every existing branch, and emits once per refusal.
Belegstufe: DESK-BEWIESEN -- the metal proof is the probe returning tokens.
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.

1 participant