From 16fb58be26c0fbea898dbb8f399e42e0a30fe077 Mon Sep 17 00:00:00 2001
From: Andrew Werner <ajwerner@cockroachlabs.com>
Date: Fri, 15 Mar 2019 12:55:05 -0400
Subject: [PATCH] storage: fix TestRangeInfo flake and re-enable follower reads
 by default

This PR addresses a test flake introduced by enabling follower reads in
conjunction with #35130 which makes follower reads more generally possible
in the face of lease transfer.

Fixes #35758.

Release note: None
---
 docs/generated/settings/settings.html | 2 +-
 pkg/storage/client_replica_test.go    | 7 +++++--
 pkg/storage/replica_follower_read.go  | 2 +-
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/docs/generated/settings/settings.html b/docs/generated/settings/settings.html
index a7fc2552b0f6..63f8c48759e9 100644
--- a/docs/generated/settings/settings.html
+++ b/docs/generated/settings/settings.html
@@ -33,7 +33,7 @@
 <tr><td><code>kv.bulk_io_write.max_rate</code></td><td>byte size</td><td><code>8.0 EiB</code></td><td>the rate limit (bytes/sec) to use for writes to disk on behalf of bulk io ops</td></tr>
 <tr><td><code>kv.bulk_sst.sync_size</code></td><td>byte size</td><td><code>2.0 MiB</code></td><td>threshold after which non-Rocks SST writes must fsync (0 disables)</td></tr>
 <tr><td><code>kv.closed_timestamp.close_fraction</code></td><td>float</td><td><code>0.2</code></td><td>fraction of closed timestamp target duration specifying how frequently the closed timestamp is advanced</td></tr>
-<tr><td><code>kv.closed_timestamp.follower_reads_enabled</code></td><td>boolean</td><td><code>false</code></td><td>allow (all) replicas to serve consistent historical reads based on closed timestamp information</td></tr>
+<tr><td><code>kv.closed_timestamp.follower_reads_enabled</code></td><td>boolean</td><td><code>true</code></td><td>allow (all) replicas to serve consistent historical reads based on closed timestamp information</td></tr>
 <tr><td><code>kv.closed_timestamp.target_duration</code></td><td>duration</td><td><code>30s</code></td><td>if nonzero, attempt to provide closed timestamp notifications for timestamps trailing cluster time by approximately this duration</td></tr>
 <tr><td><code>kv.follower_read.target_multiple</code></td><td>float</td><td><code>3</code></td><td>if above 1, encourages the distsender to perform a read against the closest replica if a request is older than kv.closed_timestamp.target_duration * (1 + kv.closed_timestamp.close_fraction * this) less a clock uncertainty interval. This value also is used to create follower_timestamp(). (WARNING: may compromise cluster stability or correctness; do not edit without supervision)</td></tr>
 <tr><td><code>kv.import.batch_size</code></td><td>byte size</td><td><code>32 MiB</code></td><td>the maximum size of the payload in an AddSSTable request (WARNING: may compromise cluster stability or correctness; do not edit without supervision)</td></tr>
diff --git a/pkg/storage/client_replica_test.go b/pkg/storage/client_replica_test.go
index e1cbf26d3b8f..a57373a2ba6f 100644
--- a/pkg/storage/client_replica_test.go
+++ b/pkg/storage/client_replica_test.go
@@ -1603,8 +1603,11 @@ func TestRangeInfo(t *testing.T) {
 	if pErr != nil {
 		t.Fatal(pErr)
 	}
-	lhsLease, _ = lhsReplica1.GetLease()
-	rhsLease, _ = rhsReplica1.GetLease()
+	// Read the expected lease from replica0 rather than replica1 as it may serve
+	// a follower read which will contain the new lease information before
+	// replica1 has applied the lease transfer.
+	lhsLease, _ = lhsReplica0.GetLease()
+	rhsLease, _ = rhsReplica0.GetLease()
 	expRangeInfos = []roachpb.RangeInfo{
 		{
 			Desc:  *lhsReplica1.Desc(),
diff --git a/pkg/storage/replica_follower_read.go b/pkg/storage/replica_follower_read.go
index 59bdcb747741..7ec03d95f927 100644
--- a/pkg/storage/replica_follower_read.go
+++ b/pkg/storage/replica_follower_read.go
@@ -32,7 +32,7 @@ import (
 var FollowerReadsEnabled = settings.RegisterBoolSetting(
 	"kv.closed_timestamp.follower_reads_enabled",
 	"allow (all) replicas to serve consistent historical reads based on closed timestamp information",
-	false,
+	true,
 )
 
 // canServeFollowerRead tests, when a range lease could not be