Skip to content

Commit 7c168dd

Browse files
craig[bot]pav-kv
andcommitted
Merge #157012
157012: kvserver: deflake TestRestoreReplicas r=tbg a=pav-kv There is no need to expect the exact location of the leaseholder in this test. Make it robust to lease movements. Resolves #157001 Co-authored-by: Pavel Kalinnikov <pavel@cockroachlabs.com>
2 parents fda4ace + d055acf commit 7c168dd

File tree

1 file changed

+18
-22
lines changed

1 file changed

+18
-22
lines changed

pkg/kv/kvserver/client_raft_test.go

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -392,40 +392,36 @@ func TestRestoreReplicas(t *testing.T) {
392392

393393
require.NoError(t, tc.Restart())
394394

395-
// Find the leaseholder and follower. The restart may cause the Raft
396-
// leadership to bounce around a bit, since we don't fully enable Raft
397-
// prevote, so we loop for a bit until we find the leaseholder.
395+
// The restart may cause the Raft leadership and the lease to bounce around a
396+
// bit, so we loop until we find the leaseholder and able to propose the
397+
// second increment.
398398
incArgs = incrementArgs(key, 5)
399-
var followerStore *kvserver.Store
400399
testutils.SucceedsSoon(t, func() error {
401400
var pErr *kvpb.Error
402401
for i := 0; i < tc.NumServers(); i++ {
403402
_, pErr = kv.SendWrapped(ctx, tc.GetFirstStoreFromServer(t, i).TestSender(), incArgs)
404403
if pErr == nil {
405-
followerStore = tc.GetFirstStoreFromServer(t, 1-i)
406-
break
404+
return nil
407405
}
408406
require.IsType(t, &kvpb.NotLeaseHolderError{}, pErr.GetDetail())
409407
}
410408
return pErr.GoError()
411409
})
412410

413-
// The follower should now return a NLHE.
414-
_, pErr = kv.SendWrapped(ctx, followerStore.TestSender(), incArgs)
415-
require.Error(t, pErr.GoError())
416-
require.IsType(t, &kvpb.NotLeaseHolderError{}, pErr.GetDetail())
417-
418-
testutils.SucceedsSoon(t, func() error {
419-
getArgs := getArgs(key)
420-
if reply, err := kv.SendWrappedWith(ctx, followerStore.TestSender(), kvpb.Header{
421-
ReadConsistency: kvpb.INCONSISTENT,
422-
}, getArgs); err != nil {
423-
return errors.Errorf("failed to read data: %s", err)
424-
} else if e, v := int64(28), mustGetInt(reply.(*kvpb.GetResponse).Value); v != e {
425-
return errors.Errorf("failed to read correct data: expected %d, got %d", e, v)
426-
}
427-
return nil
428-
})
411+
// Both servers should eventually observe the new value.
412+
for i := 0; i < tc.NumServers(); i++ {
413+
testutils.SucceedsSoon(t, func() error {
414+
getArgs := getArgs(key)
415+
if reply, err := kv.SendWrappedWith(ctx, tc.GetFirstStoreFromServer(t, i).TestSender(), kvpb.Header{
416+
ReadConsistency: kvpb.INCONSISTENT,
417+
}, getArgs); err != nil {
418+
return errors.Errorf("failed to read data: %s", err)
419+
} else if e, v := int64(28), mustGetInt(reply.(*kvpb.GetResponse).Value); v != e {
420+
return errors.Errorf("failed to read correct data: expected %d, got %d", e, v)
421+
}
422+
return nil
423+
})
424+
}
429425

430426
validate := func(s *kvserver.Store) {
431427
repl := s.LookupReplica(key)

0 commit comments

Comments
 (0)