Merge #158153

158153: mma: make external caller mostly use an ExternalRangeChange r=sumeerbhola a=sumeerbhola

An ExternalRangeChange does not have the currently mutable fields of PendingRangeChange (and the set of mutable fields is going to expand in the near future). Todos related to limitation of viewing a change as a "change replicas" or "lease transfer" etc., are moved to ExternalRangeChange, so compartmentalized to methods that are called by integration code outside the mma package.

There is still some potential future cleanup involving unexporting PendingRangeChange, which is not in scope for this PR.

The PendingRangeChange becomes a temporary container for a slice of []*pendingReplicaChanges. Printing this struct no longer involves viewing it as a "change replicas" or "lease transfer". Furthermore, the ReplicaChange.replicaChangeType field is removed and the change type is now a function of prev and next. This sets us up for making prev mutable in a future PR.

Informs #157049

Epic: CRDB-55052

Release note: None

Co-authored-by: sumeerbhola <sumeer@cockroachlabs.com>

Author: craig[bot]

pkg/kv/kvserver/allocator/mmaprototype/BUILD.bazel

@@ -13,6 +13,7 @@ go_library(
         "load.go",
         "memo_helper.go",
         "messages.go",
+        "range_change.go",
         "rebalance_advisor.go",
         "store_load_summary.go",
         "store_set.go",

pkg/kv/kvserver/allocator/mmaprototype/allocator.go

@@ -56,16 +56,16 @@ type Allocator interface {
 	// Calls to AdjustPendingChangeDisposition must be correctly sequenced with
 	// full state updates from the local node provided in
 	// ProcessNodeLoadResponse.
-	AdjustPendingChangeDisposition(change PendingRangeChange, success bool)
+	AdjustPendingChangeDisposition(change ExternalRangeChange, success bool)
 
 	// RegisterExternalChange informs this allocator about yet to complete
 	// changes to the cluster which were not initiated by this allocator. The
 	// ownership of all state inside change is handed off to the callee. If ok
-	// is true, the change was registered, and the caller should subsequently
-	// use the same change in a subsequent call to
+	// is true, the change was registered, and the caller is returned an
+	// ExternalRangeChange that it should subsequently use in a call to
 	// AdjustPendingChangeDisposition when the changes are completed, either
 	// successfully or not. If ok is false, the change was not registered.
-	RegisterExternalChange(change PendingRangeChange) (ok bool)
+	RegisterExternalChange(change PendingRangeChange) (_ ExternalRangeChange, ok bool)
 
 	// ComputeChanges is called periodically and frequently, say every 10s.
 	//
@@ -82,7 +82,7 @@ type Allocator interface {
 	// the allocator, to avoid re-proposing the same change and to make
 	// adjustments to the load.
 	ComputeChanges(
-		ctx context.Context, msg *StoreLeaseholderMsg, opts ChangeOptions) []PendingRangeChange
+		ctx context.Context, msg *StoreLeaseholderMsg, opts ChangeOptions) []ExternalRangeChange
 
 	// AdminRelocateOne is a helper for AdminRelocateRange.
 	//

pkg/kv/kvserver/allocator/mmaprototype/allocator_state.go

@@ -254,7 +254,7 @@ func (a *allocatorState) ProcessStoreLoadMsg(ctx context.Context, msg *StoreLoad
 }
 
 // AdjustPendingChangeDisposition implements the Allocator interface.
-func (a *allocatorState) AdjustPendingChangeDisposition(change PendingRangeChange, success bool) {
+func (a *allocatorState) AdjustPendingChangeDisposition(change ExternalRangeChange, success bool) {
 	a.mu.Lock()
 	defer a.mu.Unlock()
 	rs, ok := a.cs.ranges[change.RangeID]
@@ -274,7 +274,7 @@ func (a *allocatorState) AdjustPendingChangeDisposition(change PendingRangeChang
 	// can be made about whether these changes will be found in the allocator's
 	// state. We gather the found changes.
 	var changes []*pendingReplicaChange
-	for _, c := range change.pendingReplicaChanges {
+	for _, c := range change.Changes {
 		ch, ok := a.cs.pendingChanges[c.changeID]
 		if !ok {
 			continue
@@ -303,25 +303,27 @@ func (a *allocatorState) AdjustPendingChangeDisposition(change PendingRangeChang
 }
 
 // RegisterExternalChange implements the Allocator interface.
-func (a *allocatorState) RegisterExternalChange(change PendingRangeChange) (ok bool) {
+func (a *allocatorState) RegisterExternalChange(
+	change PendingRangeChange,
+) (_ ExternalRangeChange, ok bool) {
 	a.mu.Lock()
 	defer a.mu.Unlock()
 	if err := a.cs.preCheckOnApplyReplicaChanges(change); err != nil {
 		a.mmaMetrics.ExternalFailedToRegister.Inc(1)
 		log.KvDistribution.Infof(context.Background(),
 			"did not register external changes: due to %v", err)
-		return false
+		return ExternalRangeChange{}, false
 	} else {
 		a.mmaMetrics.ExternaRegisterSuccess.Inc(1)
 	}
 	a.cs.addPendingRangeChange(change)
-	return true
+	return MakeExternalRangeChange(change), true
 }
 
 // ComputeChanges implements the Allocator interface.
 func (a *allocatorState) ComputeChanges(
 	ctx context.Context, msg *StoreLeaseholderMsg, opts ChangeOptions,
-) []PendingRangeChange {
+) []ExternalRangeChange {
 	a.mu.Lock()
 	defer a.mu.Unlock()
 	if msg.StoreID != opts.LocalStoreID {