Skip to content

Commit

Permalink
Merge #100661
Browse files Browse the repository at this point in the history
100661: kv: deflake BenchmarkMVCCGCWithForegroundTraffic r=irfansharif a=nvanbenschoten

Fixes #96266.

The test became flaky after #89632, which made it possible for single-replica tests to see the effects of pre-application raft proposal acks. This was tripping up the MVCC GC in this benchmark, leading to `request to GC non-deleted, latest value of "test"` errors.

Release note: None

Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
  • Loading branch information
craig[bot] and nvanbenschoten committed Apr 7, 2023
2 parents 8e6f530 + 319a201 commit e21779b
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 2 deletions.
7 changes: 5 additions & 2 deletions pkg/kv/kvserver/replica_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8778,7 +8778,10 @@ func BenchmarkMVCCGCWithForegroundTraffic(b *testing.B) {
tc := testContext{}
stopper := stop.NewStopper()
defer stopper.Stop(ctx)
tc.Start(ctx, b, stopper)
sc := TestStoreConfig(nil)
sc.TestingKnobs.DisableCanAckBeforeApplication = true
tc.manualClock = timeutil.NewManualTime(timeutil.Unix(0, 123))
tc.StartWithStoreConfig(ctx, b, stopper, sc)

key := roachpb.Key("test")

Expand All @@ -8790,7 +8793,7 @@ func BenchmarkMVCCGCWithForegroundTraffic(b *testing.B) {
ba.Header = header
ba.Add(args)
resp, err := tc.Sender().Send(ctx, ba)
require.Nil(b, err)
require.Nil(b, err, "%+v", err)
return resp
}

Expand Down
6 changes: 6 additions & 0 deletions pkg/kv/kvserver/store_rebalancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,12 @@ func NewStoreRebalancer(
subscribedToSpanConfigs: func() bool {
// The store rebalancer makes use of span configs. Wait until we've
// established subscription.
if rq.store.cfg.SpanConfigSubscriber == nil {
// Testing-only branch. testContext-style tests do not configure a
// SpanConfigSubscriber, so they always return false from this function.
// This has the effect of disabling the StoreRebalancer.
return false
}
return !rq.store.cfg.SpanConfigSubscriber.LastUpdated().IsEmpty()
},
}
Expand Down

0 comments on commit e21779b

Please sign in to comment.