This change is 

Looks good so far!

Reviewed 2 of 2 files at r1.
Review status: all files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.

pkg/storage/store.go, line 1861 at r1 (raw file):

	// the PushTxnQueue. Note that we pass in an right lease for prevLease so
	// that we don't unnecessarily update the timestamp cache.
	rightRng.leasePostApply(ctx, rightLease, rightReplicaID, rightLease)

Can you just drop the last argument to leasePostApply and instead load prevLease in leasePostApply? In this callsite, we already have rightRng.Lease == rightLease so it will continue to be a noop, and similarly the other call site won't have to fetch the lease manually. I think the same holds for the ReplicaID which in both cases is picked from mu which you may as well do inside the function.

pkg/storage/store.go, line 1861 at r1 (raw file):

	// the PushTxnQueue. Note that we pass in an right lease for prevLease so
	// that we don't unnecessarily update the timestamp cache.
	rightRng.leasePostApply(ctx, rightLease, rightReplicaID, rightLease)

Looking through the git blame for this call to leasePostApply, I found this: #14305

It seems that the original call intentionally wanted things to look like the lease was changing hands so that pushTxnQueue.Clear() is called in leasePostApply. ISTM that we've changed this back now; is the test that was added in that PR not failing? Can you check what's going on there?

Comments from Reviewable

Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions.

pkg/storage/store.go, line 1861 at r1 (raw file):

I reran the storage tests and nothing fails. I read through all of the comments for that PR as well and the main concern was for the pushTxnQueue to be enabled after the call to leasePostApply, which it is because that's only gated by iAmTheLeaseHolder. The tests added in that PR only check to see if the queue is enabled, but don't test the contents.

pkg/storage/store.go, line 1861 at r1 (raw file):

Good point, cleaned up.

Comments from Reviewable

Reviewed 2 of 2 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

pkg/storage/store.go, line 1861 at r1 (raw file):

Oh, gotcha, I was looking at the wrong part of the code, namely the one where the queue is disabled. No problem then!

Comments from Reviewable

See https://teamcity.cockroachdb.com/viewLog.html?buildId=300601&buildTypeId=Cockroach_UnitTests for fixing the CI.

Thanks!

Comments from Reviewable

I think we want to cherry-pick this into 1.0.x. If we do it soon it can go in to 1.0.4 (the cutoff is today).

Review status: 1 of 2 files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.

pkg/storage/store.go, line 1858 at r3 (raw file):

	// Invoke the leasePostApply method to ensure we properly initialize
	// the replica according to whether it holds the lease. This enables
	// the PushTxnQueue. Note that we pass in an right lease for prevLease so

This comment got a little muddled ("an right lease", and there's no longer a prevLease argument).

Comments from Reviewable