Skip to content

misc: update test config - #990

Merged
zhyncs merged 1 commit into
sgl-project:mainfrom
zhyncs:tmp
Aug 10, 2024
Merged

zhyncs merged 1 commit into
sgl-project:mainfrom
zhyncs:tmp

Conversation

@zhyncs

@zhyncs zhyncs commented Aug 8, 2024

Copy link
Copy Markdown
Contributor

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

Please explain the motivation behind this PR and the goal you aim to achieve with it.

Modification

Briefly describe the changes made in this PR.

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.

@zhyncs

zhyncs commented Aug 9, 2024

Copy link
Copy Markdown
Contributor Author

Let's hold this PR for now, until I finish configuring more runners. Then we can consider switching to this configuration. Although it will take longer, with 2 runners, the time will be overlapped.

@zhyncs zhyncs changed the title [DO NOT MERGE] misc: update e2e test config misc: update test config Aug 10, 2024
@zhyncs
zhyncs merged commit e712837 into sgl-project:main Aug 10, 2024
@zhyncs
zhyncs deleted the tmp branch August 10, 2024 18:20
timethink pushed a commit to timethink/sglang that referenced this pull request Mar 9, 2025
hanwlax added a commit to hanwlax/sglang that referenced this pull request Jul 13, 2026
efschu pushed a commit to efschu/htsglang that referenced this pull request Aug 28, 2026
…eeps its one ref

Boot 9 died in 24s on the lock_ref>0 underflow (full_component.py:320,
via cache_unfinished_req's dec at the FIRST chunk stash). R12 closed the
pair on metal: ONE inc (schedule_policy.py:2021, transferred
stash-to-stash by unified_radix_cache.py:1354-1355) met TWO decs -- the
give-back at the void (slice line 270, lock_ref_returned=True) and the
stash's own dec. Both give-back sites (sgl-project#984 park, sgl-project#986 orphan-queue)
assert 're-admission takes a FRESH ref', which is FALSE for a carried
chunk: the chunked continuation holds ONE admission ref across all its
chunk passes (schedule_policy.py:1441/:1747/:2008, scheduler.py:9059).
The sgl-project#988 guard removed the accidental compensating inc of the second
waiting-queue visit, exposing the imbalance (that causal half is
desk-inferred; the pair itself is measured).

Fix, two lines at the ONE shared function: pp_give_back_admission_lock_ref
returns False for the request currently held as self.chunked_req --
ownership discriminated by identity; the sgl-project#986 orphan route keeps giving
back (its orphan has by definition LEFT the field). The assert is never
weakened.

Context (R12's design evaluation, register 20:xx): the user's re-entry
design is ALREADY shipped as sgl-project#856/W30-FIX-B but scoped by _live_reqs to
batch residents; extending it is measurement-gated on sgl-project#972/sgl-project#975 + a GDN
host anchor (ABSENT today) -- registered as posten, not smuggled in
here. Import smoke green; suites in the test-agent lane.
efschu pushed a commit to efschu/htsglang that referenced this pull request Aug 28, 2026
… and the mamba park fence

sgl-project#990's ownership guard is right in intent and Boot 10 confirmed it on metal
(no underflow). It discriminates the wrong thing: 'is still in the field right
now' instead of 'will keep the field'.

MEASURED at b27d7c2 by instrumenting the give-back's caller:
  member IS current chunked_req -> pp_queue_orphaned_chunked_req, guard fires,
      lock_ref stays 1, request sits in waiting_queue with its admission ref
  ordinary member               -> pp_park_voided_batch_member, guard does not
      fire, lock_ref -> 0
So in these two shapes the guard's only observed firing is the harmful one.
Cause is a statement order: the re-home runs BEFORE the field is overwritten
(:7909/:7913, :9261/:9265) and the comment there requires that order for
#968b -- 'Re-homed BEFORE the overwrite -- after it, the reference is already
gone'. Re-admission then takes a fresh ref: +1 per void cycle, prefix never
evictable, sgl-project#969's leak by a new route.

Recorded as unittest.expectedFailure, not inverted and not deleted. Inverting
would encode the leak as correct; deleting loses the only executable record.
When ownership is asked properly this becomes an UNEXPECTED SUCCESS, which is
a loud self-clearing signal to drop the marker. The suite stays green.

The rid-discriminator arm is withdrawn rather than shipped half-understood:
its shape is entangled with the re-home and needs its own study.

MAMBA (boot 10's next form, mamba_pool_idx=None at cache_unfinished_req):
measured that the void-PARK does NOT release the slot, and fenced that, plus
the park's prefix contract beside it so the two cannot be traded off. What
produced the None is NOT established and the file says so in an executable
arm rather than guessing between the two candidates -- the release route via
reset_for_retract, or a re-admission that never re-acquires. Guessing there is
exactly this window's recurring class.

One of my own arms cited the wrong function for the clearer docstring and went
red; the citation is corrected in place with a note, since an arm citing the
wrong function is that same class and was caught only by running it.

Test: 969 -> 16 defs, 15 passed 1 xfailed; 990b -> 4 defs, 4 passed.
efschu pushed a commit to efschu/htsglang that referenced this pull request Aug 28, 2026
…d acquired

Boot 10 (b27d7c2) died on PP0 at 19:58:17 with
`AttributeError: 'NoneType' object has no attribute 'unsqueeze'` at
mem_cache/unified_cache_components/mamba_component.py:1056, reached via
_pp_process_batch_result -> process_batch_result_prefill ->
maybe_cache_unfinished_req -> cache_unfinished_req, rid
e3d6a9ff11c5460cb115f266f2b187a3.

ROOT (class, not effect). The admission loop's revert site in
managers/scheduler.py frees `req.mamba_pool_idx` whenever a request is not
added to `can_run_list`, guarded by `not req.session`. That predicate is
upstream's PROXY for "nobody else already owned this slot" -- correct
upstream, where a session was the only carrier of a pre-existing slot. This
fork invalidated it: sgl-project#984 (void-park), #968b (rehome-on-displace) and sgl-project#971
(rehome-on-refusal) re-queue requests UNRESET, holding the live mamba slot
of a prefill batch that is still IN FLIGHT. The revert then hands a live
owner's slot back to the allocator and nulls the field; the in-flight
batch's result arrives and dereferences it.

The silent direction is the same defect: when the free lands outside the
in-flight window, HybridReqToTokenPool.alloc re-acquires a fresh slot with
mamba_needs_clear=True -- a ZEROED recurrent state in place of the
request's own (sgl-project#767's shape), with no crash and no log line. Both close at
the provenance test.

FIX, at the one junction. Stamp provenance where the slot is acquired
speculatively by an admission round's prefix match --
unified_cache_components/mamba_component.py (COW resume + host load-back),
mamba_radix_cache.py, hi_mamba_radix_cache.py (both siblings) -- and free
at the revert site only when that stamp is set. The stamp is cleared the
moment HybridReqToTokenPool.alloc takes the slot into a batch, and on every
release/reset path (free_mamba_cache, _rollback_alloc, release_kv_cache's
early branch, reset_for_retract, Req.__init__ default). This is sgl-project#984's own
"pages versus claim" doctrine applied to the third increment: the lock ref
(sgl-project#990) and the chunk (sgl-project#971) already carry provenance; the mamba slot was
the one that did not. It opens neither sgl-project#929 (deficit) nor sgl-project#924 (surplus) --
the stamp is one-shot and dies with the slot.

AFFIRMATIVE REPORTING. None of the five mamba free sites named a rid, which
is why the specimen could only be reached by elimination. The refused
give-back now logs rid + rejection result, so the next boot is the metal
proof of the root rather than a yes/no on the crash.

BACKSTOP, named as a defect. mamba_component.prepare_for_caching_req now
declines retention with a counted, rid-bearing warning instead of
dereferencing a None slot. This is NOT the fix and is stated as such in
place: it costs the step's retention and re-prefills the chunk, which the
standing no-double-prefill order forbids as a routine. It exists because a
crash here takes the whole instance while a decline takes one step.

SIBLING SWEEP: all five writers of `mamba_pool_idx = None` audited and the
stamp cleared at each. gdn_slot_runtime.py:136 is not wired on this build
(the gdn_resident_state_slots gate is off, no "armed" line in any boot log)
and is left untouched. streaming_session.py's clears stay covered by the
retained `not req.session` term.
FUTURE CHECK: a new producer of a pre-existing slot cannot reopen this
silently -- it either stamps at its acquire (and is given back correctly)
or it does not (and is kept), and the backstop's counter names it with a
rid if it manages to drop one anyway.

Evidence: desk. py_compile + import smoke on all seven touched files;
lifecycle probe (Req default False, reset_for_retract clears). Belegstufe:
DESK-BEWIESEN, not metal -- boot 11 is the test.
efschu pushed a commit to efschu/htsglang that referenced this pull request Aug 28, 2026
…y what it owns

Boot 18 (ecedf3f) is the specimen and it is the important one: plain
PP=3 with the flip PROVABLY off (`PHASE-POLICY arming` 0, `PHASE-FLIP DONE`
0, chain-breaker check NONE, `command not found` 0), 31 batches, then PP0
died on

  AssertionError: Double-free detected: slots not currently allocated:
  [60934, 60935, ... 60971+]

reached with no flip link in the chain at all: _event_loop_pp_body ->
get_next_batch_to_run -> get_new_batch_prefill -> check_hicache_events ->
drain_storage_control_queues -> _drain_release -> mem_pool_host.free.

CLASS, and it is the FOURTH instance today: many producers, one give-back,
no provenance. sgl-project#990 (lock_ref), sgl-project#991 (mamba slot), sgl-project#993 (req-pool row), now
the host region. `cc.host_mem_release_queue` has FIFTEEN producers -- eight
in unified_radix_cache.py, plus swa_component, mamba_component,
hiradix_cache (x3), hi_mamba_radix_cache (x2) and cache_controller itself --
and `_drain_release` freed whatever arrived, with no dedup and no ownership
test.

TWO CHANGES, and the first matters more than the second.

1. PROVENANCE AT THE ENQUEUE. `append_host_mem_release` stamps the caller's
   `module:lineno` per page into `host_release_provenance`. Taken from the
   caller frame rather than added as a parameter to all fifteen sites, so a
   producer added LATER is stamped without anyone remembering to. This is
   what makes the next 90-second boot identify the producer PAIR itself
   instead of us picking the likeliest of fifteen.

2. OWNERSHIP AT THE GIVE-BACK. `_drain_release` now frees the unique, owned
   subset and reports the rest with the site that queued it. Two shapes are
   covered: the same span queued twice, and a span queued again after it was
   already freed. Freeing an unallocated slot is never correct, so dropping
   those is not a heuristic -- it is the give-back doing what it always
   should have done.

WHY THE W35 GUARD DID NOT COVER IT, which is a coverage answer and not a
retrofit. `append_host_mem_release` already carries a sgl-project#719/W35 guard whose
own text says "freeing them against the current pool is the W35 double-free".
That guard routes by BINDING GENERATION -- it catches a producer naming
slots from the pool bound before a cutover. Two producers queueing the same
span WITHIN one generation are, to it, two ordinary current-generation
batches: structurally invisible, not merely missed. The guard names the
hazard, not the gap.

SCOPE, stated plainly: this is an INSTANCE fix. It closes the host-region
give-back. The CLASS -- an ownership authority over every pooled resource,
so a give-back can always ask "is this mine" -- stays open, and its docking
points are sgl-project#902 (ownership authority beyond KV rows, where host regions are
already listed as a sibling) and #888b's seed "every participant declares
the paths of its release sites". Four instances in one day is the argument
for doing it once, centrally, rather than a fifth time.

FUTURE CHECK: a sixteenth producer needs no edit to be stamped, and a slot
it double-queues is named with its own file and line on the first
occurrence instead of killing rank 0.

Evidence: desk. py_compile on both files; enqueue stamp, accessor and drain
check verified in the loaded modules; behavioural probe of the drain algebra
on the boot-18 shape (span 60934/5 queued twice + one already-free slot)
showing dupes/unowned/freed split correctly AND that the unfixed form would
have called free() on the unowned slot -- i.e. the probe can fail.
Belegstufe: DESK-BEWIESEN. Boot 19 on the 90-second control arm is the test.
efschu pushed a commit to efschu/htsglang that referenced this pull request Aug 29, 2026
…l-project#801 channel-asymmetry family)

Boot 59 gave the first complete three-rank stack of this death class, and it
names its own cause. PP2 sat in torch.distributed.isend under
send_tensor_dict -- the last rank's output wrap, which addresses
ranks[(rank_in_group + 1) % world_size] and so wraps 2 -> 0. That pair had
never carried anything, a torch p2p pair is built lazily, and the build needs
BOTH ends. PP0 was already parked in _pp_commit_admission_send_work with PP1
beside it, so nobody joined. Closed three-arc cycle, with the ranks at two
different lines of ONE loop body: PP2 at :4454, PP0/PP1 at :4514.

The first use of a pair must not also be the use that has to succeed while the
peers are somewhere else. So the ring walks its neighbours once, over the same
groups the real send uses -- device_group for device tensors, cpu_group for
the metadata half -- before any loop is entered. Every rank sends to its
successor and receives from its predecessor in one shot, which is the ring the
loop drives, so no rank waits on a peer that is not simultaneously present.
One byte per direction per group, idempotent.

Wired at dispatch_event_loop because that is the single entry to every loop,
flip and non-flip alike, and once per process.

UNGATED, deliberately: the cycle reproduces with enable_phase_flip off, which
this window measured directly (sgl-project#990 confirmed on this tree by boot 58 dying
flip-free on the same collective). Gating it would leave plain PP holding the
defect. Not barlink either -- barlink has no send/recv (sgl-project#732); this is the
ordinary send_tensor_dict wrap on the default group.

Evidence: desk, executed. py_compile on both files, and a path-identity check
against send_tensor_dict: same device_group/cpu_group, same
(rank_in_group + 1) % world_size arithmetic, same self.ranks[] mapping, and
neither side passes tag=, so both sit in the default tag space. The warmup
emits PP-P2P-WARMUP with its pairs and duration per rank, so its absence is as
visible as its failure. Belegstufe: DESK-BEWIESEN.
efschu pushed a commit to efschu/htsglang that referenced this pull request Sep 2, 2026
User order 2026-08-29: the ring-commit and admission-congruence apparatus
exists only for the retired second HiCache implementation and is left out, not
rebuilt. The kill criterion agreed at boot 52 -- two further ring-family
deaths -- is met five times over: 53, 55, 56, 57, 58, 59, 68, 69, 70, 71, with
boot 58 flip-free, so the family was never the flip's (sgl-project#990).

And the last measurement says the same thing from the other side. Boot 71,
three py-spy stacks at one instant: PP0 and PP1 both blocked in
`_pp_commit_admission_send_work` waiting for their admission-decision send to
be taken, PP2 blocked in the output wrap 2->0 that PP0 would have taken next.
A closed three-arc send cycle in which every arc belongs to this apparatus.
Plain upstream PP has no admission-decision ring at all, which is why
upstream does not have this deadlock. De-cyclising it would have been an
eleventh fix to a mechanism with no remaining purpose.

So three things are gated off when enable_phase_flip is false, each at its
definition so no call site can slip past:
  * _pp_commit_admission_send_work -- the reap that PP0/PP1 died in
  * _pp_send_admission_decision    -- the send it reaps
  * the HiCacheCollectiveTimeout bound in _pp_commit_comm_work, which falls
    back to the raw `work.wait()` the code already documents as the escape
    hatch of SGLANG_HICACHE_COLLECTIVE_TIMEOUT_S.

Dropping the bound removes a deadline, not a guarantee: `bounded_wait` takes a
single torch Work handle and its label only names the parked thread -- it is
not a collective, which I had claimed earlier and retracted on reading the
function.

Also recorded, because the branch does not do what its name says: before this
commit strip/minimal-flip-hicache stripped NOTHING. Against its base it was
+58/-3 lines in three commits, two of them mine (#1002d, sgl-project#1003) plus a census
line, and `pp-ring-commit` stood untouched at scheduler_pp_mixin.py:7160.

Evidence: desk, executed. py_compile; three gates present, each behind the
same predicate. Belegstufe: DESK-BEWIESEN.
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