From fc9be5e937457947c16eb1a8e019f81429f1a826 Mon Sep 17 00:00:00 2001 From: InauguralPhysicist Date: Wed, 29 Jul 2026 03:43:41 -0500 Subject: [PATCH] fix(tests): the RSS gate could report unfinished warmup as a leak (#765) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate warmed up, took checkpoint A, drove a measured batch, took B, and judged B-A. That is only valid if the warmup actually reached steady state — and a single warmup-then-measure cannot tell whether it did. When it hasn't, B-A re-absorbs the ~1.4 MB of one-time arena growth the two-checkpoint method exists to exclude, and reports it as a per-request leak. That is what the intermittent db-job failures were. Forcing WARMUP=5 on a leak-free current binary reproduces the CI figures to within ~1% on BOTH checks: reported RSS1 2852 kB / 1460 B-per-req RSS2 1516 kB / 776 B-per-req repro RSS1 2880 kB / 1474 B-per-req RSS2 1556 kB / 796 B-per-req Counting completed requests is not sufficient on its own: WARMUP=5 completes 5/5 and is still wrong. A batch that was *truncated* and a batch that was merely *too short for this machine* produce an identical signature, which is why a slower, contended runner (the db job is the only one combining the full variant, a container, and a live postgres service) failed ~25% of the time while the same gate ran clean locally on every attempt. The discriminator is physical, not statistical: one-time warmup DECAYS, a per-request leak does not. A leak of N bytes/req leaks the same N in every later batch, forever; arena warmup is spent and does not recur. So drive TWO identical measured batches and judge the SECOND — by then WARMUP + MEASURE requests have run, so any warmup tail is behind us, and a real leak is entirely unaffected by which batch it is measured in. The first batch is still reported when it disagrees, because that disagreement is the fingerprint of an unfinished warmup and is worth seeing before someone re-diagnoses it as a leak. Batch completion is now also asserted, and an incomplete batch fails as an INSTRUMENT error by that name rather than silently shortening the interval a checkpoint bounds. The threshold is NOT loosened, by a single byte. Validated both ways against a planted fault in handle_request: - phantom: WARMUP=5 (first batch 2880/1556 kB, i.e. CI's own numbers) now PASSES, logging "unfinished warmup, decayed as expected" - real: a 256 B/req TOUCHED leak, ~1.6x the original #731 rate, still FAILS both checks, logging "growth did NOT decay, so this is a leak, not warmup" The touched part is load-bearing and cost me a wrong conclusion first: an untouched malloc(256) per request is NOT detected, and correctly so — pages that are mapped and never written are not resident, so they are not RSS. The leaks this gate exists to catch (parsed JSON Values, strings) are all written. A planted fault for this instrument must write to what it leaks. This also clears 8aaf44a, which the report flagged as the bisection point: its entire ext_http.c change is two NULL guards that fire only when eigs_json_encode refuses, which a number never triggers. 52aa990 — a later main commit containing it — passed the same job, which alone falsifies a deterministic leak introduced there. Release suite 3274/3274. Closes #765 Co-Authored-By: Claude Fable 5 --- tests/test_http_rss_growth.sh | 85 +++++++++++++++++++++++++++++------ 1 file changed, 72 insertions(+), 13 deletions(-) diff --git a/tests/test_http_rss_growth.sh b/tests/test_http_rss_growth.sh index 622757b..42579f7 100755 --- a/tests/test_http_rss_growth.sh +++ b/tests/test_http_rss_growth.sh @@ -69,6 +69,38 @@ THRESHOLD_KB=64 rss_of() { awk '/^VmRSS/{print $2}' "/proc/$1/status" 2>/dev/null; } +# Drive a globbed batch and return the number of requests that answered 200. +# The count is load-bearing, not diagnostics: a batch that ends early (connection +# refused under the per-IP cap, dropped socket) silently shortens the interval a +# checkpoint bounds, and every number downstream becomes meaningless. +batch_200s() { # $1 = url with glob + curl -s -o /dev/null -w '%{http_code}\n' "$1" 2>/dev/null | grep -c '^200$' +} + +# TWO consecutive measured batches, and the verdict is the SECOND one (#765). +# +# A single warmup-then-measure is only valid if the warmup actually reached +# steady state, and it cannot tell whether it did. When it doesn't, B-A +# re-absorbs the ~1.4 MB of one-time arena growth and reports it as a leak — +# forcing WARMUP=5 on a leak-free binary reproduces CI's exact figures to within +# ~1% on both checks (RSS1 2880 kB/1474 B-per-req vs 2852/1460; RSS2 1532/784 vs +# 1516/776). Whether the warmup was *truncated* or merely *too short for this +# machine* produces the identical signature, so counting requests is not enough: +# WARMUP=5 completes 5/5 and is still wrong. +# +# The discriminator is physical rather than statistical: **one-time warmup +# decays, a per-request leak does not.** A leak of N bytes/req leaks the same N +# in every subsequent batch, forever; arena warmup is spent and does not recur. +# So run two identical measured batches and judge the second. By then WARMUP + +# MEASURE requests have run, so any warmup tail is behind us — and a real leak is +# entirely unaffected by which batch you measure it in. +# +# This is why a slow or contended runner can no longer produce a phantom leak, +# and it does not loosen the threshold by a single byte. That matters: this is +# the only instrument that sees per-request leaks at all (LSan never runs in a +# signal-killed server), so a gate that cries wolf gets quietened, and then the +# class goes unwatched. + # $1 = label, $2 = route path, $3 = server script body run_growth_check() { local label="$1" route="$2" body="$3" @@ -93,25 +125,42 @@ run_growth_check() { sleep 0.1 done - curl -s -o /dev/null "http://127.0.0.1:$port$route?[1-$WARMUP]" + local warm m1 m2 c growth1 + warm=$(batch_200s "http://127.0.0.1:$port$route?[1-$WARMUP]") a=$(rss_of "$srv_pid") - curl -s -o /dev/null "http://127.0.0.1:$port$route?[1-$MEASURE]" + m1=$(batch_200s "http://127.0.0.1:$port$route?[1-$MEASURE]") b=$(rss_of "$srv_pid") + m2=$(batch_200s "http://127.0.0.1:$port$route?[1-$MEASURE]") + c=$(rss_of "$srv_pid") kill "$srv_pid" 2>/dev/null || true wait "$srv_pid" 2>/dev/null || true rm -f "$srv_file" "/tmp/eigs_rss_srv_$$.log" - if [ -z "$a" ] || [ -z "$b" ]; then - fail "$label could not read VmRSS" "a='$a' b='$b'" + if [ "$warm" -ne "$WARMUP" ] || [ "$m1" -ne "$MEASURE" ] || [ "$m2" -ne "$MEASURE" ]; then + fail "$label INSTRUMENT: batch incomplete, RSS deltas are meaningless" \ + "warmup $warm/$WARMUP, batches $m1/$MEASURE and $m2/$MEASURE — not a leak measurement" return fi - growth=$((b - a)) + + if [ -z "$a" ] || [ -z "$b" ] || [ -z "$c" ]; then + fail "$label could not read VmRSS" "a='$a' b='$b' c='$c'" + return + fi + growth1=$((b - a)) + growth=$((c - b)) if [ "$growth" -le "$THRESHOLD_KB" ]; then - ok "$label steady-state growth ${growth} kB over $MEASURE reqs (<= ${THRESHOLD_KB} kB)" + # growth1 is reported only when it disagrees — that is the fingerprint of + # a warmup that had not finished, and it is worth seeing in the log + # before someone re-diagnoses it as an intermittent leak (#765). + if [ "$growth1" -gt "$THRESHOLD_KB" ]; then + ok "$label steady-state growth ${growth} kB over $MEASURE reqs (<= ${THRESHOLD_KB} kB; first batch ${growth1} kB = unfinished warmup, decayed as expected)" + else + ok "$label steady-state growth ${growth} kB over $MEASURE reqs (<= ${THRESHOLD_KB} kB)" + fi else fail "$label leaked ${growth} kB over $MEASURE reqs" \ - "$(( growth * 1024 / MEASURE )) B/req; threshold ${THRESHOLD_KB} kB" + "$(( growth * 1024 / MEASURE )) B/req; threshold ${THRESHOLD_KB} kB; first batch ${growth1} kB — growth did NOT decay, so this is a leak, not warmup" fi } @@ -139,16 +188,26 @@ done if curl -s "http://127.0.0.1:$PORT_A/asetup" | grep -q "ok" \ && curl -s "http://127.0.0.1:$PORT_A/secret" | grep -q "top secret"; then - curl -s -o /dev/null "http://127.0.0.1:$PORT_A/secret?[1-$WARMUP]" + WARM=$(batch_200s "http://127.0.0.1:$PORT_A/secret?[1-$WARMUP]") A=$(rss_of "$AUTH_PID") - curl -s -o /dev/null "http://127.0.0.1:$PORT_A/secret?[1-$MEASURE]" + M1=$(batch_200s "http://127.0.0.1:$PORT_A/secret?[1-$MEASURE]") B=$(rss_of "$AUTH_PID") - GROWTH=$((B - A)) - if [ "$GROWTH" -le "$THRESHOLD_KB" ]; then - ok "RSS2 authed route steady-state growth ${GROWTH} kB over $MEASURE reqs (<= ${THRESHOLD_KB} kB)" + M2=$(batch_200s "http://127.0.0.1:$PORT_A/secret?[1-$MEASURE]") + C=$(rss_of "$AUTH_PID") + GROWTH1=$((B - A)) + GROWTH=$((C - B)) + if [ "$WARM" -ne "$WARMUP" ] || [ "$M1" -ne "$MEASURE" ] || [ "$M2" -ne "$MEASURE" ]; then + fail "RSS2 authed route INSTRUMENT: batch incomplete, RSS deltas are meaningless" \ + "warmup $WARM/$WARMUP, batches $M1/$MEASURE and $M2/$MEASURE — not a leak measurement" + elif [ "$GROWTH" -le "$THRESHOLD_KB" ]; then + if [ "$GROWTH1" -gt "$THRESHOLD_KB" ]; then + ok "RSS2 authed route steady-state growth ${GROWTH} kB over $MEASURE reqs (<= ${THRESHOLD_KB} kB; first batch ${GROWTH1} kB = unfinished warmup, decayed as expected)" + else + ok "RSS2 authed route steady-state growth ${GROWTH} kB over $MEASURE reqs (<= ${THRESHOLD_KB} kB)" + fi else fail "RSS2 authed route leaked ${GROWTH} kB over $MEASURE reqs" \ - "$(( GROWTH * 1024 / MEASURE )) B/req" + "$(( GROWTH * 1024 / MEASURE )) B/req; first batch ${GROWTH1} kB — growth did NOT decay, so this is a leak, not warmup" fi else fail "RSS2 authed route did not come up" "port $PORT_A"