kvstreamer: clean up Result struct

[yuzefovich]

This commit cleans up the Result struct in order to reduce its memory
size, bringing it down into 48 byte size class.

name                             old time/op    new time/op    delta
IndexJoin/Cockroach-24             6.29ms ± 1%    6.22ms ± 2%   -1.06%  (p=0.024 n=9+9)
IndexJoin/MultinodeCockroach-24    7.99ms ± 1%    7.93ms ± 2%     ~     (p=0.165 n=10+10)

name                             old alloc/op   new alloc/op   delta
IndexJoin/Cockroach-24             1.64MB ± 1%    1.48MB ± 0%   -9.25%  (p=0.000 n=9+10)
IndexJoin/MultinodeCockroach-24    2.37MB ± 1%    2.20MB ± 1%   -7.06%  (p=0.000 n=10+10)

name                             old allocs/op  new allocs/op  delta
IndexJoin/Cockroach-24              8.15k ± 1%     7.15k ± 1%  -12.28%  (p=0.000 n=8+10)
IndexJoin/MultinodeCockroach-24     12.7k ± 1%     11.7k ± 1%   -8.18%  (p=0.000 n=10+10)

The main change of this commit is the removal of the concept of "enqueue
keys" from the Streamer API in favor of relying on Result.Position.
When requests are unique, then a single Result can only satisfy
a single enqueue key; however, for non-unique requests a single Result
can satisfy multiple requests and, thus, can have multiple enqueue keys.
At the moment, only unique requests are supported though. Once
non-unique requests are supported too, we'll need to figure out how to
handle those (maybe we'll be returning a Result N number of times if
it satisfies N original requests with different values for
Position).

Also, initially multiple "enqueue keys" support was envisioned for the
case of multiSpanGenerator in the lookup joins (i.e. multi-equality
lookup joins); however, I believe we should push that complexity out of
the streamer (into TxnKVStreamer) which is what this commit does.

Other changes done in this commit:

• unexport ScanResp.Complete field since this is currently only
  used within the kvstreamer package
• reorder all existing fields so that the footprint of the struct is
  minimized (in particular, scanComplete field is moved to the bottom
  and ScanResp anonymous struct is removed)
• make subRequestIdx int32 rather than int. This value is bound
  by the number of ranges in the cluster, so max int32 is more than
  sufficient.

Addresses: #82159.
Addresses: #82160.

Release note: None

[cockroach-teamcity]

This change is 

[rharding6373]

Thanks for the cleanup!

Reviewed 8 of 8 files at r1.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @michae2 and @yuzefovich)

---

-- commits line 26 at r1:
So is there a chance that enqueue keys will be added back when non-unique requests are supported? It seems like we'd need more than the number of duplicate requests in order to handle the non-unique case.

[yuzefovich]

TFTR!

bors r+

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @michae2 and @rharding6373)

---

-- commits line 26 at r1:

Previously, rharding6373 (Rachael Harding) wrote…

So is there a chance that enqueue keys will be added back when non-unique requests are supported? It seems like we'd need more than the number of duplicate requests in order to handle the non-unique case.

Yes, it's possible - I haven't thought through that use case fully. I tried making Result an interface so that depending on Hints.Unique and Hints.SingleRowLookup as well as the streamer mode different objects would be used, but on a quick prototype benchmarks didn't look good. When working on the support for non-unique requests I want to prototype the idea of returning the same Result many number of times rather than specifying all "enqueue keys" that a single Result satisfied, the performance is TBD.

For the time being I'm focusing on the performance and stability aspect of the streamer, and this change seems quite beneficial for that. Non-unique requests' support is not strictly necessary for the lookup joins (because the joinreader itself de-duplicates requests), so maybe we'll have it this way for this release. TBD.

[craig]

Build succeeded:

• GitHub CI (Cockroach)