Skip to content

test(server): fix the reach-rank expired-snapshot round deadline flake (#237) #269

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dborup merged 1 commit into from
Oct 6, 2026
Merged

test(server): fix the reach-rank expired-snapshot round deadline flake (#237) #269

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 19 additions & 1 deletion cmd/server/reach_rank_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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)
}
}()
}
Expand All @@ -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
Expand Down
Loading