mmaprototype: fix missing change id bug

Previously, processStoreLeaseholderMsgInternal iterated over the slice
rs.pendingChanges while modifying it in-place by swapping elements to the back
and shortening the slice. Since the slice header is copied at the start of a
range loop, this led to attempts to delete changes that are technically already
out of the length had already been removed. This commit fixes the issue by
copying the change IDs before the loop to avoid iterating over a mutating slice.

Epic: none
Release note: none

Author: wenyihu6

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

@@ -1377,8 +1377,12 @@ func (cs *clusterState) processStoreLeaseholderMsgInternal(
 		// Since this range is going away, mark all the pending changes as
 		// enacted. This will allow the load adjustments to also be garbage
 		// collected in the future.
-		for _, change := range rs.pendingChanges {
-			cs.pendingChangeEnacted(change.ChangeID, now, true)
+		changeIDs := make([]ChangeID, len(rs.pendingChanges))
+		for i, change := range rs.pendingChanges {
+			changeIDs[i] = change.ChangeID
+		}
+		for _, changeID := range changeIDs {
+			cs.pendingChangeEnacted(changeID, now, true)
 		}
 		// Remove from the storeStates.
 		for _, replica := range rs.replicas {

pkg/kv/kvserver/allocator/mmaprototype/testdata/cluster_state/removed_ranges

@@ -0,0 +1,57 @@
+# This test tests an edge case where the leaseholder is removed from the range.
+# Regression test for a bug where processStoreLeaseholderMsgInternal was
+# iterating over the pending changes and removing them inside the loop.
+set-store
+  store-id=1 node-id=1 attrs=purple locality-tiers=region=us-west-1,zone=us-west-1a
+  store-id=2 node-id=2 attrs=yellow locality-tiers=region=us-east-1,zone=us-east-1a
+----
+node-id=1 failure-summary=ok locality-tiers=region=us-west-1,zone=us-west-1a,node=1
+  store-id=1 membership=full attrs=purple locality-code=1:2:3:
+node-id=2 failure-summary=ok locality-tiers=region=us-east-1,zone=us-east-1a,node=2
+  store-id=2 membership=full attrs=yellow locality-code=4:5:6:
+
+store-leaseholder-msg 
+store-id=1
+  range-id=1 load=[80,80,80] raft-cpu=20 config=(num_replicas=3 constraints={'+region=us-west-1:1'} voter_constraints={'+region=us-west-1:1'})
+    store-id=1 replica-id=1 type=VOTER_FULL leaseholder=true
+----
+
+ranges
+----
+range-id=1 load=[cpu:80, write-bandwidth:80, byte-size:80] raft-cpu=20
+  store-id=1 replica-id=1 type=VOTER_FULL leaseholder=true
+
+make-pending-changes range-id=1
+  rebalance-replica: remove-store-id=1 add-store-id=2
+----
+pending(2)
+change-id=1 store-id=2 node-id=2 range-id=1 load-delta=[cpu:88, write-bandwidth:88, byte-size:88] start=0s
+  prev=(replica-id=none type=VOTER_FULL)
+  next=(replica-id=unknown type=VOTER_FULL leaseholder=true)
+change-id=2 store-id=1 node-id=1 range-id=1 load-delta=[cpu:-80, write-bandwidth:-80, byte-size:-80] start=0s
+  prev=(replica-id=1 type=VOTER_FULL leaseholder=true)
+  next=(replica-id=none type=VOTER_FULL)
+
+# Advance time to 1s so that we can see pending changes for removed ranges are
+# properly enacted.
+tick seconds=1
+----
+t=1s
+
+store-leaseholder-msg 
+store-id=1
+----
+
+# The pending changes are enacted with enactedAtTime set to 1s.
+get-pending-changes
+----
+pending(2)
+change-id=1 store-id=2 node-id=2 range-id=1 load-delta=[cpu:88, write-bandwidth:88, byte-size:88] start=0s enacted=1s
+  prev=(replica-id=none type=VOTER_FULL)
+  next=(replica-id=unknown type=VOTER_FULL leaseholder=true)
+change-id=2 store-id=1 node-id=1 range-id=1 load-delta=[cpu:-80, write-bandwidth:-80, byte-size:-80] start=0s enacted=1s
+  prev=(replica-id=1 type=VOTER_FULL leaseholder=true)
+  next=(replica-id=none type=VOTER_FULL)
+
+ranges
+----