From 5ee3bb683a7e64463f8d88c52945fe9bb9446bad Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Thu, 22 Oct 2026 11:29:31 +0300 Subject: [PATCH 1/2] test(server): make the busy-rescan re-arm assertion timing-free TestInformationalRescan_BusyServerIsRetriedNotDropped sampled the rescan timer map at one fixed instant. fireInformationalRescan drops the entry at fire time and re-creates it only after the gates and the claim ran, so the sample could land in that window under CI load (4 of 8 recent main runs). Read infoRescanSeq before scheduling and wait for a sequence above before+1, which only a busy re-arm can produce. No production change. --- .../server/scan_informational_rescan_test.go | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/internal/server/scan_informational_rescan_test.go b/internal/server/scan_informational_rescan_test.go index 543e892a8..ac8118c55 100644 --- a/internal/server/scan_informational_rescan_test.go +++ b/internal/server/scan_informational_rescan_test.go @@ -39,6 +39,17 @@ func (s *Server) rescanPending(name string) bool { return s.infoRescanTimers[name] != nil } +// rescanSeq is the sequence number of the server's armed rescan timer, 0 when +// none is armed (also for the moment between a fire and its re-arm). +func (s *Server) rescanSeq(name string) uint64 { + s.infoScanMu.Lock() + defer s.infoScanMu.Unlock() + if pending := s.infoRescanTimers[name]; pending != nil { + return pending.seq + } + return 0 +} + func TestInformationalRescan_DebouncedAndCoalesced(t *testing.T) { fake := newFakeSecurityScanner() // The server was scanned before: a rescan is for exactly that case. @@ -139,10 +150,19 @@ func TestInformationalRescan_BusyServerIsRetriedNotDropped(t *testing.T) { s.infoScanQueued["srv"] = true // an informational scan holds the claim s.infoScanMu.Unlock() + // Every arm takes the next sequence number: the change itself takes + // before+1, so anything above that is a re-arm by a busy fire. + s.infoScanMu.Lock() + before := s.infoRescanSeq + s.infoScanMu.Unlock() s.scheduleInformationalRescan("srv") - time.Sleep(4 * testRescanDebounce) + // The timer entry is dropped when the timer fires and re-created only after + // the gates and the claim ran, so "is a timer armed right now" is false for a + // moment on every busy fire. Wait for the proof of a re-arm (a newer sequence + // number) instead of sampling the map at a fixed instant. + require.Eventually(t, func() bool { return s.rescanSeq("srv") > before+1 }, 3*time.Second, 5*time.Millisecond, + "re-armed while busy") assert.Empty(t, fake.startScanAttempts(), "must not scan on top of a claimed slot") - assert.True(t, s.rescanPending("srv"), "re-armed while busy") s.infoScanMu.Lock() delete(s.infoScanQueued, "srv") From 9871eedbf2f50223a0e23b06615e44a371bd9a55 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Thu, 22 Oct 2026 11:49:54 +0300 Subject: [PATCH 2/2] test(scanner): bound refresh-concurrency scan latency by p90 plus an absolute ceiling The two guards compared the single worst of 300 scans against 50*median+50ms; one GC pause or descheduled goroutine on a macOS runner hit 119ms against a 109ms bound. Bound p90 relative to the baseline and keep an absolute 2s ceiling on the worst scan, which no scheduler stall reaches but any wait behind the gate or a fetch deadline would. --- .../scanner/tpa_refresh_concurrency_test.go | 32 ++++++++++++++++--- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/internal/security/scanner/tpa_refresh_concurrency_test.go b/internal/security/scanner/tpa_refresh_concurrency_test.go index 7c960ebe0..43326463b 100644 --- a/internal/security/scanner/tpa_refresh_concurrency_test.go +++ b/internal/security/scanner/tpa_refresh_concurrency_test.go @@ -60,9 +60,29 @@ func boolStr(b bool) string { } func median(ds []time.Duration) time.Duration { + return percentile(ds, 0.5) +} + +// percentile returns the q-quantile (0 < q <= 1) of ds. The latency guards +// below bound p90 relative to the baseline rather than the single worst scan: +// one GC pause or a descheduled goroutine on a loaded CI runner makes one +// scan take 100 ms without saying anything about the lock the guard is +// about. The single worst scan keeps an absolute ceiling (scanWorstCeiling) +// instead: a scan that really waited on the refresh waits for the gate or a +// multi-second fetch deadline, which no scheduler hiccup reaches. +// scanWorstCeiling is the absolute bound on any single scan while a refresh +// is blocked: well above a scheduler stall, well below the shortest wait a +// scan could spend behind the refresh (the test gate, or a fetch deadline). +const scanWorstCeiling = 2 * time.Second + +func percentile(ds []time.Duration, q float64) time.Duration { s := append([]time.Duration(nil), ds...) sort.Slice(s, func(i, j int) bool { return s[i] < s[j] }) - return s[len(s)/2] + i := int(float64(len(s)) * q) + if i >= len(s) { + i = len(s) - 1 + } + return s[i] } // scanLoop runs n scans, returning the generations seen and the slowest scan. @@ -104,11 +124,12 @@ func TestRefreshConcurrency_ScansIgnoreABlackholedFetchLeg(t *testing.T) { t.Fatal("the fetch leg never reached the channel") } - gens, worst, _ := scanLoop(300) + gens, worst, during := scanLoop(300) assert.Equal(t, map[string]int{"old": 300}, gens, "every scan runs against the serving (old) bundle") if !raceDetectorEnabled { - assert.Less(t, worst, 50*base+50*time.Millisecond, "a hung fetch must not slow scans (baseline median %s)", base) + assert.Less(t, percentile(during, 0.9), 50*base+50*time.Millisecond, "a hung fetch must not slow scans (baseline median %s)", base) + assert.Less(t, worst, scanWorstCeiling, "no scan waited behind the hung fetch") } } @@ -137,10 +158,11 @@ func TestRefreshConcurrency_ScansDuringASlowSelfCheckSeeOldOrNew(t *testing.T) { } // While the activation is mid-flight every scan still sees the old bundle. - during, worst, _ := scanLoop(300) + during, worst, took := scanLoop(300) assert.Equal(t, map[string]int{"old": 300}, during) if !raceDetectorEnabled { - assert.Less(t, worst, 50*base+50*time.Millisecond, "a slow self-check must not slow scans (baseline median %s)", base) + assert.Less(t, percentile(took, 0.9), 50*base+50*time.Millisecond, "a slow self-check must not slow scans (baseline median %s)", base) + assert.Less(t, worst, scanWorstCeiling, "no scan waited behind the self-check") } // Scans racing the swap itself see exactly the old or the new bundle.