This change is 

Reviewed 8 of 10 files at r1, 7 of 7 files at r3, 2 of 2 files at r4, 2 of 2 files at r5, 1 of 1 files at r6.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

pkg/storage/queue.go, line 540 at r5 (raw file):

	if purgatoryErr, ok := bq.mu.purgatory[rangeID]; ok {
		cb(purgatoryErr)
		return true

Do we still need the change in r4 if MaybeAddCallback returns true for ranges in purgatory?

pkg/storage/replica_backpressure.go, line 145 at r4 (raw file):

		}) {
			// No split ongoing. We may have raced with its completion.
			return errors.Errorf("range large enough to require backpressure, but no split ongoing")

Do these errors make it back to the client?

What about the race in the comment? Shouldn't we check whether the range is still large enough to require backpressure?

Is it possible that we race with the start of a split? That is, if the range has just gotten large enough to require backpressure but hasn't been added to the split queue yet?

Comments from Reviewable

Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

pkg/storage/queue.go, line 540 at r5 (raw file):

Nope, see below.

pkg/storage/replica_backpressure.go, line 145 at r4 (raw file):

Yes they do, and there's really no good way to avoid the race. It's possible that we race with the start of a split. It's also possible that we race with the completion of a split. Even worse, it's possible that we race with the completion of a split and then the range gets too large again, so it looks like the split never happened. It's even possible that this node isn't even the leaseholder anymore and so the splitQueue rejected the split attempt. This was all concerning me to the point where I was probably going to pull this commit. Conservative backpressure is better than over-aggressive backpressure that may cause client-visible errors. I'm glad that it sounds like you agree.

Comments from Reviewable

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

pkg/storage/replica_backpressure.go, line 145 at r4 (raw file):

PTAL.

Comments from Reviewable

Reviewed 1 of 8 files at r7, 2 of 2 files at r8, 1 of 1 files at r9.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

TFTR!

bors r+

Build succeeded

• GitHub CI (Cockroach)