kvserver: allow reliably pinning and expiring leases in tests

[tbg]

In many tests¹² we "just" want the lease for range X to be on store Y and to stay there even when the test is being stressed under nightly and perhaps on an overloaded VM. There are a number of factors that can get in the way of this humble goal.

• ability to acquire lease is tied to raft leadership³⁴
• leases are expiration-based (either directly or through liveness)

The second point is a major source of flakiness, and a pretty annoying one at that: pretty rare, but affects lots of tests, and no satisfying solutions (skip test under stress? Mess with timeouts?).

The reason our leases have an expiration is because we want the ability to fail over in case of an unexpected node crash. But in our TestCluster integration tests, this is not a concern and we should be able to pin leases by essentially giving them an infinite lifetime. (Such leases can still be shortened later). Liveness makes this a little more annoying, but we could likely just use "vanilla" expiration-based leases by default, which may be the direction CRDB as a whole is heading as well⁵

So one concrete suggestion would be for (TestCluster).TransferLease to default to giving out long-lived expiration-based leases (you'd have to opt into shorter leases with a config option and tests would fail of nodes in the cluster got shut down without this option). Additionally, automatic lease transfers (replicate queue / store rebalancer) should be off by default. This can be seen as a refinement of the ReplicationMode. Note that there is evidence that a number of tests are "accidentally" using the default ReplicationAuto which also allows for rebalances and lease transfers - this is often explicitly counter to what the test wants⁶

Expiring leases is another use case. The best strategy currently used is to set up the cluster with a hybrid manual clock (which ticks like a clock but can also be accelerated by a delta) and to go through some arcane incantations to forward the clock by the right amount, together with a retry loop just in case a lease acquisition is racing with the clock bump⁷. This could be made easier, too, so that we can "just" call tc.ExpireLeases() or something like that⁸, and this should work by default.

Note: I see that we already have some way to "pin leases":

cockroach/pkg/kv/kvserver/testing_knobs.go

Lines 527 to 551 in 66c9f93

	// PinnedLeasesKnob is a testing know for controlling what store can acquire a
	// lease for specific ranges.
	type PinnedLeasesKnob struct {
	mu struct {
	syncutil.Mutex
	pinned map[roachpb.RangeID]roachpb.StoreID
	}
	}
	// NewPinnedLeases creates a PinnedLeasesKnob.
	func NewPinnedLeases() *PinnedLeasesKnob {
	p := &PinnedLeasesKnob{}
	p.mu.pinned = make(map[roachpb.RangeID]roachpb.StoreID)
	return p
	}
	// PinLease makes it so that only the specified store can take a lease for the
	// specified range. Replicas on other stores attempting to acquire a lease will
	// get NotLeaseHolderErrors. Lease transfers away from the pinned store are
	// permitted.
	func (p *PinnedLeasesKnob) PinLease(rangeID roachpb.RangeID, storeID roachpb.StoreID) {
	p.mu.Lock()
	defer p.mu.Unlock()
	p.mu.pinned[rangeID] = storeID
	}

I assume it works by restricting which raft instance can win elections, and which replica can become leader, by suppressing any message that could lead to an unwanted outcome. Seems like an involved hammer but maybe it can be adopted more, too, and improved.

Jira issue: CRDB-30095

Epic CRDB-39956

Footnotes

1. https://github.com/cockroachdb/cockroach/issues/107200#issuecomment-1647808624 ↩

2. see https://github.com/cockroachdb/cockroach/issues/101824#issuecomment-1614777449 ↩

3. see https://github.com/cockroachdb/cockroach/issues/107523 ↩

4. I think calling TransferLease takes care of this though, so maybe not an actual issue ↩

5. https://github.com/cockroachdb/cockroach/issues/94592 ↩

6. https://github.com/cockroachdb/cockroach/issues/101824#issuecomment-1649893853 ↩

7. https://github.com/cockroachdb/cockroach/pull/107442 ↩

8. see https://github.com/cockroachdb/cockroach/blob/fd6f2830764bf6ca0c94f849ad2aa9cca10fd38b/pkg/testutils/testcluster/testcluster.go#L1099-L1106 for something ~related that we could build out ↩

[blathers-crl]

cc @cockroachdb/replication