Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 19 additions & 15 deletions internal/server/profile_tool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -756,35 +756,39 @@ func selectablePinnedCtx(pin string, allowed ...string) context.Context {
// The oracle here is deterministic, not wall-clock: the allocation profile of
// one predicate call is identical for every pin outcome over the same fleet
// (the early-returning version allocated 2 / 0 / 1 times respectively).
//
// AllocsPerRun counts process-wide mallocs, so a goroutine still winding down
// from an earlier test (a runtime fixture's shutdown, an index observer)
// inflates whichever case it overlaps — CI once read 48 for one case and 12
// for the others — and, independently, `go test -race` disables the
// tiny-object allocator, which can make two otherwise-identical cases differ
// by a small constant on every back-to-back reading with no gap between
// them. Noise only ever ADDS, so retryUntilAllocsMatch (defined alongside
// its identical use in TestProfileMiddleware_RefusalWorkIndependentOfFleet)
// retries a full reading of the three cases, with a real sleep between
// attempts, until one attempt finds them all equal.
func TestSelectableProfileNames_PinOutcomesDoSameWork(t *testing.T) {
const n = 64
alive := selectableProbeConfig(n)
deleted := selectableProbeConfig(n)
deleted.Profiles[0].Name = "was-the-pin" // same fleet size, the pin is gone

cases := map[string]struct {
caseConfigs := map[string]struct {
ctx context.Context
cfg *config.Config
}{
"reachable pin first": {selectablePinnedCtx("pin", "pin-srv"), alive},
"zero-reach pin first": {selectablePinnedCtx("pin", "other-srv"), alive},
"deleted pin": {selectablePinnedCtx("pin", "pin-srv"), deleted},
}
// AllocsPerRun counts process-wide mallocs, so a goroutine still winding
// down from an earlier test (a runtime fixture's shutdown, an index
// observer) inflates whichever case it overlaps — CI once read 48 for
// one case and 12 for the others. Noise only ever ADDS, so the minimum
// over a few samples of the predicate alone (index built outside the
// window) is the deterministic figure this test is about.
allocs := map[string]float64{}
for name, c := range cases {
idx := newProfileIndex(c.cfg)
best := math.Inf(1)
for i := 0; i < 7; i++ {
best = math.Min(best, testing.AllocsPerRun(50, func() { idx.selectableNames(c.ctx) }))
}
allocs[name] = best

settleBackgroundGoroutines(t)
cases := make(map[string]func(), len(caseConfigs))
for name, c := range caseConfigs {
idx, ctx := newProfileIndex(c.cfg), c.ctx // index built outside the measured window
cases[name] = func() { idx.selectableNames(ctx) }
}
allocs := retryUntilAllocsMatch(15, 50, "reachable pin first", cases)
for name, got := range allocs {
require.Equal(t, allocs["reachable pin first"], got, "%s must allocate exactly like a reachable pin: %v", name, allocs)
}
Expand Down
117 changes: 97 additions & 20 deletions internal/server/profile_url_gate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"net/http"
"net/http/httptest"
"path/filepath"
goruntime "runtime"
"slices"
"strings"
"sync/atomic"
Expand Down Expand Up @@ -180,6 +181,87 @@ func profileGateRefusal(t *testing.T, handler http.Handler, agent *auth.AuthCont
return rec
}

// settleBackgroundGoroutines waits for the process's live goroutine count to
// stop moving before an allocation-parity measurement starts. internal/server
// runs its tests in one shared binary; a goroutine still winding down from an
// earlier test (a runtime fixture's shutdown, an SSE/HTTP client closing
// against an already-stopped httptest server) can still be allocating when a
// later, unrelated test opens its testing.AllocsPerRun window — AllocsPerRun
// counts every goroutine's mallocs process-wide, not just the calling one's.
// This is a SINGLE settle pass before measurement begins, not one per sample:
// a prior attempt forced runtime.GC() between every measurement instead and
// made the flakiness worse, not better — each GC is itself a stop-the-world
// synchronization point that hands other goroutines a fresh chance to run
// right as the next window opens, and doing that hundreds of times over a
// run added churn rather than removing it.
func settleBackgroundGoroutines(t *testing.T) {
t.Helper()
last := goruntime.NumGoroutine()
stable := 0
deadline := time.Now().Add(2 * time.Second)
for stable < 3 && time.Now().Before(deadline) {
time.Sleep(10 * time.Millisecond)
n := goruntime.NumGoroutine()
if n == last {
stable++
} else {
stable = 0
last = n
}
}
}

// retryUntilAllocsMatch takes one fresh testing.AllocsPerRun reading of every
// case in "cases" and returns as soon as all of them agree with "baseline"
// on the SAME attempt, sleeping a real 20ms between attempts otherwise (up
// to "attempts" tries) — the shape TestProfileMiddleware_RefusalWorkIndepen-
// dentOfFleet originally used inline, now shared with its sibling
// TestSelectableProfileNames_PinOutcomesDoSameWork.
//
// A round-robin MINIMUM-of-many-samples was tried first (keep, per case, the
// lowest reading seen over several back-to-back rounds) on the theory that
// process-wide noise only ever ADDS allocations. It does not work here:
// measured directly under `go test -race`, two fleets of different size can
// differ by exactly one allocation on EVERY back-to-back call, with no
// fluctuation at all across dozens of tight-loop rounds — go's race build
// disables the tiny-object allocator (objects that would normally share one
// heap slot each get their own; see runtime/malloc.go's raceenabled checks),
// and which case's measurement window absorbs the resulting off-by-one comes
// down to incidental free-list/size-class state that repeated sampling with
// no gap in between never disturbs, so the minimum converges on two
// different, both "clean," values instead of one shared value. A REAL sleep
// between attempts does let it move: confirmed by direct comparison in
// isolation under -race, requiring an exact match across every case on one
// shared attempt (this function) passes reliably where round-robin-minimum
// sampling with the identical total sleep budget fails on every run. Forcing
// this with runtime.GC() between measurements instead of sleeping was tried
// even earlier and made the flakiness worse, not better — see
// settleBackgroundGoroutines above, which handles the OTHER, independent
// noise source (a goroutine leftover from an earlier test in this package's
// shared binary) with a single settle pass before measurement starts.
func retryUntilAllocsMatch(attempts, runs int, baseline string, cases map[string]func()) map[string]float64 {
restore := goruntime.GOMAXPROCS(1)
defer goruntime.GOMAXPROCS(restore)

allocs := make(map[string]float64, len(cases))
for attempt := 0; attempt < attempts; attempt++ {
if attempt > 0 {
time.Sleep(20 * time.Millisecond)
}
for name, fn := range cases {
allocs[name] = testing.AllocsPerRun(runs, fn)
}
same := true
for name := range cases {
same = same && allocs[name] == allocs[baseline]
}
if same {
break
}
}
return allocs
}

// TestProfileMiddleware_RefusalWorkIndependentOfFleet (Spec 105 PR D codex
// round 2, finding 1): the uniform refusal must not cost work proportional to
// the number of OTHER profiles the operator has configured. A gate that
Expand All @@ -195,10 +277,12 @@ func profileGateRefusal(t *testing.T, handler http.Handler, agent *auth.AuthCont
// profiles, for every refusal branch. (HEAD before the fix: 31 allocations
// over the empty fleet, ~4 129 over the large one — 1.7 µs vs 0.9 ms at
// 10 000 profiles.) AllocsPerRun counts every goroutine's mallocs and the
// package's other tests may leave background work behind, so a reading is
// retried into a quiet window — noise only ever adds, and a fleet-
// proportional gate is off by thousands, so it can never pass. The pure
// predicate is pinned at zero allocations without any retry in
// package's other tests may leave background work behind, plus a `-race`-
// only allocator artifact independent of that (see retryUntilAllocsMatch),
// so a reading is retried into an attempt where the four fleets agree —
// noise only ever adds, and a fleet-proportional gate is off by thousands,
// so it can never pass regardless of how the noise is filtered. The pure
// predicate is pinned at zero allocations the same way in
// TestProfileIndex_SelectableAllocatesNothing.
func TestProfileMiddleware_RefusalWorkIndependentOfFleet(t *testing.T) {
fleets := profileGateFleets()
Expand All @@ -211,26 +295,19 @@ func TestProfileMiddleware_RefusalWorkIndependentOfFleet(t *testing.T) {
for _, f := range fleets {
f.srv.profileIndexes.For(f.cfg)
}
settleBackgroundGoroutines(t)

for name, c := range profileGateRefusalCases {
t.Run(name, func(t *testing.T) {
var allocs map[string]float64
for attempt := 0; attempt < 10; attempt++ {
allocs = map[string]float64{}
for fleet, f := range fleets {
handler := f.handler(next)
allocs[fleet] = testing.AllocsPerRun(20, func() { profileGateRefusal(t, handler, c.agent, c.path) })
}
same := true
for fleet := range fleets {
same = same && allocs[fleet] == allocs["no profiles"]
}
if same {
return
}
time.Sleep(20 * time.Millisecond)
cases := make(map[string]func(), len(fleets))
for fleet, f := range fleets {
handler := f.handler(next)
cases[fleet] = func() { profileGateRefusal(t, handler, c.agent, c.path) }
}
allocs := retryUntilAllocsMatch(15, 20, "no profiles", cases)
for _, got := range allocs {
require.Equal(t, allocs["no profiles"], got, "%s must allocate exactly like the empty fleet on every fleet: %v", name, allocs)
}
t.Fatalf("%s must allocate exactly like the empty fleet on every fleet: %v", name, allocs)
})
}
}
Expand Down
Loading