test: stop four concurrency tests from failing on a busy machine

All four pass on a quiet box and fail on a loaded one, each for its own
reason. None of them is testing the clock, so none of them should be
failing on it.

- ollama_cloud_usage_test.go: the second caller was released with
  `close(release)` right after its goroutine was started, not after it
  had reached the singleflight group. When the first refresh won that
  race, the second became a new singleflight execution, re-read the
  account, saw the LastAttemptAt the first one had just written, and
  came back with the 30-second manual-refresh 429 at line 685. It now
  counts account loads and waits for the second caller's own load,
  which happens right before it joins the group. Adds a counting
  GetByID to the test repo.

- gateway_hotpath_optimization_test.go: a 20ms sleep was meant to let
  all 12 callers reach the cache before the loader was released. A
  caller that arrived after the load had finished got a hit, not a
  miss, so the miss count came out 11 of 12. It now waits on the miss
  counter itself, which is the value the test asserts on.

- token_refresh_pool_health_test.go: the floor was `configuredSpacing`
  minus 10ms, i.e. 40ms out of 50ms. Each start timestamp is taken
  after the rate gate releases the goroutine, so scheduler delay can
  compress one observed gap with the gate behaving correctly — seen at
  37ms and again at 13ms. The floor is now a tenth of the configured
  spacing. Measured with providerQPS=20, 8 attempts, concurrency 2:
  gate at 50ms gives a minimum gap of 49.97ms, gate at 0 gives 22µs.
  So an unpaced gate sits three orders of magnitude under the 5ms floor
  and is still caught, while jitter has room to move. A comment warns
  against replacing this with an assertion on the total span of the
  starts: the span is set by how long each attempt takes under the
  concurrency limit, not by the gate — 471ms paced against 241ms
  unpaced — so a span check passes with the gate disabled.

- prompt_guard_test.go: the bound only has to show the failover shared
  the first endpoint's 70ms deadline and did not take the second
  endpoint's own 500ms one. An unshared deadline lands near 535ms, so
  350ms still fails loudly (seen: 224ms against a 180ms bound).

Tests only; no product code is touched. Each fix was checked in both
directions: it passes with the behaviour intact, and it still fails when
the behaviour is broken on purpose (for the QPS one, by swapping the
shared rate gate for a zero-interval one).
This commit is contained in:
Nick
2026-07-25 21:59:20 +07:00
parent 2730c1c43b
commit 291a737422
4 changed files with 58 additions and 3 deletions
@@ -222,7 +222,12 @@ func TestGuardEvaluatorFlagSharedDeadlineFailClosedAndContextCancel(t *testing.T
elapsed := time.Since(started)
require.Error(t, err)
require.Equal(t, 2, calls)
require.Less(t, elapsed, 180*time.Millisecond)
// The bound only has to prove the failover shared the first endpoint's
// 70ms deadline instead of taking the second endpoint's own 500ms one.
// An unshared deadline lands at ~535ms, so 350ms still fails loudly
// while leaving room for scheduler delay on a busy CI machine. A
// tighter bound made this test flaky, not stricter.
require.Less(t, elapsed, 350*time.Millisecond)
require.GreaterOrEqual(t, elapsed, 50*time.Millisecond)
require.Equal(t, int64(1), metrics.Snapshot().Failovers)
require.Equal(t, int64(1), metrics.Snapshot().Timeouts)
@@ -219,7 +219,15 @@ func TestGetUserGroupRateMultiplier_UsesCacheAndSingleflight(t *testing.T) {
}
close(start)
time.Sleep(20 * time.Millisecond)
// Wait for every caller to have recorded its cache miss before releasing the
// loader. A fixed sleep raced here: a goroutine that reached the cache after
// the singleflight load had already finished got a hit instead of a miss, and
// the miss assertion below saw 11 of 12. The miss counter is the observable
// that says "all callers are now inside the singleflight group".
require.Eventually(t, func() bool {
_, miss, _, _, _ := GatewayUserGroupRateCacheStats()
return miss == int64(concurrent)
}, 5*time.Second, time.Millisecond, "all callers must miss the cache before the loader is released")
close(unblock)
wg.Wait()
@@ -36,6 +36,14 @@ type ollamaUsageTestRepo struct {
disableAutoAttempts atomic.Int64
disableAutoCalls atomic.Int64
groupResolveCalls atomic.Int64
getByIDCalls atomic.Int64
}
// GetByID counts loads so a test can wait for a caller to reach the point just
// before the singleflight group, instead of guessing with a sleep.
func (r *ollamaUsageTestRepo) GetByID(ctx context.Context, id int64) (*Account, error) {
r.getByIDCalls.Add(1)
return r.upstreamBillingProbeAccountRepo.GetByID(ctx, id)
}
func (r *ollamaUsageTestRepo) ListOllamaCloudUsageGroupAccounts(_ context.Context, anchors []*Account) ([]Account, error) {
@@ -805,7 +813,19 @@ func TestOllamaCloudUsageRefreshSingleflightAndRunnerDeduplicateSharedGroup(t *t
errs := make(chan error, 2)
go func() { _, err := svc.Refresh(context.Background(), first.ID); errs <- err }()
<-started
// The first caller is now parked in the stub, having loaded the account twice
// (once to build the group key, once inside the singleflight function).
loadsBeforeSecond := repo.getByIDCalls.Load()
go func() { _, err := svc.Refresh(context.Background(), second.ID); errs <- err }()
// Only release the first caller once the second one has loaded its own
// account, which happens immediately before it joins the singleflight group.
// Releasing right after starting the goroutine raced: if the first refresh
// finished first, the second became a fresh singleflight execution, re-read
// the account, saw the LastAttemptAt just written, and failed with the 30s
// manual-refresh 429 instead of sharing the in-flight result.
require.Eventually(t, func() bool {
return repo.getByIDCalls.Load() > loadsBeforeSecond
}, 5*time.Second, time.Millisecond, "the second caller must reach the singleflight group before the first is released")
close(release)
require.NoError(t, <-errs)
require.NoError(t, <-errs)
@@ -630,7 +630,29 @@ func TestTokenRefreshService_SaturatedProviderPreservesConcurrencyAndActualQPSSt
starts := refresher.startsSnapshot()
require.Len(t, starts, attemptCount)
configuredSpacing := time.Second / time.Duration(providerQPS)
minimumObservedSpacing := configuredSpacing - 10*time.Millisecond
// The floor is a tenth of the configured spacing, not spacing minus a few
// milliseconds. Each start timestamp is taken after the rate gate releases
// the goroutine, so scheduler delay can compress one observed gap without
// the gate having done anything wrong: this assertion failed on a loaded
// machine at 13ms and again at 37ms against a 40ms floor, both times with
// the gate pacing correctly.
//
// A tenth still separates the two cases by a wide margin. Measured with
// providerQPS=20 (50ms spacing), 8 attempts, providerConcurrency=2:
//
// gate at 50ms -> minimum observed gap 49.97ms
// gate at 0 -> minimum observed gap 22µs
//
// So an unpaced gate lands three orders of magnitude below the 5ms floor
// and is still caught, while jitter on a busy machine has room to move.
//
// Do not swap this for an assertion on the total span of the starts: the
// span is dominated by how long each attempt takes under
// providerConcurrency, not by the gate. Same measurement — 471ms paced
// against 241ms unpaced — so a span check passes with the gate disabled and
// tests nothing.
minimumObservedSpacing := configuredSpacing / 10
actualMinimumSpacing := starts[1].Sub(starts[0])
for i := 1; i < len(starts); i++ {
spacing := starts[i].Sub(starts[i-1])