Skip to content

Commit 6d1fb75

Browse files
craig[bot]wenyihu6
andcommitted
Merge #144476
144476: kvserver: metamorphically enable kv.closed_timestamp.lead_for_global_reads_auto_tune.enabled r=arulajmani a=wenyihu6 **kvserver: metamorphically enable kv.closed_timestamp.lead_for_global_reads_auto_tune.enabled** This commit metamorphically enables kv.closed_timestamp.lead_for_global_reads_auto_tune.enabled. Part of: #143888 Release note: none --- **kvserver: clarify round-trip latency in policy refresher comments** Previously, it might be unclear to readers unless they trace the behavior back through rpccontext. This commit updates the comments in the policy refresher to explicitly state that the latency being referred to is round-trip. Epic: none Release note: none Co-authored-by: wenyihu6 <[email protected]>
2 parents 844a17b + b1b93c9 commit 6d1fb75

File tree

4 files changed

+19
-14
lines changed

4 files changed

+19
-14
lines changed

pkg/kv/kvserver/closedts/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ go_library(
1313
"//pkg/kv/kvserver/closedts/ctpb",
1414
"//pkg/settings",
1515
"//pkg/util/hlc",
16+
"//pkg/util/metamorphic",
1617
],
1718
)
1819

pkg/kv/kvserver/closedts/policyrefresher/policy_refresher.go

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,10 @@ type PolicyRefresher struct {
4040
// when there is a leaseholder change or when there is a span config change.
4141
refreshNotificationCh chan struct{}
4242

43-
// getNodeLatencies returns a map of node IDs to their measured latencies
44-
// from the current node. Replicas use this information to determine
45-
// appropriate closed timestamp policies.
43+
// getNodeLatencies returns a map of node IDs to their observed round-trip
44+
// latencies from the current node. Replicas use this information to determine
45+
// appropriate closed timestamp policies. See rpc.RemoteClockMonitor for more
46+
// details.
4647
getNodeLatencies func() map[roachpb.NodeID]time.Duration
4748

4849
// latencyCache caches the latency information from getNodeLatencies. This
@@ -85,10 +86,10 @@ func NewPolicyRefresher(
8586
// Replica defines a thin interface to update closed timestamp policies.
8687
type Replica interface {
8788
// RefreshPolicy informs the replica that it should refresh its closed
88-
// timestamp policy. A latency map, which includes observed latency to other
89-
// nodes in the system by the PolicyRefresher may be supplied (or may be nil),
90-
// in which case it may be used to correctly place a replica in its latency
91-
// based global reads bucket.
89+
// timestamp policy. A latency map, which includes round-trip observed latency
90+
// to other nodes in the system by the PolicyRefresher may be supplied (or may
91+
// be nil), in which case it may be used to correctly place a replica in its
92+
// latency based global reads bucket.
9293
RefreshPolicy(map[roachpb.NodeID]time.Duration)
9394
}
9495

@@ -118,17 +119,19 @@ func (pr *PolicyRefresher) EnqueueReplicaForRefresh(replica Replica) {
118119
}
119120
}
120121

121-
// updateLatencyCache refreshes the cached latency information by fetching fresh
122-
// measurements from the actual RPC context. This can be expensive, so we only
123-
// do this periodically based on the configured refresh interval.
122+
// updateLatencyCache refreshes the cached round-trip latency information by
123+
// fetching fresh measurements from the actual RPC context. This can be
124+
// expensive, so we only do this periodically based on the configured refresh
125+
// interval.
124126
func (pr *PolicyRefresher) updateLatencyCache() {
125127
pr.mu.Lock()
126128
defer pr.mu.Unlock()
127129
pr.mu.latencyCache = pr.getNodeLatencies()
128130
}
129131

130-
// getCurrentLatencies returns the current latency information if auto-tuning is
131-
// enabled and the cluster has been fully upgraded to v25.2, or nil otherwise.
132+
// getCurrentLatencies returns the current round-trip latency information if
133+
// auto-tuning is enabled and the cluster has been fully upgraded to v25.2, or
134+
// nil otherwise.
132135
func (pr *PolicyRefresher) getCurrentLatencies() map[roachpb.NodeID]time.Duration {
133136
// Testing knobs only.
134137
if pr.knobs != nil && pr.knobs.InjectedLatencies != nil {

pkg/kv/kvserver/closedts/setting.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"time"
1010

1111
"github.com/cockroachdb/cockroach/pkg/settings"
12+
"github.com/cockroachdb/cockroach/pkg/util/metamorphic"
1213
)
1314

1415
// NB: These settings are SystemVisible because they need to be read by e.g.
@@ -86,6 +87,6 @@ var LeadForGlobalReadsAutoTuneEnabled = settings.RegisterBoolSetting(
8687
"furthest follower will be used to adjust closed timestamp policies for ranges"+
8788
"ranges configured to serve global reads. "+
8889
"kv.closed_timestamp.lead_for_global_reads_override takes precedence if set.",
89-
false,
90+
metamorphic.ConstantWithTestBool("kv.closed_timestamp.lead_for_global_reads_auto_tune.enabled", false),
9091
settings.WithPublic,
9192
)

pkg/kv/kvserver/replica.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1339,7 +1339,7 @@ func (r *Replica) closedTimestampPolicyRLocked() ctpb.RangeClosedTimestampPolicy
13391339
}
13401340

13411341
// RefreshPolicy updates the replica's cached closed timestamp policy based on
1342-
// span configurations and provided node latencies.
1342+
// span configurations and provided node round-trip latencies.
13431343
func (r *Replica) RefreshPolicy(latencies map[roachpb.NodeID]time.Duration) {
13441344
policy := func() ctpb.RangeClosedTimestampPolicy {
13451345
desc, conf := r.DescAndSpanConfig()

0 commit comments

Comments
 (0)