From 1be2119196c7cf27ea9b4d1b063f8726d94fd399 Mon Sep 17 00:00:00 2001
From: Alex Robinson <alexdwanerobinson@gmail.com>
Date: Wed, 12 Sep 2018 14:04:32 -0500
Subject: [PATCH] kv: Don't sort replicas by latency if we know the leaseholder

This avoids a bunch of unnecessary latency lookups and sorting in the
common case.

Release note: None
---
 pkg/kv/dist_sender.go      | 19 +++++++++++--------
 pkg/kv/dist_sender_test.go |  5 +++--
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/pkg/kv/dist_sender.go b/pkg/kv/dist_sender.go
index 04beabe6f4f3..84036983ba46 100644
--- a/pkg/kv/dist_sender.go
+++ b/pkg/kv/dist_sender.go
@@ -433,23 +433,26 @@ func (ds *DistSender) sendSingleRange(
 	// Try to send the call.
 	replicas := NewReplicaSlice(ds.gossip, desc)
 
-	// Rearrange the replicas so that they're ordered in expectation of
-	// request latency.
-	var latencyFn LatencyFunc
-	if ds.rpcContext != nil {
-		latencyFn = ds.rpcContext.RemoteClocks.Latency
-	}
-	replicas.OptimizeReplicaOrder(ds.getNodeDescriptor(), latencyFn)
-
 	// If this request needs to go to a lease holder and we know who that is, move
 	// it to the front.
+	var knowLeaseholder bool
 	if !ba.IsReadOnly() || ba.ReadConsistency.RequiresReadLease() {
 		if storeID, ok := ds.leaseHolderCache.Lookup(ctx, desc.RangeID); ok {
 			if i := replicas.FindReplica(storeID); i >= 0 {
 				replicas.MoveToFront(i)
+				knowLeaseholder = true
 			}
 		}
 	}
+	if !knowLeaseholder {
+		// Rearrange the replicas so that they're ordered in expectation of
+		// request latency.
+		var latencyFn LatencyFunc
+		if ds.rpcContext != nil {
+			latencyFn = ds.rpcContext.RemoteClocks.Latency
+		}
+		replicas.OptimizeReplicaOrder(ds.getNodeDescriptor(), latencyFn)
+	}
 
 	br, err := ds.sendRPC(ctx, desc.RangeID, replicas, ba)
 	if err != nil {
diff --git a/pkg/kv/dist_sender_test.go b/pkg/kv/dist_sender_test.go
index cc70d50d2aa8..4f421dfc3513 100644
--- a/pkg/kv/dist_sender_test.go
+++ b/pkg/kv/dist_sender_test.go
@@ -269,9 +269,9 @@ func TestSendRPCOrder(t *testing.T) {
 		{
 			args:  &roachpb.PutRequest{},
 			tiers: append(nodeTiers[5], roachpb.Tier{Key: "irrelevant", Value: ""}),
-			// Compare only the first resulting addresses as we have a lease holder
+			// Compare only the first resulting address as we have a lease holder
 			// and that means we're only trying to send there.
-			expReplica:  []roachpb.NodeID{2, 5, 4, 0, 0},
+			expReplica:  []roachpb.NodeID{2, 0, 0, 0, 0},
 			leaseHolder: 2,
 		},
 		// Inconsistent Get without matching attributes but lease holder (node 3). Should just
@@ -335,6 +335,7 @@ func TestSendRPCOrder(t *testing.T) {
 	ds := NewDistSender(cfg, g)
 
 	for n, tc := range testCases {
+		log.Infof(context.TODO(), "testcase %d", n)
 		verifyCall = makeVerifier(tc.expReplica)
 
 		{