fix(validation): make an I-MEM incident diagnosable, and stop double-counting cause splits - #302
Merged
Merged
Conversation
…counting causes
Four changes, all prompted by triaging the v1.5.11 24h soak gate failure and
finding the artifact could not answer the questions that mattered.
1. Paired heap profiles. The forensics collector already captures heap.pprof
at incident time, but ONE snapshot cannot show growth: the retaining
allocation site can only be inferred from composition. The property loop now
fetches a second profile the instant the slope oracles begin judging
(properties.SlopeWarmup(), newly exported so the two use the same clock) and
writes it as heap-warm.pprof beside the cell's tallies. Triage becomes a
measurement:
go tool pprof -inuse_space -base heap-warm.pprof <incident>/heap.pprof
The v1.5.11 I-MEM-1 failure could not be attributed for exactly this reason;
three separate hypotheses had to be refuted by other means.
2. h2c churn read budget 10s -> 20s. At 10s it exactly equalled celeris's
default ReadHeaderTimeout, so when the server timed a request out the
walker's deadline expired in the same instant and which side fired first was
scheduling noise. That makes the eof/timeout cause split undiagnostic
precisely when it matters -- the soak recorded one h2c_hang with
cause=timeout at the full budget, and the artifact cannot say whether the
server stalled or merely hit its own header deadline.
3. ws_handshake_fail cause split. Three distinct failures -- the peer vanishing
before the status line, a real non-101 rejection, and a stall mid-header-
drain -- collapsed into one counter, mirroring what celeris#470 already fixed
for h2c_hang. A single failure in ~47,598 attempts, as the soak recorded, was
not diagnosable at all.
4. The gate no longer counts a cause split as a violation of its own. The soak
printed "5 violations" for three distinct events, because one h2c hang was
counted once as h2c_hang and again as h2c_hang_timeout. recordHang always
increments the total alongside a cause, so the two are strictly coupled and
gating both is pure double-counting. The causes are now folded into the
total's message (" (timeout=1)") so nothing diagnostic is lost while the
violation count equals the event count.
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.
Four changes, all prompted by triaging the v1.5.11 24h soak gate failure (run 34003151551) and finding the artifact could not answer the questions that mattered.
1. Paired heap profiles — the important one
The forensics collector already captures
heap.pprofat incident time. But one snapshot cannot show growth: the retaining allocation site can only be inferred from composition, never read off.The property loop now fetches a second profile the instant the slope oracles begin judging and writes it as
heap-warm.pprofbeside the cell's tallies. Triage becomes a measurement rather than an inference:properties.SlopeWarmup()is newly exported so the baseline lands on exactly the clock the predicates use — a profile taken earlier would diff against cold-start ramp instead of steady state.This is not hypothetical. The v1.5.11
I-MEM-1failure could not be attributed for precisely this reason: three separate hypotheses (the boundedMemoryKVsweep, the celeris#507 session state, the ratelimit map) each had to be refuted by other means, and the actual cause is still unestablished. With a baseline profile the next occurrence names its own site.2. h2c churn read budget: 10s → 20s
At 10s it exactly equalled celeris's default
ReadHeaderTimeout. When the server timed a request out, the walker's deadline expired in the same instant, and which side fired first was decided by scheduling noise.That makes the eof/timeout cause split undiagnostic precisely when it matters. The soak recorded one
h2c_hangwithcause=timeoutat the full budget, and the artifact cannot say whether the server stalled or merely hit its own header deadline. A walker budget strictly longer than the server's means a server-side timeout arrives as something classifiable, and only a genuine stall exhausts ours.3. ws_handshake_fail cause split
Three distinct failures collapsed into one counter: the peer vanishing before the status line, a real non-101 rejection, and a stall mid-header-drain. This mirrors what celeris#470 already established for
h2c_hang.New:
ws_handshake_fail_{eof,timeout,reset,status,other}, which sum to the existing total. The soak recorded a single failure in ~47,598 attempts and it was not diagnosable at all.4. The gate no longer double-counts cause splits
The soak printed "5 violations" for three distinct events, because one h2c hang was counted once as
h2c_hangand again ash2c_hang_timeout.recordHangalways increments the total alongside a cause, so the two are strictly coupled — gating both is pure double-counting. Causes are now folded into the total's message (" (timeout=1)"), so nothing diagnostic is lost while the violation count equals the event count.Two new tests pin the contract: one defect must produce exactly one violation, with the cause surviving in the message. The four table rows that asserted each cause is independently a violation are removed — they encoded the behaviour this fixes.
Verification
go build ./...,go vet ./validation/...clean. Full suite green:Related
Filed while triaging the same run: #297 (9 of 14 predicates uninstrumented in every cell), #298 (h2c never upgrades, so
h2c_hangis vacuous), #299 (nightly cells too short to judge memory), #300 (Tier 1 never runs; one hardcoded spec), #301 (io_uring alwaysfixed_files=false).