mmaprototype: release rangeAnalyzedConstraints properly

[wenyihu6]

Epic: CRDB-55052
Release note: none

---

mmaprototype: release rangeAnalyzedConstraints properly

Previously, we weren’t calling releaseRangeAnalyzedConstraints on
rangeAnalyzedConstraints, which could lead to resource leak. This
commit calls releaseRangeAnalyzedConstraints when they are unused.

[blathers-crl]

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

[cockroach-teamcity]

This change is 

[tbg]

@tbg reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @sumeerbhola)

---

-- commits line 5 at r1:
Could it leak? Releasing them puts them back into the pool, but they should otherwise have gotten garbage collected.

[sumeerbhola]

@sumeerbhola reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @wenyihu6)

---

pkg/kv/kvserver/allocator/mmaprototype/cluster_state.go line 1665 at r1 (raw file):

		// change, or we noticed a divergence in membership above and fell through
		// here.
		releaseRangeAnalyzedConstraints(rs.constraints)

nit: can we make a method on rangeState called clearAnalyzedConstraints that both sets rs.constraints to nil and calls releaseRangeAnalyzedConstraints. I am nervous about multiple places remembering to do both.

---

pkg/kv/kvserver/allocator/mmaprototype/cluster_state.go line 1665 at r1 (raw file):

		// change, or we noticed a divergence in membership above and fell through
		// here.
		releaseRangeAnalyzedConstraints(rs.constraints)

releaseRangeAnalyzedConstraints assumes the parameter is non-nil. It isn't clear all these callers satisfy that constraint.

---

pkg/kv/kvserver/allocator/mmaprototype/cluster_state.go line 1707 at r1 (raw file):

			delete(cs.stores[replica.StoreID].adjusted.replicas, r)
		}
		releaseRangeAnalyzedConstraints(rs.constraints)

this case would also call the aforementioned method.

[wenyihu6]

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @sumeerbhola and @tbg)

---

-- commits line 5 at r1:

Previously, tbg (Tobias Grieger) wrote…

Could it leak? Releasing them puts them back into the pool, but they should otherwise have gotten garbage collected.

I was thinking of not returning something to the pool as a resource leak, but I’m not sure if that’s the right terminology. I rephrased it as not putting things back into the pool.

---

pkg/kv/kvserver/allocator/mmaprototype/cluster_state.go line 1665 at r1 (raw file):

Previously, sumeerbhola wrote…

nit: can we make a method on rangeState called clearAnalyzedConstraints that both sets rs.constraints to nil and calls releaseRangeAnalyzedConstraints. I am nervous about multiple places remembering to do both.

Good call, done.

---

pkg/kv/kvserver/allocator/mmaprototype/cluster_state.go line 1665 at r1 (raw file):

Previously, sumeerbhola wrote…

releaseRangeAnalyzedConstraints assumes the parameter is non-nil. It isn't clear all these callers satisfy that constraint.

Good call, there were panics. Fixed.

---

pkg/kv/kvserver/allocator/mmaprototype/cluster_state.go line 1707 at r1 (raw file):

Previously, sumeerbhola wrote…

this case would also call the aforementioned method.

Done.

[sumeerbhola]

@sumeerbhola reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (and 1 stale) (waiting on @tbg)

[wenyihu6]

TFTRs!

bors r+

[craig]

🕐 Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

[craig]

Build succeeded:

• linux_amd64_fips_build
• examples_orms
• check_generated_code
• acceptance
• local_roachtest_fips
• unit_tests
• docker_image_amd64
• lint
• local_roachtest
• linux_amd64_build