From 20c1985c73b00c6edc476e5f3c033288431568c2 Mon Sep 17 00:00:00 2001 From: Arul Ajmani Date: Tue, 20 Aug 2024 21:23:21 +0000 Subject: [PATCH] kvclient: improve logging in OptimizeReplicaOrder Epic: none Release note: None --- pkg/kv/kvclient/kvcoord/dist_sender.go | 4 ++-- pkg/kv/kvclient/kvcoord/dist_sender_rangefeed.go | 2 +- pkg/kv/kvclient/kvcoord/replica_slice.go | 2 ++ pkg/kv/kvclient/kvcoord/replica_slice_test.go | 2 +- pkg/sql/physicalplan/replicaoracle/oracle.go | 4 ++-- 5 files changed, 8 insertions(+), 6 deletions(-) diff --git a/pkg/kv/kvclient/kvcoord/dist_sender.go b/pkg/kv/kvclient/kvcoord/dist_sender.go index 59242b1bc12c..b81021eb0273 100644 --- a/pkg/kv/kvclient/kvcoord/dist_sender.go +++ b/pkg/kv/kvclient/kvcoord/dist_sender.go @@ -2551,7 +2551,7 @@ func (ds *DistSender) sendToReplicas( // First order by latency, then move the leaseholder to the front of the // list, if it is known. if !ds.dontReorderReplicas { - replicas.OptimizeReplicaOrder(ds.st, ds.nodeIDGetter(), ds.healthFunc, ds.latencyFunc, ds.locality) + replicas.OptimizeReplicaOrder(ctx, ds.st, ds.nodeIDGetter(), ds.healthFunc, ds.latencyFunc, ds.locality) } idx := -1 @@ -2571,7 +2571,7 @@ func (ds *DistSender) sendToReplicas( case kvpb.RoutingPolicy_NEAREST: // Order by latency. - replicas.OptimizeReplicaOrder(ds.st, ds.nodeIDGetter(), ds.healthFunc, ds.latencyFunc, ds.locality) + replicas.OptimizeReplicaOrder(ctx, ds.st, ds.nodeIDGetter(), ds.healthFunc, ds.latencyFunc, ds.locality) log.VEventf(ctx, 2, "routing to nearest replica; leaseholder not required order=%v", replicas) default: diff --git a/pkg/kv/kvclient/kvcoord/dist_sender_rangefeed.go b/pkg/kv/kvclient/kvcoord/dist_sender_rangefeed.go index eb27238a22b1..fdecb11a3112 100644 --- a/pkg/kv/kvclient/kvcoord/dist_sender_rangefeed.go +++ b/pkg/kv/kvclient/kvcoord/dist_sender_rangefeed.go @@ -649,7 +649,7 @@ func newTransportForRange( if err != nil { return nil, err } - replicas.OptimizeReplicaOrder(ds.st, ds.nodeIDGetter(), ds.healthFunc, ds.latencyFunc, ds.locality) + replicas.OptimizeReplicaOrder(ctx, ds.st, ds.nodeIDGetter(), ds.healthFunc, ds.latencyFunc, ds.locality) opts := SendOptions{class: defRangefeedConnClass} return ds.transportFactory(opts, replicas), nil } diff --git a/pkg/kv/kvclient/kvcoord/replica_slice.go b/pkg/kv/kvclient/kvcoord/replica_slice.go index 8fc8445f2a13..bc2a164b49b6 100644 --- a/pkg/kv/kvclient/kvcoord/replica_slice.go +++ b/pkg/kv/kvclient/kvcoord/replica_slice.go @@ -220,6 +220,7 @@ type HealthFunc func(roachpb.NodeID) bool // leaseholder is known by the caller, the caller will move it to the // front if appropriate. func (rs ReplicaSlice) OptimizeReplicaOrder( + ctx context.Context, st *cluster.Settings, nodeID roachpb.NodeID, healthFn HealthFunc, @@ -229,6 +230,7 @@ func (rs ReplicaSlice) OptimizeReplicaOrder( // If we don't know which node we're on or its locality, and we don't have // latency information to other nodes, send the RPCs randomly. if nodeID == 0 && latencyFn == nil && len(locality.Tiers) == 0 { + log.VEvent(ctx, 2, "randomly shuffling replicas to route to") shuffle.Shuffle(rs) return } diff --git a/pkg/kv/kvclient/kvcoord/replica_slice_test.go b/pkg/kv/kvclient/kvcoord/replica_slice_test.go index 955567882cbb..02e787d23c86 100644 --- a/pkg/kv/kvclient/kvcoord/replica_slice_test.go +++ b/pkg/kv/kvclient/kvcoord/replica_slice_test.go @@ -327,7 +327,7 @@ func TestReplicaSliceOptimizeReplicaOrder(t *testing.T) { } // Randomize the input order, as it's not supposed to matter. shuffle.Shuffle(test.slice) - test.slice.OptimizeReplicaOrder(st, test.nodeID, healthFn, latencyFn, test.locality) + test.slice.OptimizeReplicaOrder(context.Background(), st, test.nodeID, healthFn, latencyFn, test.locality) var sortedNodes []roachpb.NodeID sortedNodes = append(sortedNodes, test.slice[0].NodeID) for i := 1; i < len(test.slice); i++ { diff --git a/pkg/sql/physicalplan/replicaoracle/oracle.go b/pkg/sql/physicalplan/replicaoracle/oracle.go index 9cf7fd47d28b..efc984307799 100644 --- a/pkg/sql/physicalplan/replicaoracle/oracle.go +++ b/pkg/sql/physicalplan/replicaoracle/oracle.go @@ -211,7 +211,7 @@ func (o *closestOracle) ChoosePreferredReplica( if err != nil { return roachpb.ReplicaDescriptor{}, false, err } - replicas.OptimizeReplicaOrder(o.st, o.nodeID, o.healthFunc, o.latencyFunc, o.locality) + replicas.OptimizeReplicaOrder(ctx, o.st, o.nodeID, o.healthFunc, o.latencyFunc, o.locality) repl := replicas[0].ReplicaDescriptor // There are no "misplanned" ranges if we know the leaseholder, and we're // deliberately choosing non-leaseholder. @@ -279,7 +279,7 @@ func (o *binPackingOracle) ChoosePreferredReplica( if err != nil { return roachpb.ReplicaDescriptor{}, false, err } - replicas.OptimizeReplicaOrder(o.st, o.nodeID, o.healthFunc, o.latencyFunc, o.locality) + replicas.OptimizeReplicaOrder(ctx, o.st, o.nodeID, o.healthFunc, o.latencyFunc, o.locality) // Look for a replica that has been assigned some ranges, but it's not yet full. minLoad := int(math.MaxInt32)