Skip to content

Commit

Permalink
kvserver: deflake TestFollowerReadsWithStaleDescriptor
Browse files Browse the repository at this point in the history
A preceeding change (cockroachdb#62696) introduced a flakey update to this test.
Prior to that change, this test was using 2 voting replicas but that
change tried to make it use 1 voter and 1 non-voter instead (as a litmus
test for the new syntax added in cockroachdb#62696).

The test rebalances a replica away from a node and ensures that a
historical read sent immediately afterwards gets re-routed to the
leaseholder replica, since the receiving store had its replica
destroyed. However, when we're using a non-voter in this test, that
non-voter may not have learned about this replication change by the time
it receives this historical query and that fails the assertion.

This commit re-organizes the test and fixes the flake.

Release note: None
  • Loading branch information
aayushshah15 committed Mar 31, 2021
1 parent 46a8edd commit 04c09fe
Showing 1 changed file with 26 additions and 8 deletions.
34 changes: 26 additions & 8 deletions pkg/ccl/kvccl/kvfollowerreadsccl/followerreads_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -540,8 +540,7 @@ func TestFollowerReadsWithStaleDescriptor(t *testing.T) {
n1 := sqlutils.MakeSQLRunner(tc.Conns[0])
n1.Exec(t, `CREATE DATABASE t`)
n1.Exec(t, `CREATE TABLE test (k INT PRIMARY KEY)`)
n1.Exec(t, `ALTER TABLE test EXPERIMENTAL_RELOCATE VOTERS VALUES (ARRAY[1], 1)`)
n1.Exec(t, `ALTER TABLE test EXPERIMENTAL_RELOCATE NON_VOTERS VALUES (ARRAY[2], 1)`)
n1.Exec(t, `ALTER TABLE test EXPERIMENTAL_RELOCATE VOTERS VALUES (ARRAY[1,2], 1)`)
// Speed up closing of timestamps, as we'll in order to be able to use
// follower_read_timestamp().
// Every 0.2s we'll close the timestamp from 0.4s ago. We'll attempt follower reads
Expand Down Expand Up @@ -571,12 +570,31 @@ func TestFollowerReadsWithStaleDescriptor(t *testing.T) {
require.Equal(t, roachpb.StoreID(1), entry.Lease().Replica.StoreID)
require.Equal(t, []roachpb.ReplicaDescriptor{
{NodeID: 1, StoreID: 1, ReplicaID: 1},
{NodeID: 2, StoreID: 2, ReplicaID: 2, Type: roachpb.ReplicaTypeNonVoter()},
{NodeID: 2, StoreID: 2, ReplicaID: 2},
}, entry.Desc().Replicas().Descriptors())

// Relocate the follower. n2 will no longer have a replica.
n1.Exec(t, `ALTER TABLE test EXPERIMENTAL_RELOCATE VOTERS VALUES (ARRAY[1,3], 1)`)
n1.Exec(t, `ALTER TABLE test EXPERIMENTAL_RELOCATE NON_VOTERS VALUES (ARRAY[], 1)`)
// Remove the follower and add a new non-voter to n3. n2 will no longer have a
// replica.
n1.Exec(t, `ALTER TABLE test EXPERIMENTAL_RELOCATE VOTERS VALUES (ARRAY[1], 1)`)
n1.Exec(t, `ALTER TABLE test EXPERIMENTAL_RELOCATE NON_VOTERS VALUES (ARRAY[3], 1)`)

// Wait until the new non-voter is upreplicated to n3.
testutils.SucceedsSoon(
t, func() error {
return tc.Server(2).GetStores().(*kvserver.Stores).VisitStores(
func(s *kvserver.Store) error {
repl := s.LookupReplica(tablePrefix)
if repl == nil {
return errors.Errorf("no replica found on store %s", s)
}
if !repl.IsInitialized() {
return errors.Errorf("non-voter not initialized")
}
return nil
},
)
},
)

// Execute the query again and assert the cache is updated. This query will
// not be executed as a follower read since it attempts to use n2 which
Expand All @@ -585,15 +603,15 @@ func TestFollowerReadsWithStaleDescriptor(t *testing.T) {
n4.Exec(t, historicalQuery)
// As a sanity check, verify that this was not a follower read.
rec := <-recCh
require.False(t, kv.OnlyFollowerReads(rec), "query was not served through follower reads: %s", rec)
require.False(t, kv.OnlyFollowerReads(rec), "query was served through follower reads: %s", rec)
// Check that the cache was properly updated.
entry = n4Cache.GetCached(ctx, tablePrefix, false /* inverted */)
require.NotNil(t, entry)
require.False(t, entry.Lease().Empty())
require.Equal(t, roachpb.StoreID(1), entry.Lease().Replica.StoreID)
require.Equal(t, []roachpb.ReplicaDescriptor{
{NodeID: 1, StoreID: 1, ReplicaID: 1},
{NodeID: 3, StoreID: 3, ReplicaID: 3},
{NodeID: 3, StoreID: 3, ReplicaID: 3, Type: roachpb.ReplicaTypeNonVoter()},
}, entry.Desc().Replicas().Descriptors())

// Make a note of the follower reads metric on n3. We'll check that it was
Expand Down

0 comments on commit 04c09fe

Please sign in to comment.