Skip to content

Commit

Permalink
kv: deflake TestFollowerReadsWithStaleDescriptor
Browse files Browse the repository at this point in the history
Fixes #108087.

This fix avoids a data race in the test. The race was harmless, but
could cause the test to fail when run with the race detector.

Release note: None
  • Loading branch information
nvanbenschoten committed Aug 3, 2023
1 parent ab13de5 commit 6db50f7
Showing 1 changed file with 8 additions and 6 deletions.
14 changes: 8 additions & 6 deletions pkg/ccl/kvccl/kvfollowerreadsccl/followerreads_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"fmt"
"math"
"strings"
"sync/atomic"
"testing"
"time"

Expand Down Expand Up @@ -697,7 +698,8 @@ func TestFollowerReadsWithStaleDescriptor(t *testing.T) {
// The test uses follower_read_timestamp().
defer utilccl.TestingEnableEnterprise()()

historicalQuery := `SELECT * FROM test AS OF SYSTEM TIME follower_read_timestamp() WHERE k=2`
var historicalQuery atomic.Value
historicalQuery.Store(`SELECT * FROM test AS OF SYSTEM TIME follower_read_timestamp() WHERE k=2`)
recCh := make(chan tracingpb.Recording, 1)

tc := testcluster.StartTestCluster(t, 4,
Expand Down Expand Up @@ -732,7 +734,7 @@ func TestFollowerReadsWithStaleDescriptor(t *testing.T) {
},
SQLExecutor: &sql.ExecutorTestingKnobs{
WithStatementTrace: func(trace tracingpb.Recording, stmt string) {
if stmt == historicalQuery {
if stmt == historicalQuery.Load().(string) {
recCh <- trace
}
},
Expand Down Expand Up @@ -786,7 +788,7 @@ func TestFollowerReadsWithStaleDescriptor(t *testing.T) {
// not be executed as a follower read since it attempts to use n2 which
// doesn't have a replica any more and then it tries n1 which returns an
// updated descriptor.
n4.Exec(t, historicalQuery)
n4.Exec(t, historicalQuery.Load().(string))
// As a sanity check, verify that this was not a follower read.
rec := <-recCh
require.False(t, kv.OnlyFollowerReads(rec), "query was served through follower reads: %s", rec)
Expand All @@ -812,7 +814,7 @@ func TestFollowerReadsWithStaleDescriptor(t *testing.T) {
// Run a historical query and assert that it's served from the follower (n3).
// n4 should attempt to route to n3 because we pretend n3 has a lower latency
// (see testing knob).
n4.Exec(t, historicalQuery)
n4.Exec(t, historicalQuery.Load().(string))
rec = <-recCh

// Look at the trace and check that we've served a follower read.
Expand Down Expand Up @@ -855,8 +857,8 @@ func TestFollowerReadsWithStaleDescriptor(t *testing.T) {
// the ReplicaInfo twice for the same range. This allows us to verify that
// the cached - in the spanResolverIterator - information is correctly
// preserved.
historicalQuery = `SELECT * FROM [SELECT * FROM test WHERE k=2 UNION ALL SELECT * FROM test WHERE k=3] AS OF SYSTEM TIME follower_read_timestamp()`
n4.Exec(t, historicalQuery)
historicalQuery.Store(`SELECT * FROM [SELECT * FROM test WHERE k=2 UNION ALL SELECT * FROM test WHERE k=3] AS OF SYSTEM TIME follower_read_timestamp()`)
n4.Exec(t, historicalQuery.Load().(string))
rec = <-recCh

// Sanity check that the plan was distributed.
Expand Down

0 comments on commit 6db50f7

Please sign in to comment.