row: set TargetBytes for kvfetcher

[tbg]

This finishes up the work in #44925 and completes the TargetBytes functionality. This is then used in kvfetcher, which henceforth aims to return no more than ~1mb per request. Additional commits restore the hotspotsplits roachtest and fix it. Reverting the relevant commit from this PR, the test failed nine out of ten times). With all commits, it passed ten times.

The one question I have whether TargetBytes should be set to a value higher than 1mb to avoid a performance regression (where should I look?).

Fixes #33660.

[cockroach-teamcity]

This change is 

[jordanlewis]

I think the query type that's most sensitive to fiddling with the request batch sizes is something like COUNT on a large table. This minimizes CPU and maximizes data transfer.

[tbg]

SG. Can you give me a large table (tpcc.x?) that you find representative (enough), just so that I have something concrete to work with?

[jordanlewis]

tpch.lineitem or maybe tpcc.order_line? something that takes seconds to return, I don't think it matters much.

[tbg]

tpch.lineitem or maybe tpcc.order_line? something that takes seconds to return, I don't think it matters much.

I just tried this via select count(*) from tpcc.order_line; on a four-node large tpcc cluster (~50gb order_line). The results are... not clear? I tested my binary first, the results kept getting better over time (probably RocksDB cleaning up in the background), best I got right before starting into the master binary was 1m22s. The master binary got pretty consistently 1m15s.

Looking at this again though, for this query I would like the 10k rows limit to be the one taking effect, not the 1mb limit. 1mb ~ 10k rows at an average row size of 100b, which is too small. So I'm just going to bump the limit up to 10mb - I'm mostly trying not to get in the way of "reasonable" scans, all I want to do is curb pathological ones.

[nvanbenschoten]

Reviewed 7 of 7 files at r1, 1 of 1 files at r2, 1 of 1 files at r3, 1 of 1 files at r4.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @RaduBerinde and @tbg)

---

pkg/server/server_test.go, line 469 at r1 (raw file):

				// Iterate through MaxSpanRequestKeys=1..n and TargetBytes=1..m
				// and (where n and m are chosen to reveal the full result set
				// in one page). At each(*) combination, paginate

nit: the comment wrapping here is strange

---

pkg/storage/replica_evaluate.go, line 339 at r1 (raw file):

				baHeader.TargetBytes -= retBytes
			} else {
				baHeader.TargetBytes = -1

Could you add a comment like we have above for this?

[tbg]

bors r=nvanbenschoten

[tbg]

bors r=nvanbenschoten

[craig]

Build succeeded

• GitHub CI (Cockroach)