Merge cockroachdb#113744

113744: kvserver: deflake `TestReplicaCircuitBreaker_ExemptRequests` r=erikgrinaker a=erikgrinaker **kvserver: add `Replica.NumPendingProposals()` test helper** **kvserver: deflake `TestReplicaCircuitBreaker_ExemptRequests`** It was possible for a reproposal from a previous subtest to cause the circuit breaker to trip again when remove quorum. This would violate assertions that were expecting a different command to fail. This patch waits for all pending proposals to complete before tripping the circuit breaker again. Resolves cockroachdb#112073. Epic: none Release note: None Co-authored-by: Erik Grinaker <[email protected]>
DarrylWong · Nov 3, 2023 · 9e439be · 9e439be
2 parents 354b78c + b81abba
commit 9e439be
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 1 deletion.
diff --git a/pkg/kv/kvserver/client_replica_circuit_breaker_test.go b/pkg/kv/kvserver/client_replica_circuit_breaker_test.go
@@ -673,10 +673,12 @@ func TestReplicaCircuitBreaker_ExemptRequests(t *testing.T) {
 		})
 	}
 
-	// Restore the breaker via the probe.
+	// Restore the breaker via the probe, and wait for any pending (re)proposals
+	// from previous tests to be flushed.
 	resumeHeartbeats()
 	tc.SetProbeEnabled(n1, true)
 	tc.UntripsSoon(t, tc.Write, n1)
+	tc.WaitForProposals(t, n1)
 
 	// Lose quorum (liveness stays intact).
 	tc.SetSlowThreshold(10 * time.Millisecond)
@@ -886,6 +888,19 @@ func (cbt *circuitBreakerTest) UntripsSoon(t *testing.T, method func(idx int) er
 	})
 }
 
+func (cbt *circuitBreakerTest) WaitForProposals(t *testing.T, idx int) {
+	t.Helper()
+	testutils.SucceedsSoon(t, func() error {
+		t.Helper()
+
+		repl := cbt.repls[idx].Replica
+		if n := repl.NumPendingProposals(); n > 0 {
+			return errors.Errorf("%d pending proposals", n)
+		}
+		return nil
+	})
+}
+
 func (cbt *circuitBreakerTest) ExpireAllLeasesAndN1LivenessRecord(
 	t *testing.T, pauseHeartbeats bool,
 ) (undo func()) {

diff --git a/pkg/kv/kvserver/helpers_test.go b/pkg/kv/kvserver/helpers_test.go
@@ -400,6 +400,12 @@ func (r *Replica) QuotaReleaseQueueLen() int {
 	return len(r.mu.quotaReleaseQueue)
 }
 
+func (r *Replica) NumPendingProposals() int {
+	r.mu.RLock()
+	defer r.mu.RUnlock()
+	return r.numPendingProposalsRLocked()
+}
+
 func (r *Replica) IsFollowerActiveSince(
 	ctx context.Context, followerID roachpb.ReplicaID, threshold time.Duration,
 ) bool {