Skip to content

Commit

Permalink
Merge #93134
Browse files Browse the repository at this point in the history
93134: storage: refactor the iteration for scans r=yuzefovich a=yuzefovich

**storage: refactor the iteration for scans**

This commit refactors the iteration that we have for handling scans and
reverse scans in order to split the "get" part from the "advance key"
part. This is needed for the KV projection pushdown work in order to
avoid copying each key-value pair: unlike with the existing
`pebbleResults` struct where we append all KVs into `repr` buffer (and,
thus, copy each KV anyway), with the projection pushdown work we want to
perform the decoding directly on the unsafe KV that the iterator is
pointing at. With the previous "eager" advancement of the iterator, that
KV pair would get invalidated before we could decode it (to keep only the
needed parts of the KV). Now the decoder will be able to access the
unsafe KV (and copy out the necessary parts) before it is invalidated.

An additional boolean is introduced to indicate whether a new KV was
added into the result set or not (also needed for the pushdown work) but
is currently unused.

Previously, the logic was as:
- seek to the first key
- in the loop:
  - get one KV and advance to the next key (both actions as a single
operation).

Now, the logic becomes:
- seek to the first key
- in the loop:
  - get one KV
  - if the iteration can continue, advance to the next key.

This is implemented by introducing a simple state machine for the
"advance key" functions. The benchmark results show minor changes,
mostly positive, some negative:
https://gist.github.com/yuzefovich/4f976075e8dc9c5da9171c281787432d

Informs: #82323

Epic: CRDB-14837

Release note: None

**storage: eliminate a copy of the key in some cases with reverse scans**

This commit changes the contract of `prevKey` method to require that
a copy is passed in. Previously, the method would make a copy of the key
on its own before using the iterator, but in one of the two places where
`prevKey` is used it would be redundant since the caller (`seekVersion`)
already made a copy.

Epic: None

Release note: None

Co-authored-by: Yahor Yuzefovich <[email protected]>
  • Loading branch information
craig[bot] and yuzefovich committed Dec 10, 2022
2 parents 62f7164 + f9da101 commit dc93fa8
Showing 1 changed file with 208 additions and 103 deletions.
Loading

0 comments on commit dc93fa8

Please sign in to comment.