kvclient: ship range descriptor in NLHE

We recently changed a NLHE to carry the range descriptor generation to
ensure to avoid thrashing the range cache if the replica had a stale
view of the range. In cockroachdb#75742, we saw issues caused by the dist sender
having a stale range descriptor. This patch switches from sending just
the descriptor generation back in NLHE to shipping back the entire
range descriptor. In the future, we may want to solve the issue above
by updating the range cache with the fresher range descriptor thus
skipping a cache eviction and range descriptor lookup.

References cockroachdb#75742

Release note: None

pkg/kv/kvclient/kvcoord/dist_sender.go

@@ -2145,10 +2145,10 @@ func (ds *DistSender) sendToReplicas(
 
 					var updatedLeaseholder bool
 					if tErr.Lease != nil {
-						updatedLeaseholder = routing.UpdateLease(ctx, tErr.Lease, tErr.DescriptorGeneration)
+						updatedLeaseholder = routing.UpdateLease(ctx, tErr.Lease, tErr.RangeDesc.Generation)
 					} else if tErr.LeaseHolder != nil {
 						// tErr.LeaseHolder might be set when tErr.Lease isn't.
-						routing.UpdateLeaseholder(ctx, *tErr.LeaseHolder, tErr.DescriptorGeneration)
+						routing.UpdateLeaseholder(ctx, *tErr.LeaseHolder, tErr.RangeDesc.Generation)
 						updatedLeaseholder = true
 					}
 					// Move the new leaseholder to the head of the queue for the next

pkg/kv/kvclient/kvcoord/dist_sender_test.go

@@ -1088,7 +1088,12 @@ func TestDistSenderIgnoresNLHEBasedOnOldRangeGeneration(t *testing.T) {
 		calls = append(calls, ba.Replica.NodeID)
 		if ba.Replica.NodeID == 2 {
 			reply := &roachpb.BatchResponse{}
-			err := &roachpb.NotLeaseHolderError{Lease: &ambiguousLease, DescriptorGeneration: oldGeneration}
+			err := &roachpb.NotLeaseHolderError{
+				Lease: &ambiguousLease,
+				RangeDesc: roachpb.RangeDescriptor{
+					Generation: oldGeneration,
+				},
+			}
 			reply.Error = roachpb.NewError(err)
 			return reply, nil
 		}

pkg/kv/kvserver/replica_range_lease.go

@@ -914,9 +914,9 @@ func newNotLeaseHolderError(
 	l roachpb.Lease, proposerStoreID roachpb.StoreID, rangeDesc *roachpb.RangeDescriptor, msg string,
 ) *roachpb.NotLeaseHolderError {
 	err := &roachpb.NotLeaseHolderError{
-		RangeID:              rangeDesc.RangeID,
-		DescriptorGeneration: rangeDesc.Generation,
-		CustomMsg:            msg,
+		RangeID:   rangeDesc.RangeID,
+		RangeDesc: *rangeDesc,
+		CustomMsg: msg,
 	}
 	if proposerStoreID != 0 {
 		err.Replica, _ = rangeDesc.GetReplicaDescriptor(proposerStoreID)

pkg/roachpb/errors.proto

@@ -56,12 +56,14 @@ message NotLeaseHolderError {
   optional roachpb.Lease lease = 4;
   optional int64 range_id = 3 [(gogoproto.nullable) = false,
       (gogoproto.customname) = "RangeID", (gogoproto.casttype) = "RangeID"];
-  // The range descriptor generation of the replica the error originated from.
-  // Used by the DistSender's RangeCache to determine whether the error was
-  // returned because the replica had a stale understanding of who the
-  // leaseholder is.
-  optional int64 descriptor_generation = 6 [(gogoproto.nullable) = false,
-    (gogoproto.customname) = "DescriptorGeneration", (gogoproto.casttype) = "RangeGeneration"];
+  // The range descriptor from the replica the error originated from.
+  // The generation of the descriptor is used by the DistSender's RangeCache to
+  // determine whether the error was returned because the replica had a stale
+  // understanding of who the leaseholder is.
+  // TOOD(arul): In the future we may want to update the RangeCache if the
+  // returned range descriptor is newer than what is in the RangeCache. This
+  // would help us avoid a cache eviction/descriptor lookup.
+  optional roachpb.RangeDescriptor range_desc = 6 [(gogoproto.nullable)=false];
   // If set, the Error() method will return this instead of composing its
   // regular spiel. Useful because we reuse this error when rejecting a command
   // because the lease under which its application was attempted is different