From cfa4e0a585ffd14c4b18085f5e9efa55cc93aba1 Mon Sep 17 00:00:00 2001 From: dborup Date: Mon, 5 Oct 2026 18:50:56 +0200 Subject: [PATCH] test(server): stop the reach-rank hammer loop from starving its own refresh TestReachRank_ExpiredSnapshotAlwaysRefreshes hammers an expired snapshot from 16 goroutines with no pause between requests. On a loaded host those 16 take every available P from the one goroutine the round is waiting for, the single singleflight refresh, and the delay lands inside loadDegreeSnapshot. A round then misses its 3 s deadline with nothing wrong in production (#237). Each caller now pauses 100 us between requests. Every round still has 16 concurrent callers hitting the stale snapshot continuously; the test just stops issuing ~530k requests per process. The deadline is unchanged. Also takes the test-hardening item from the #234 review that belongs to this test: awaitDegreeRefresh moves into t.Cleanup, so the join also runs when a round assertion fails and a failing run cannot leave a stray refresh behind for the next test. Cleanup is LIFO, so it runs before resetReachState's. Co-Authored-By: Claude Opus 5 --- cmd/server/reach_rank_test.go | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/cmd/server/reach_rank_test.go b/cmd/server/reach_rank_test.go index b8cb8cef3..fe508cc55 100644 --- a/cmd/server/reach_rank_test.go +++ b/cmd/server/reach_rank_test.go @@ -929,6 +929,10 @@ func TestReachRank_ExpiredSnapshotAlwaysRefreshes(t *testing.T) { a, b := pk64("a1"), pk64("b2") db := newReachRankDB(t, []rankTestNode{{a, "A"}, {b, "B"}}, nil, star(a, b)) srv := newReachRankServer(t, db, &Config{}) + // LIFO: this join runs before resetReachState's cleanup clears the + // snapshot, and unlike a tail call it also runs when a round below fails, + // so a failing run cannot leave a stray refresh behind (#234 review). + t.Cleanup(func() { awaitDegreeRefresh(t, srv) }) if _, err := srv.getDegreeSnapshot(context.Background()); err != nil { t.Fatal(err) } @@ -945,6 +949,7 @@ func TestReachRank_ExpiredSnapshotAlwaysRefreshes(t *testing.T) { defer wg.Done() for publishedSnap(srv) == expired && time.Now().Before(deadline) { srv.getDegreeSnapshot(context.Background()) + time.Sleep(degreeHammerPause) } }() } @@ -953,9 +958,22 @@ func TestReachRank_ExpiredSnapshotAlwaysRefreshes(t *testing.T) { t.Fatalf("round %d: expired snapshot never refreshed", round) } } - awaitDegreeRefresh(t, srv) } +// degreeHammerPause is how long each caller in +// TestReachRank_ExpiredSnapshotAlwaysRefreshes waits between requests. Without +// it the 16 callers spin, and on a loaded host they take every available P +// from the one goroutine the round is actually waiting for: the singleflight +// refresh. The delay then lands inside loadDegreeSnapshot (measured: a 276 ms +// round, 272 ms of it in the load, with all 16 callers parked on the +// singleflight group mutex and the refresh parked mid-query), and a round can +// miss its deadline with nothing wrong in production (#237). The pause leaves +// 16 callers hitting the stale snapshot continuously in every round — ~30k +// requests per process, over 100 per round — while cutting the round-duration +// tail several-fold, so the deadline measures the liveness property and not +// the test's own CPU contention. +const degreeHammerPause = 100 * time.Microsecond + // awaitDegreeRefresh waits for the singleflight snapshot refresh a test may // have left running (#224). A caller that read the expired snapshot just // before the refresh published can register one more refresh before the