Skip to content

Commit

Permalink
storage: deflake TestStoreRangeRemoveDead
Browse files Browse the repository at this point in the history
This took me longer to track down than I'm comfortable admitting. The
test wasn't properly checking for full upreplication because it was only
looking at the first range. It still is, but that's now all the ranges
-- it's finicky to upgrade the check.

Fixes #33497.
Closes #33486.

Release note: None
  • Loading branch information
tbg committed Jan 9, 2019
1 parent cff3e88 commit fe2156e
Showing 1 changed file with 22 additions and 2 deletions.
24 changes: 22 additions & 2 deletions pkg/storage/client_raft_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3012,7 +3012,12 @@ func TestStoreRangeRemoveDead(t *testing.T) {
defer leaktest.AfterTest(t)()
sc := storage.TestStoreConfig(nil)
sc.TestingKnobs.DisableReplicaRebalancing = true
mtc := &multiTestContext{storeConfig: &sc}
mtc := &multiTestContext{
storeConfig: &sc,
// This test needs to verify full replication, but as written the check
// only works if there's only one range initially.
startWithSingleRange: true,
}

storage.TimeUntilStoreDead.Override(&sc.Settings.SV, storage.TestTimeUntilStoreDead)
// Disable declined and failed reservations. Their default values are on
Expand Down Expand Up @@ -3066,13 +3071,26 @@ func TestStoreRangeRemoveDead(t *testing.T) {
panic(err)
}
}

case <-stopper.ShouldQuiesce():
return
}
}
})
stopper.RunWorker(ctx, func(ctx context.Context) {
tickerDur := storage.TestTimeUntilStoreDead / 2
ticker := time.NewTicker(tickerDur)
defer ticker.Stop()

for {
select {
case <-ticker.C:
// Force the repair queues on all alive stores to run.
for _, s := range nonDeadStores {
if err := s.ForceReplicationScanAndProcess(); err != nil {
panic(err)
}
}

case <-stopper.ShouldQuiesce():
return
}
Expand All @@ -3081,6 +3099,8 @@ func TestStoreRangeRemoveDead(t *testing.T) {

// Wait for up-replication, including at least one replica on s2 (deadStore).
testutils.SucceedsSoon(t, func() error {
// NB: this only checks the first range, so it'll give false positives
// if there's more than one (see the note at the beginning of the test).
replicas := getRangeMetadata(roachpb.RKeyMin, mtc, t).Replicas
var found bool
for _, replica := range replicas {
Expand Down

0 comments on commit fe2156e

Please sign in to comment.