Merge #40370

40370: storage: prepare for kv.atomic_replication_changes=true r=nvanbenschoten a=tbg

First three commits are #40363.

----

This PR enables atomic replication changes by default. But most of it is
just dealing with the fallout of doing so:

1. we don't handle removal of multiple learners well at the moment. This will
   be fixed more holistically in #40268, but it's not worth waiting for that
   because it's easy for us to just avoid the problem.
2. tests that carry out splits become quite flaky because at the beginning of
   a split, we transition out of a joint config if we see one, and due to
   the initial upreplication we often do. If we lose the race against the
   replicate queue, the split catches an error for no good reason.
   I took this as an opportunity to refactor the descriptor comparisons
   and to make this specific case a noop, but making it easier to avoid
   this general class of conflict where it's avoidable in the future.

There are probably some more problems that will only become apparent over time,
but it's quite simple to turn the cluster setting off again and to patch things
up if we do.

Release note (general change): atomic replication changes are now enabled
by default.

Co-authored-by: Tobias Schottdorf <[email protected]>

pkg/roachpb/api.proto

@@ -720,6 +720,7 @@ message AdminUnsplitResponse {
 // now encompass all keys from its original start key to the end key
 // of the subsumed range. If AdminMerge is called on the final range
 // in the key space, it is a noop.
+// The request must be addressed to the start key of the left hand side.
 message AdminMergeRequest {
   option (gogoproto.equal) = true;
 

pkg/roachpb/metadata.go

@@ -100,6 +100,24 @@ func (a Attributes) String() string {
 	return strings.Join(a.Attrs, ",")
 }
 
+// NewRangeDescriptor returns a RangeDescriptor populated from the input.
+func NewRangeDescriptor(
+	rangeID RangeID, start, end RKey, replicas ReplicaDescriptors,
+) *RangeDescriptor {
+	repls := append([]ReplicaDescriptor(nil), replicas.All()...)
+	for i := range repls {
+		repls[i].ReplicaID = ReplicaID(i + 1)
+	}
+	desc := &RangeDescriptor{
+		RangeID:       rangeID,
+		StartKey:      start,
+		EndKey:        end,
+		NextReplicaID: ReplicaID(len(repls) + 1),
+	}
+	desc.SetReplicas(MakeReplicaDescriptors(repls))
+	return desc
+}
+
 // RSpan returns the RangeDescriptor's resolved span.
 func (r *RangeDescriptor) RSpan() RSpan {
 	return RSpan{Key: r.StartKey, EndKey: r.EndKey}

pkg/storage/client_merge_test.go

@@ -2259,7 +2259,7 @@ func TestStoreRangeMergeDeadFollowerDuringTxn(t *testing.T) {
 
 	args := adminMergeArgs(lhsDesc.StartKey.AsRawKey())
 	_, pErr := client.SendWrapped(ctx, store0.TestSender(), args)
-	expErr := "merge of range into .* failed: waiting for all right-hand replicas to catch up"
+	expErr := "merge failed: waiting for all right-hand replicas to catch up"
 	if !testutils.IsPError(pErr, expErr) {
 		t.Fatalf("expected %q error, but got %v", expErr, pErr)
 	}

pkg/storage/merge_queue.go

@@ -353,7 +353,9 @@ func (mq *mergeQueue) process(
 		humanizeutil.IBytes(mergedStats.Total()),
 		mergedQPS,
 	)
-	_, pErr := lhsRepl.AdminMerge(ctx, roachpb.AdminMergeRequest{}, reason)
+	_, pErr := lhsRepl.AdminMerge(ctx, roachpb.AdminMergeRequest{
+		RequestHeader: roachpb.RequestHeader{Key: lhsRepl.Desc().StartKey.AsRawKey()},
+	}, reason)
 	switch err := pErr.GoError(); err.(type) {
 	case nil:
 	case *roachpb.ConditionFailedError: