kvserver: don't allow VOTER_DEMOTING to acquire lease after transfer

[shralex]

This PR restricts the case when a VOTER_DEMOTING_LEARNER can aquire the lease in a joint configuration to only the case where it was the last leaseholder. Since it is being removed, we only want it to get the lease if no other replica can aquire it, the scenario described in #83687

This fix solves a potential starvation scenario where a VOTER_DEMOTING_LEARNER keeps transferring the lease to the VOTER_INCOMING, succeeding, but then re-acquiring because the VOTER_INCOMING is dead and the lease expires. In this case, we would want another replica to pick up the lease, which would allow us to exit the joint configuration.

This PR also removes the leaseHolderRemovalAllowed parameter of CheckCanReceiveLease,
since it is always true since 22.2.

Release note (bug fix): narrows down the conditions under which a VOTER_DEMOTING_LEARNER can acquire the lease in a joint configuration to a) there has to be an VOTER_INCOMING in the configuration and b) the VOTER_DEMOTING_LEARNER was the last leaseholder. This prevents it from acquiring the lease unless it is the only one that can acquire it, because transferring the lease away is necessary before exiting the joint config (without the fix the system can be stuck in a joint configuration in some rare situations).

Fixes: #88667
See also #89340

[cockroach-teamcity]

This change is 

[nvanbenschoten]

This . Thanks for putting it together.

This PR also removes the leaseHolderRemovalAllowed parameter of CheckCanReceiveLease,
since it is always true since 22.2.

This is fine, but it will make the backport of this PR to v22.1.x a little harder. Reminder that we need to backport this to:

• branch-release-v22.1.x
• branch-release-v22.2.x
• branch-release-v22.2.0

Consider pulling that into a separate commit. You could then use the backport tool to backport only the first part of this change. Although I guess it's going to skew on 9f5f60b no matter what.

Release note (bug fix): stability fix.

Since we're going to have to backport this to v22.1, let's add some more detail to the release note.

Reviewed 9 of 9 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @shralex)

---

pkg/roachpb/metadata_replicas.go line 514 at r1 (raw file):

// to update replica filter in report to keep it correct.
func CheckCanReceiveLease(
	wouldbeLeaseholder ReplicaDescriptor, replDescs ReplicaSet, wasLastLeaseholder bool,

Could we add a paragraph to the comment on this function explaining what this wasLastLeaseholder param means?

---

pkg/kv/kvserver/client_lease_test.go line 461 at r1 (raw file):

	}

	// Complte the replication change.

s/Complte/Complete/

---

pkg/kv/kvserver/batcheval/cmd_lease_test.go line 279 at r1 (raw file):

		leaseholderType         roachpb.ReplicaType
		anotherReplicaType      roachpb.ReplicaType
		wasLastLeaseholderTrue  bool

nit: consider renaming these to expIfWasLastLeaseholderTrue bool and expIfWasLastLeaseholderFalse bool to make it more clear that these are expected outputs for the test, not inputs.

[shralex]

Updated the release note. I think the test will conflict either way, not sure a separate commit is easier.

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

---

pkg/roachpb/metadata_replicas.go line 514 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

Could we add a paragraph to the comment on this function explaining what this wasLastLeaseholder param means?

Done

---

pkg/kv/kvserver/client_lease_test.go line 461 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

s/Complte/Complete/

Done

---

pkg/kv/kvserver/batcheval/cmd_lease_test.go line 279 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

nit: consider renaming these to expIfWasLastLeaseholderTrue bool and expIfWasLastLeaseholderFalse bool to make it more clear that these are expected outputs for the test, not inputs.

Done

[nvanbenschoten]

once the test flakiness is resolved.

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @shralex)

[shralex]

bors r+

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)

[blathers-crl]

Encountered an error creating backports. Some common things that can go wrong:

1. The backport branch might have already existed.
2. There was a merge conflict.
3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.

---

error creating merge commit from 17d288b to blathers/backport-release-22.1-89564: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 22.1.x failed. See errors above.

---

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