kvserver: use SubsumeReplica when merging

Epic: none
Release note: none

Author: pav-kv

pkg/kv/kvserver/kvstorage/destroy.go

@@ -179,20 +179,28 @@ func DestroyReplica(
 // (and the corresponding system and lock table keys). The latter are inherited
 // by the subsuming range.
 //
+// The forceSortedKeys flag, if true, forces the writes to be generated in the
+// sorted order of keys. This is to support feeding the writes from this
+// function to SSTables, in the snapshot ingestion path.
+//
 // Returns SelectOpts which can be used to reflect on the key spans that this
 // function clears.
 // TODO(pav-kv): get rid of SelectOpts.
 func SubsumeReplica(
-	ctx context.Context, reader storage.Reader, writer storage.Writer, info DestroyReplicaInfo,
+	ctx context.Context,
+	reader storage.Reader,
+	writer storage.Writer,
+	info DestroyReplicaInfo,
+	forceSortedKeys bool,
 ) (rditer.SelectOpts, error) {
-	// NB: set MustUseClearRange to true because this call is used for generating
-	// SSTables when ingesting a snapshot, which requires Clears and Puts to be
-	// written in key order. DestroyReplica sets RangeTombstoneKey after clearing
-	// the unreplicated span which may contain higher keys.
+	// NB: if required, set MustUseClearRange to true. This call can be used for
+	// generating SSTables when ingesting a snapshot, which requires Clears and
+	// Puts to be written in key order. DestroyReplica sets RangeTombstoneKey
+	// after clearing the unreplicated span which may contain higher keys.
 	opts := ClearRangeDataOptions{
 		ClearReplicatedByRangeID:   true,
 		ClearUnreplicatedByRangeID: true,
-		MustUseClearRange:          true,
+		MustUseClearRange:          forceSortedKeys,
 	}
 	return rditer.SelectOpts{
 		ReplicatedByRangeID:   opts.ClearReplicatedByRangeID,

pkg/kv/kvserver/replica_app_batch.go

@@ -359,21 +359,10 @@ func (b *replicaAppBatch) runPostAddTriggersReplicaOnly(
 		rhsRepl.mu.Unlock()
 		rhsRepl.readOnlyCmdMu.Unlock()
 
-		// Use math.MaxInt32 (MergedTombstoneReplicaID) as the nextReplicaID as an
-		// extra safeguard against creating new replicas of the RHS. This isn't
-		// required for correctness, since the merge protocol should guarantee that
-		// no new replicas of the RHS can ever be created, but it doesn't hurt to
-		// be careful.
-		if err := kvstorage.DestroyReplica(
-			ctx, b.batch, b.batch,
-			rhsRepl.destroyInfoRaftMuLocked(),
-			kvstorage.MergedTombstoneReplicaID,
-			kvstorage.ClearRangeDataOptions{
-				ClearReplicatedByRangeID:   true,
-				ClearUnreplicatedByRangeID: true,
-			},
+		if _, err := kvstorage.SubsumeReplica(
+			ctx, b.batch, b.batch, rhsRepl.destroyInfoRaftMuLocked(), false, /* forceSortedKeys */
 		); err != nil {
-			return errors.Wrapf(err, "unable to destroy replica before merge")
+			return errors.Wrapf(err, "unable to subsume replica before merge")
 		}
 
 		// Shut down rangefeed processors on either side of the merge.

pkg/kv/kvserver/snapshot_apply_prepare.go

@@ -150,7 +150,7 @@ func (s *snapWriteBuilder) clearSubsumedReplicaDiskData(ctx context.Context) err
 	for _, sub := range s.subsume {
 		// We have to create an SST for the subsumed replica's range-id local keys.
 		if err := s.writeSST(ctx, func(ctx context.Context, w storage.Writer) error {
-			opts, err := kvstorage.SubsumeReplica(ctx, reader, w, sub)
+			opts, err := kvstorage.SubsumeReplica(ctx, reader, w, sub, true /* forceSortedKeys */)
 			s.cleared = append(s.cleared, rditer.Select(sub.RangeID, opts)...)
 			return err
 		}); err != nil {