fix(harness): the no-loss stranding budget sat on the healthy distribution's centre - #115
Merged
Merged
Conversation
…ution's centre test_load_runner::test_run_load_end_to_end_no_loss red the required windows-2025 leg on `main` at 9b03057. Nothing was lost: sent=90 acked=44 timeouts=46 nak=0 errors=0 engine_read=52 engine_written=104 sink_received=104 backlog=0 max_drain_seconds observed 4.7 / threshold 30.0 OK 104 written, 104 received at the sink, backlog drained in 4.7s of a 30s bound, leg finished in 20:05 well inside every cap. It failed the reconcile. THE BUDGET WAS THE BUG. `_reconcile` excuses unconfirmed sends (in-flight at a connection close, no ACK seen) up to `max(unconfirmed_budget, sent // 2)`, past which it declares a systemic no-ACK fault. That fraction was sized when the worst teardown stranding on record was 14/90 (~16%), which report.py documented as "half is ~3x the worst seen". windows-2025 has since produced 46/90 (~51%) on a lossless run -- so the "3x headroom" was 0.98x and the run failed by ONE message over a budget of 45. A threshold sitting on the centre of the healthy distribution is a coin flip, not a detector. Same defect as the ubuntu step cap in #104 (a green 775s run against a 780s bound) and as test_coord_lock's arrival-spread assumption: a bound tuned to what was observed once, then met by a slower runner. THE TWO DETECTORS DISAGREED. tests/test_load_runner.py already recorded the 46/90 observation in a comment and tuned ITS systemic-ACK check to `acked >= sent // 4` -- tolerating 75% stranding. But the reconcile's budget failed at half + 1, so the tuning never applied to the path that actually fired. The comment said "~half is healthy"; the code said "fail above half". Raises the fraction to three quarters in all three deliberately-synchronised copies (report, connscale, estate) plus the multishard doc-comment. Three quarters is ~1.5x the worst healthy value on record, while a dead ACK path strands ~100% and still blows it -- and it makes the two detectors encode the SAME tolerance. DELIBERATELY NOT CHANGED: `excused = 0 if over_budget else unconfirmed`. Clamping it to the budget gives a better failure message (the current one claims "lost 38 on intake" for a run that lost nothing), but test_harness_reconcile pins the all-or-nothing behaviour on purpose -- "with the flood masking a real shortfall, nothing is excused: the loss is reported too" -- and that cliff is only reachable once the run has already failed. Left alone rather than overridden as a side-effect; noted in-code for whoever wants to revisit it deliberately. The anti-vacuity floor is untouched and still unconditional: `read >= sent // 2`, enforced separately from the excusal. On the failing run it PASSED (52 >= 45) -- the run met the guarantee the budget exists to protect and failed anyway. Tests: renames the cap pin to ...capped_at_three_quarters_of_the_run and moves its boundary to 67/68, and adds a regression pin carrying the exact 9b03057 counters (90/46/52/104) that must reconcile clean. 1,371 harness/load tests green; ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
wshallwshall
enabled auto-merge (squash)
August 1, 2026 10:36
This was referenced Aug 1, 2026
wshallwshall
added a commit
that referenced
this pull request
Aug 1, 2026
…healthy runner (#117) Diagnosis only; no code change. The CI symptom was fixed in #115 (06fd327) by widening the reconcile's stranding budget; this files the capacity fact that fix deliberately did not address. The load smoke offers 60 msg/s at a listener whose ingress is strictly serial per connection (mllp.py awaits the durable commit per frame), so total ingress is pool_size / commit-latency. On windows-2025 that is under 60/s: it stranded ~51% at 60/s twice (9b03057, 56f7d24) with byte-identical counters -- 90 sent, 44 acked, 46 stranded, 52 read -- on runs that lost nothing. Includes a RETRACTION of this item's own first write-up. It claimed the signature reproduced on a healthy developer box by raising the offered rate alone, citing one 600/s run that stranded 456 of 900 (50.7%). Four repeats of that command stranded 0 every time; the outlier was taken while an unrelated suite ran concurrently. Concluding from n=1 is the exact failure mode this item documents. Surviving claim: an unloaded box strands ZERO at up to 10x the profile's rate, while windows-2025 strands ~51% at 60/s, reproducibly. Byte-identical repetition rules out weather ON THAT LEG; it says nothing about a developer box. Also records what is NOT determined (why that runner's commit is ~10x slower -- disk, Defender, contention; none measured, none measurable from outside) and one adjacent unverified finding (at saturation engine_read cleared the unconditional `read >= sent // 2` floor by two messages; a floor breach is a hard failure no budget widening can rescue).
wshallwshall
added a commit
that referenced
this pull request
Aug 1, 2026
… one point (#118) test_load_runner red the required windows-2025 leg twice on main (9b03057, 56f7d24) with byte-identical counters -- 90 sent, 44 acked, 46 stranded, 52 read -- on runs that lost nothing. #115 widened the reconcile's stranding budget so a saturated-but-lossless run stops failing. That fixed the symptom and said nothing about the cause, and a pass/fail test at ONE fixed offered rate structurally cannot: the question is whether windows-2025 is slow or windows is, and that is a rate, not a verdict. Adds `harness.load.ingress_probe` (sweeps offered rate, prints one machine- parseable RESULT line per run) and a workflow_dispatch-only workflow that runs it across ubuntu-latest / windows-2022 / windows-2025 and writes a side-by-side table to the step summary. ubuntu is the CONTROL: it establishes what "not saturated" looks like on hosted hardware so a slow Windows row can be read against it. REPEATS ARE THE POINT, and this is a correction to my own earlier claim. One 600/s run on a developer box produced 456 stranded of 900 (50.7%) -- a near-exact match for the windows-2025 CI signature -- and I wrote it up as a clean reproduction. Four repeats of the same command on the same box then produced 0 stranded, every time; the outlier was taken while an unrelated test suite was running. So stranding here is a CONTENTION artifact, not a clean function of offered rate, and n=1 is not a measurement. Hence --repeat, defaulted to 3, with the correction recorded in the module docstring so the next reader does not redo it. What survives that correction is the weaker, still-useful claim: an unloaded box strands ZERO at up to 10x the CI profile's rate, while windows-2025 stranded ~51% at the profile's own 60/s, twice, byte-identically. Deliberately NOT a required context and structurally unfit to become one: no pull_request trigger, and the probe exits 0 even when the reconcile fails, because a machine too slow to keep up is the finding rather than an error. Recorded in .github/required-contexts.txt with the other advisory workflows. Prints no derived per-second figure either -- engine_read/wall looks like a service rate and is not one (wall includes stop grace, drain and settle, so it lands at ~25/s whether the run offered 60/s or 600/s). Scope: both legs are hosted VMs, so a slow-2025/fast-2022 result narrows the cause to the 2025 IMAGE. It cannot clear or convict Windows Server 2025 as a deployment target -- only the self-hosted WS2025 rig can, and it was unregistered (actions/runners -> total_count 0) when this was written. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
test_load_runner::test_run_load_end_to_end_no_lossred the required windows-2025 leg onmainat9b03057f. Nothing was lost.104 written, 104 received at the sink, backlog drained in 4.7s of a 30s bound, leg finished in 20:05 — inside every cap. It failed the reconcile.
The budget was the bug
_reconcileexcuses unconfirmed sends (in-flight at a connection close, no ACK seen) up tomax(unconfirmed_budget, sent // 2), past which it declares a systemic no-ACK fault.report.pydocumented why that fraction was chosen:It was sized against a worst-observed of 14/90. windows-2025 has since produced 46/90 (~51%) on a lossless run. The 3× headroom is 0.98×, and the run failed by one message over a budget of 45.
A threshold sitting on the centre of the healthy distribution is a coin flip, not a detector. Same defect as the ubuntu step cap in #104 (a green 775s run against a 780s bound) and as
test_coord_lock's arrival-spread assumption — a bound tuned to a single observation, later met by a slower runner.The two detectors disagreed
tests/test_load_runner.pyalready recorded the 46/90 observation in a comment, and tuned its systemic-ACK check toacked >= sent // 4— tolerating 75% stranding. But the reconcile's budget failed at half + 1, so that tuning never applied to the path that actually fires. The comment said "~half is healthy"; the code said "fail above half".The fix
Three quarters, in all three deliberately-synchronised copies (
report,connscale,estate) plus themultisharddoc-comment. That's ~1.5× the worst healthy value on record, while a dead ACK path strands ~100% and still blows it — and it makes the two detectors encode the same tolerance.What I deliberately did NOT change
excused = 0 if over_budget else unconfirmedis a cliff: at the budget everything is excused, at budget+1 nothing is, so the failure detail claimed "lost 38 on intake" for a run that lost nothing. I changed it to clamp, then reverted —test_harness_reconcilepins the all-or-nothing behaviour on purpose ("with the flood masking a real shortfall, nothing is excused: the loss is reported too"), and the cliff is only reachable once the run has already failed. Overriding a deliberate design decision as a side-effect of a different fix is how you get two bugs. Noted in-code for whoever wants to revisit it on its own merits.The anti-vacuity floor is untouched and still unconditional (
read >= sent // 2, enforced separately from the excusal). On the failing run it passed — 52 ≥ 45. The run met the guarantee the budget exists to protect and failed anyway.Tests
..._capped_at_three_quarters_of_the_run, boundary moved to 67/68.9b03057fcounters (90 sent / 46 stranded / 52 read / 104 delivered) that must reconcile clean, so this specific flake cannot come back.ruff check+ruff formatclean. Pre-commit passed.