Skip to content

Reorder CI unit tests. - #908

Merged
hnyls2002 merged 1 commit into
mainfrom
adjust-ci
Aug 3, 2024
Merged

hnyls2002 merged 1 commit into
mainfrom
adjust-ci

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

Move openai backend test to the last.

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 merged commit 947402c into main Aug 3, 2024
@hnyls2002
hnyls2002 deleted the adjust-ci branch August 3, 2024 23:18
timethink pushed a commit to timethink/sglang that referenced this pull request Mar 9, 2025
McZyWu pushed a commit to McZyWu/sgl-sglang that referenced this pull request Jul 2, 2026
Delete invalid test cases, update test_npu_lora_max_lora_rank.py, add the parameter --lora-backend
efschu pushed a commit to efschu/htsglang that referenced this pull request Aug 26, 2026
…, so 411 declines have no verdict

Routing from the write-side agent (sgl-project#872) narrowed Posten 0 correctly: the store
is healthy, 24277 canonical pages live during the R7 window, and the historic
"store is empty" readings probed a path this boot does not use. So the 671
refused walks are a MATCH problem, not a content problem. The question that
follows immediately is the one nobody could answer: a walk that matches nothing
should fall through to an L3 prefetch, and prefetch was attempted on only 264
of 675 sampled walks.

THE OBVIOUS ANSWER IS WRONG, and the code says why. It is tempting to read the
411 as the same MambaComponent refusal declining twice -- one root, two numbers.
It is not:

  * The prefetch anchor IS mamba-gated, but by an aliasing the base contract
    does not describe. base_prefix_cache.py:201-210 separates `last_host_node`
    (the L3 prefetch anchor) from `best_match_node` (the validator-consensus L2
    anchor) -- two fields, two purposes. UnifiedRadixCache sets them equal under
    HiCache (unified_radix_cache.py:1443-1449). Deliberate and commented, but it
    contradicts the field's own documentation, and two fields that are
    documented different and implemented identical are a drift source (sgl-project#747's
    shape). Named, not changed here.
  * Even so, the refusal does NOT decline the prefetch. Total refusal leaves
    best_match_node at the root, and the caller admits the root on purpose:
    `locally_eligible = last_host_node.backuped or last_host_node is
    self.tree_cache.root_node` (scheduler.py:4933-4935). A cold request with no
    match is supposed to prefetch. The first term holds.
  * The second term (>= 256 new tokens) holds for agent prompts of thousands.

That leaves the rate limiter -- AND THERE THE EVIDENCE STOPS, which is the
actual finding. None of the three terms is counted or logged. The 411 declines
are silent. Nobody can attribute them from this boot, and no amount of
re-reading the log will change that.

    eligible = (locally_eligible
                and prefetch_length >= self.prefetch_threshold
                and not self.cache_controller.prefetch_rate_limited())

Three unrelated verdicts wearing one boolean, remedies in three different
files. The sgl-project#914 shape one module over: blame without a defect.

So this is an instrument and NOT a policy change. The three terms are the same
three, evaluated in the same order, with the same outcome; `eligible` is now
`reason is None`, one expression, so the predicate and its explanation cannot
drift apart the way sgl-project#747 records these very lineages drifting. The first
failing term is named rather than all of them -- a request can trip several,
and summing them would double-count exactly as
`refused_tokens_by_component` is documented to.

Not env-gated, unlike the sgl-project#904 match census. That one builds an object and
walks validators a second time, so it pays for itself only when armed. This is
one integer increment on a path that already builds a RadixKey and takes a host
lock -- and a counter you must remember to arm cannot answer "was it ever
tried", which is the whole question. `attempted` is counted alongside the
refusals so the denominator is local (sgl-project#873: a denominator reconstructed from
another log is how a narrowed candidate set reads as a decomposition), and an
unrecorded gate says "no observation" rather than a silent zero (sgl-project#829).

HYPOTHESIS, RECORDED AS ONE. prefetch_capacity_limit is `0.5 *
mem_pool_host.size` (cache_controller.py:729), and that size is not constant
across a phase flip: sgl-project#905 measured the two host tiers at 703472 rows (PP) and
30518 (TP), 23x apart, putting the TP-phase budget near 15259 tokens -- under
four prefetches of the 4096 this window actually completed, against 5
concurrent requests per flip. The order of magnitude fits 411/675, and I am
deliberately not fitting it exactly: the PP phase has ample budget, so the
number cannot come from there alone. If it holds, the same host-tier asymmetry
behind sgl-project#905's double-free is also starving the TP-phase prefetch budget -- one
defect, two effects, to be checked rather than assumed. The counter decides it
on the next boot instead of re-arguing it.

RED-FIRST, four mutants, each red, restored green (14 tests):
  revert to the shipped conjunction (no counter) ......... 4 failed
  eligible computed twice instead of derived from reason . 2 failed
  attempts not counted (denominator lost) ................ 2 failed
  no-observation reported as attempted=0 ................. 1 failed

ONE SELF-INFLICTED DEFECT, found and fixed in the same pass, recorded because
the form is evidently easy to commit. My first version of
test_the_rate_limit_check_is_still_called_at_most_once counted occurrences in
the function's RAW source and went red because a PRE-EXISTING comment at :2674
mentions `prefetch_rate_limited()`. That is the sgl-project#908 substring defect -- prose
used to reach a verdict about code -- in a test of mine, an hour after I
rejected the same form in someone else's patch. Comment lines are now stripped
before counting.

GATE test/registered/unit/mem_cache serial, CVD="": 3 failed, 1950 passed,
1658 skipped. Both axes agree (summary 3, ANSI-stripped extraction 3). Two are
test_acceptance_emitters_758::RefillTiming, baseline-proven pre-existing
earlier in this branch; the third is test_pool_invariant_double_owned_912, a
concurrently-developed test belonging to sgl-project#912 and untouched by this commit.
The sgl-project#904 census suite is green against this change (62 passed together with
the sgl-project#913/sgl-project#914/sgl-project#915 suites), which is the regression that matters: note_refused
gained an optional third argument and its existing callers must not care.
efschu pushed a commit to efschu/htsglang that referenced this pull request Aug 26, 2026
…ated gap and a

stale double-owned reading as a leak, killing all three schedulers on idle

Five on_idle firings (two boots) showed available+evictable+withheld
exceeding total by exactly 22, never a deficit -- the opposite sign from
sgl-project#814/sgl-project#902/sgl-project#832/sgl-project#856, and never previously routed through the sgl-project#822
ownership authority. Decomposition: 21 rows from
_check_full_pool reading TokenToKVPoolAllocator.available_size() (a raw
sum of free_pages+release_pages, allocator/token.py:52-54) instead of the
union read_free_rows() already used by the phase-flip census and the sgl-project#822
audit (kv_row_ownership.py:743-843); 1 row from a single row simultaneously
claimed by the free list and the radix tree, the sgl-project#822 authority's own
EXCLUSIVITY "claimed by multiple" finding, never subtracted before.

Fix, both additive, neither a tolerance/epsilon:
- _check_full_pool now reads available via read_free_rows() when the
  allocator can enumerate; composite/watermark allocators keep
  ps.full_available_size unchanged.
- _check_pool_invariant gained a double_owned term, subtracted, sourced
  from allocator.double_owned_slots (the sgl-project#822 EXCLUSIVITY count).

Two defects found in review of the first cut, both fixed here:
1. The double_owned filter in
   phase_flip_runtime.py::_census_ownership_audit selected violations by
   substring match on Violation.detail ("more than one owner" in
   v.detail) -- prose used as control flow, against Violation's own
   "detail is never load-bearing" contract, the same shape as the
   line_gate substring defect (sgl-project#908). Fixed by giving Violation a
   structural kind field, set to EXCLUSIVITY_DOUBLED or
   EXCLUSIVITY_UNOWNED at its two EXCLUSIVITY construction sites in
   kv_row_ownership.py, and filtering on kind instead of detail text.
2. double_owned_slots is published only at phase-flip census (seam)
   events and read by on_idle at unrelated times; a reading taken before
   authority.retire() drops every claim it was computed from is stale
   past that point. Fixed by clearing double_owned_slots to None (not 0)
   at the exact point _retire_row_id_space calls authority.retire().
   Checked analytically whether staleness alone could mask a genuine
   deficit: it cannot, for any non-negative reading, because double_owned
   is subtracted and can only push total_accounted further below total.
   It can mask an unrelated, coincidentally-equal-sized surplus; that
   residual case is disclosed, not eliminated, and bounded by the
   cutover-clearing fix.

Tests (test/registered/unit/mem_cache/test_pool_invariant_double_owned_912.py,
14 tests, 31 subtests): reconstructs all five measured specimens,
red-first against unfixed code, green after the fix; separate mutants for
the original subtraction, the kind-based filter, and the retire-time
clearing, each shown to fail only its own guard when reverted; a
parametrized check that no non-negative stale double_owned value can turn
a manufactured 100-row deficit into a false pass.

Gate, hermetic (CUDA_VISIBLE_DEVICES=""), attribution proven by rerunning
identical failing node-id lists with these three files restored to HEAD
(git show HEAD:<path>) and diffing failure sets -- both axes' failures are
identical with and without this fix, i.e. pre-existing:
- mem_cache/: 2 failed, 1951 passed, 1658 skipped, 392 subtests passed
  (139.10s). Failures: test_acceptance_emitters_758.py RefillTiming x2,
  RuntimeError: No CUDA GPUs are available.
- managers/: 15 failed, 4631 passed, 18 skipped, 563 subtests passed
  (719.09s). Failures: test_arena_high_water_631.py x7 and
  test_restore_never_rebuild_677.py x4 (RuntimeError: No CUDA GPUs are
  available), test_phase_flip_rotation_wiring_809.py x4 (Exception:
  retry() exceed maximum number of retries) -- all reproduced identically
  against HEAD-restored files.

ruff check and codespell clean on every line touched by this change (both
tools also flag pre-existing issues elsewhere in phase_flip_runtime.py and
invariant_checker.py, outside this diff's hunks, left untouched as out of
scope for sgl-project#912).
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