Merge #158625

158625: ccl/multiregionccl: stabilize global_tables follower-read checks r=spilchen a=spilchen

Follower read assertions in global_tables tests were flaky due to relying on AS OF SYSTEM TIME now(), where the local replica's closed timestamp hadn't yet advanced enough. This caused transactions to exceed their uncertainty window when closed timestamp hadn't been bumped recently.

The intent of the test is to ensure that queries run as of now can use follower reads. Since timing can interphere, this added retry logic to try the query again if the trace didn't match expectations.

Closes #152099
Release note: None
Epic: none

Co-authored-by: Matt Spilchen <matt.spilchen@cockroachlabs.com>

Author: craig[bot]

pkg/ccl/multiregionccl/datadriven_test.go

@@ -261,10 +261,13 @@ SET CLUSTER SETTING kv.allocator.min_lease_transfer_interval = '5m'
 
 			case "trace-sql":
 				mustHaveArgOrFatal(t, d, serverIdx)
+				var idx int
+				d.ScanArgs(t, serverIdx, &idx)
+				shouldRetry := d.HasArg(traceSQLRetryArg)
+				retryTimeout := testutils.DefaultSucceedsSoonDuration
+
 				var rec tracingpb.Recording
 				queryFunc := func() (localRead bool, followerRead bool, err error) {
-					var idx int
-					d.ScanArgs(t, serverIdx, &idx)
 					sqlDB, err := ds.getSQLConn(idx)
 					if err != nil {
 						return false, false, err
@@ -291,22 +294,41 @@ SET CLUSTER SETTING kv.allocator.min_lease_transfer_interval = '5m'
 					}
 					return localRead, followerRead, nil
 				}
-				localRead, followerRead, err := queryFunc()
-				if err != nil {
-					return err.Error()
-				}
-				var output strings.Builder
-				output.WriteString(
-					fmt.Sprintf("served locally: %s\n", strconv.FormatBool(localRead)))
-				// Only print follower read information if the query was served locally.
-				if localRead {
+
+				runOnce := func() (string, error) {
+					localRead, followerRead, err := queryFunc()
+					if err != nil {
+						return "", err
+					}
+					var output strings.Builder
 					output.WriteString(
-						fmt.Sprintf("served via follower read: %s\n", strconv.FormatBool(followerRead)))
+						fmt.Sprintf("served locally: %s\n", strconv.FormatBool(localRead)))
+					// Only print follower read information if the query was served locally.
+					if localRead {
+						output.WriteString(
+							fmt.Sprintf("served via follower read: %s\n", strconv.FormatBool(followerRead)))
+					}
+					if d.Expected != output.String() {
+						return "", errors.AssertionFailedf("not a match, trace:\n%s\n", rec)
+					}
+					return output.String(), nil
+				}
+
+				var output string
+				var err error
+				if shouldRetry {
+					err = testutils.SucceedsWithinError(func() error {
+						var attemptErr error
+						output, attemptErr = runOnce()
+						return attemptErr
+					}, retryTimeout)
+				} else {
+					output, err = runOnce()
 				}
-				if d.Expected != output.String() {
-					return errors.AssertionFailedf("not a match, trace:\n%s\n", rec).Error()
+				if err != nil {
+					return err.Error()
 				}
-				return output.String()
+				return output
 
 			case "refresh-range-descriptor-cache":
 				mustHaveArgOrFatal(t, d, tableName)
@@ -484,6 +506,7 @@ const (
 	partitionName    = "partition-name"
 	numVoters        = "num-voters"
 	numNonVoters     = "num-non-voters"
+	traceSQLRetryArg = "retry"
 )
 
 type replicaType int

pkg/ccl/multiregionccl/testdata/global_tables

@@ -32,7 +32,7 @@ SELECT * FROM db.global WHERE k = 2
 ----
 LEAD_FOR_GLOBAL_READS
 
-trace-sql idx=1
+trace-sql idx=1 retry
 SELECT * FROM db.global WHERE k = 1
 ----
 served locally: true
@@ -43,7 +43,7 @@ SELECT * FROM db.global WHERE k = 2
 ----
 LEAD_FOR_GLOBAL_READS
 
-trace-sql idx=2
+trace-sql idx=2 retry
 SELECT * FROM db.global WHERE k = 1
 ----
 served locally: true
@@ -54,7 +54,7 @@ SELECT * FROM db.global WHERE k = 2
 ----
 LEAD_FOR_GLOBAL_READS
 
-trace-sql idx=3
+trace-sql idx=3 retry
 SELECT * FROM db.global WHERE k = 1
 ----
 served locally: true
@@ -65,7 +65,7 @@ SELECT * FROM db.global WHERE k = 2
 ----
 LEAD_FOR_GLOBAL_READS
 
-trace-sql idx=4
+trace-sql idx=4 retry
 SELECT * FROM db.global WHERE k = 1
 ----
 served locally: true
@@ -95,7 +95,7 @@ SELECT * FROM db.global WHERE k = 2
 ----
 LEAD_FOR_GLOBAL_READS
 
-trace-sql idx=5
+trace-sql idx=5 retry
 SELECT * FROM db.global WHERE k = 1
 ----
 served locally: true