Skip to content

Commit 8d9ec35

Browse files
committed
kvserver: deflake TestReplicaCircuitBreaker_Partial_Retry
The test was relying on global metric counters to assert the behavior of a single request. This was error prone, as we do not control what all other requests may be doing. Instead, just grep the trace. Closes #156284.
1 parent 6b4ed18 commit 8d9ec35

File tree

2 files changed

+7
-6
lines changed

2 files changed

+7
-6
lines changed

pkg/kv/kvclient/kvcoord/dist_sender.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,9 @@ const (
370370
// The maximum number of times a replica is retried when it repeatedly returns
371371
// stale lease info.
372372
sameReplicaRetryLimit = 10
373+
// InLeaseTransferBackoffTraceMessage is traced when DistSender backs off as a
374+
// result of a NotLeaseholderError. It is exported for testing.
375+
InLeaseTransferBackoffTraceMessage = "backing off due to NotLeaseHolderErr with stale info"
373376
)
374377

375378
var rangeDescriptorCacheSize = settings.RegisterIntSetting(
@@ -3135,8 +3138,8 @@ func (ds *DistSender) sendToReplicas(
31353138
if shouldBackoff {
31363139
ds.metrics.InLeaseTransferBackoffs.Inc(1)
31373140
log.VErrEventf(ctx, 2,
3138-
"backing off due to NotLeaseHolderErr with stale info "+
3139-
"(updatedLH=%t intentionallySentToFollower=%t leaseholderUnavailable=%t)",
3141+
InLeaseTransferBackoffTraceMessage+
3142+
" (updatedLH=%t intentionallySentToFollower=%t leaseholderUnavailable=%t)",
31403143
updatedLeaseholder, intentionallySentToFollower, leaseholderUnavailable)
31413144
} else {
31423145
inTransferRetry.Reset() // The following Next() call will not block.

pkg/kv/kvserver/client_replica_circuit_breaker_test.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -863,7 +863,6 @@ func TestReplicaCircuitBreaker_Partial_Retry(t *testing.T) {
863863
requireRUEs := func(t *testing.T, dbs []*kv.DB) {
864864
t.Helper()
865865
for _, db := range dbs {
866-
backoffMetric := (db.NonTransactionalSender().(*kv.CrossRangeTxnWrapperSender)).Wrapped().(*kvcoord.DistSender).Metrics().InLeaseTransferBackoffs
867866
ctx, finishAndGetRec := tracing.ContextWithRecordingSpan(context.Background(), db.Tracer, "requireRUEs")
868867
defer func() {
869868
rec := finishAndGetRec()
@@ -872,13 +871,12 @@ func TestReplicaCircuitBreaker_Partial_Retry(t *testing.T) {
872871
}
873872
t.Logf("failure trace:\n%s", rec)
874873
}()
875-
initialBackoff := backoffMetric.Count()
876874
err := db.Put(ctx, key, value)
877-
// Verify that we did not perform any backoff while executing this request.
878-
require.EqualValues(t, 0, backoffMetric.Count()-initialBackoff)
879875
require.Error(t, err)
880876
require.True(t, errors.HasType(err, (*kvpb.ReplicaUnavailableError)(nil)),
881877
"expected ReplicaUnavailableError, got %v", err)
878+
// Verify that we did not perform any backoff while executing this request.
879+
require.NotContains(t, finishAndGetRec(), kvcoord.InLeaseTransferBackoffTraceMessage)
882880
}
883881
t.Logf("writes failed with ReplicaUnavailableError")
884882
}

0 commit comments

Comments
 (0)