Skip to content

Commit

Permalink
storage: fix TestRangeInfo flake and re-enable follower reads by default
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ajwerner committed Mar 15, 2019
1 parent b4b081d commit d441ad1
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 4 deletions.
2 changes: 1 addition & 1 deletion docs/generated/settings/settings.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
18 changes: 16 additions & 2 deletions pkg/storage/client_replica_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1603,8 +1603,22 @@ func TestRangeInfo(t *testing.T) {
if pErr != nil {
t.Fatal(pErr)
}
lhsLease, _ = lhsReplica1.GetLease()
rhsLease, _ = rhsReplica1.GetLease()
// Retry reading the lease from replica1 until we observe the new lease
// triggered above. This is to protect against the case where the lease
// transfer is applied on replica0 but not on replica1 at the time of the
// above query. In this scenario replica0 can serve a follower read which
// and the RangeInfo will contain the updated lease. The below call to
// GetLease may still return the prior lease value if the lease transfer has
// not yet been applied.
prevLStart, prevRStart := lhsLease.Start, rhsLease.Start
testutils.SucceedsSoon(t, func() error {
lhsLease, _ = lhsReplica1.GetLease()
rhsLease, _ = rhsReplica1.GetLease()
if prevLStart.Less(lhsLease.Start) && prevRStart.Less(rhsLease.Start) {
return nil
}
return fmt.Errorf("waiting for lease transfers to be applied on replica1")
})
expRangeInfos = []roachpb.RangeInfo{
{
Desc: *lhsReplica1.Desc(),
Expand Down
2 changes: 1 addition & 1 deletion pkg/storage/replica_follower_read.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit d441ad1

Please sign in to comment.