From 0d35a0826baa25d338528ae8cf3a8275be67296f Mon Sep 17 00:00:00 2001 From: Austen McClernon Date: Mon, 13 Mar 2023 17:37:25 +0000 Subject: [PATCH] kvserver: deflake test store capacity after split This commit deflakes `TestStoreCapacityAfterSplit`. Previously it was possible for the replica load stats which underpins Capacity to be reset. The reset caused the recording duration to fall short of min stats duration, which led to a 0 value being reported for writes in store capacity. This commit bumps the manual clock twice and removes redundant leaseholder checks within a retry loop. The combination of these two changes makes the test much less likely to flake. The test is now unskipped. ``` dev test pkg/kv/kvserver -f TestStoreCapacityAfterSplit -v --stress ... 4410 runs so far, 0 failures, over 6m10s ``` Resolves: #92677 Release note: None --- pkg/kv/kvserver/client_split_test.go | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/pkg/kv/kvserver/client_split_test.go b/pkg/kv/kvserver/client_split_test.go index 1110d441ceb2..614f5ea608fd 100644 --- a/pkg/kv/kvserver/client_split_test.go +++ b/pkg/kv/kvserver/client_split_test.go @@ -2776,7 +2776,6 @@ func TestTxnWaitQueueDependencyCycleWithRangeSplit(t *testing.T) { func TestStoreCapacityAfterSplit(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) - skip.WithIssue(t, 92677) ctx := context.Background() manualClock := hlc.NewHybridManualClock() tc := testcluster.StartTestCluster(t, 2, @@ -2800,16 +2799,6 @@ func TestStoreCapacityAfterSplit(t *testing.T) { key := tc.ScratchRange(t) desc := tc.AddVotersOrFatal(t, key, tc.Target(1)) tc.TransferRangeLeaseOrFatal(t, desc, tc.Target(1)) - testutils.SucceedsSoon(t, func() error { - repl, err := s.GetReplica(desc.RangeID) - if err != nil { - return err - } - if !repl.OwnsValidLease(ctx, tc.Servers[1].Clock().NowAsClockTimestamp()) { - return errors.New("s2 does not own valid lease for this range") - } - return nil - }) tc.IncrClockForLeaseUpgrade(t, manualClock) tc.WaitForLeaseUpgrade(ctx, t, desc) @@ -2851,6 +2840,11 @@ func TestStoreCapacityAfterSplit(t *testing.T) { return nil }) + // Bump the clock again, right before calling capacity. We know that the + // writes have succeeded and should be reflected in Capacity, however the + // MinStatsDuration will cause nothing to be returned unless the last lease + // transfer is at least MinStatsDuration ago. + manualClock.Increment(int64(replicastats.MinStatsDuration)) cap, err = s.Capacity(ctx, false /* useCached */) if err != nil { t.Fatal(err) @@ -2865,7 +2859,7 @@ func TestStoreCapacityAfterSplit(t *testing.T) { // NB: The writes per second may be within some error bound below the // minExpected due to timing and floating point calculation. An error of // 0.01 (WPS) is added to avoid flaking the test. - if minExpected, a := 1/float64(replicastats.MinStatsDuration/time.Second), cap.WritesPerSecond; minExpected > a+0.01 { + if minExpected, a := 1/(float64(2*replicastats.MinStatsDuration/time.Second)), cap.WritesPerSecond; minExpected > a+0.01 { t.Errorf("expected cap.WritesPerSecond >= %f, got %f", minExpected, a) }