kvserver: Run one ScanInterleavedIntents per store concurrently

[itsbilal]

Currently, lock table migration related requests (Barrier,
ScanInterleavedIntents, PushTxn, ResolveIntent), are only run
with a concurrency of 4 regardless of the size of the cluster.

This change increases that to 4 * numNodes, and adds a new
mechanism to limit the ScanInterleavedIntents on the receiver
side to 1 per store. This throttle is applied before latches
are grabbed. Only ScanInterleavedIntents needs to be
rate-limited as it's by far the heaviest component in
the separated intents migrations.

Release justification: Category 2, low-risk update to new functionality
Release note: None.

[cockroach-teamcity]

This change is 

[dt]

I think we might actually want to make this setting-tunable. Like, these are basically like ExportRequests, right? They both have to scan a large span and look for intents. You could potentially just copy/paste the ConcurrentExportRequests limiter, adding to the limiters field that is already an attribute of each store, to avoid needing to keep your own map, and so you'd also have a Settings.SV around to setup the on-change hook. I dunno.

[itsbilal]

Thanks for the review! Yep, making it settings-tunable is probably a good idea. That way we can stuff the 4x node multiplier default into the cluster setting and have that drive that multiplier instead. I'll make that change shortly. Addressed the other points.

[itsbilal]

Done. Slight update to last comment: I didn't put the 4x fan-out-time node multiplier factor in the setting, but rather the 1x store multiplier in it. That's less useful as there's no easy way to "slow it down" by reducing it any lower than 1 (0 would effectively stop rate limiting as the limiter will just return an error upon Begin); the operator could only speed it up from the default if they want to get the migration out of the way. Maybe that's fine, but something worth noting.

[sumeerbhola]

Reviewed 6 of 8 files at r1, 4 of 4 files at r4, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @itsbilal)

---

pkg/kv/kvserver/replica_send.go, line 288 at r4 (raw file):

		reservation, err := r.store.limiters.ConcurrentScanInterleavedIntents.Begin(ctx)
		if err != nil {
			return nil

why are we ignoring the error?

---

pkg/kv/kvserver/replica_send.go, line 355 at r4 (raw file):

	// Handle any request-specific pre-latch throttles.
	if res := r.maybeThrottleRequestPreLatch(ctx, ba); res != nil {

Why do this here instead of earlier in Store.Send in maybeThrottleBatch -- it fits well with the other logic there?

[itsbilal]

Dismissed @sumeerbhola from 2 discussions.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dt, @itsbilal, and @sumeerbhola)

---

pkg/kv/kvserver/replica_send.go, line 292 at r1 (raw file):

Previously, dt (David Taylor) wrote…

2¢: I donno why I'm slightly uneasy about the panic, as well as the map lookup, plus being getOrCreate here (we would never want to create, then release). I might vote to have maybeThrottle just return a limiter.Reservation and if the caller gets a non-nil reservation, they have to defer releasing it.

Done.

---

pkg/kv/kvserver/replica_send.go, line 288 at r4 (raw file):

Previously, sumeerbhola wrote…

why are we ignoring the error?

Done.

---

pkg/kv/kvserver/replica_send.go, line 355 at r4 (raw file):

Previously, sumeerbhola wrote…

Why do this here instead of earlier in Store.Send in maybeThrottleBatch -- it fits well with the other logic there?

Done. I had missed that as I was looking at replica_send.go and not store_send.go.

---

pkg/kv/kvserver/batcheval/cmd_scan_interleaved_intents.go, line 25 at r1 (raw file):

Previously, dt (David Taylor) wrote…

I think we like this thing better (that sem isn't fair) https://github.com/cockroachdb/cockroach/blob/master/pkg/util/limit/limiter.go#L47

Done.

[sumeerbhola]

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

[itsbilal]

TFTRs!

bors r=dt

[craig]

Build succeeded:

• GitHub CI (Cockroach)