KAFKA-14491: [19/N] Combine versioned store RocksDB instances into one

[vcrfxia]

The RocksDB-based versioned store implementation introduced in #13188 currently uses two physical RocksDB instances per store instance: one for the "latest value store" and another for the "segments store." This PR combines those two RocksDB instances into one by representing the latest value store as a special "reserved" segment within the segments store. This reserved segment has segment ID -1, is never expired, and is not included in the regular Segments methods for getting or creating segments, but is represented in the physical RocksDB instance the same way as any other segment.

Committer Checklist (excluded from commit message)

• Verify design and implementation
• Verify test coverage and CI build status
• Verify documentation (including upgrade notes)

[vcrfxia]

Now that negative segment ID is allowed (only for reserved segments within LogicalKeyValueSegments), it's possible that a valid segment ID will overflow when incremented, and we want to handle this gracefully instead of throwing an exception when it happens.

[vcrfxia]

Only one reserved segment is needed for the versioned store implementation (used for the latest value store) but I've chosen to implement support for reserved segments more generally in this class. If we think this is unnecessarily complex, I can update this to be a single Optional<LogicalKeyValueSegment> (or equivalent) instead.

[vcrfxia]

This cleanup is unrelated to the changes in this PR -- I happened to notice that this method is labeled as visible for testing but is actually called in a number of places from production code, so I've removed the misleading comment.

[mjsax]

Overall LGTM.

[mjsax]

Do we need incrementWithoutOverflow here, of could we just use null?

[vcrfxia]

For a logical segment, a range query with toBound == null means that all data in the segment (after the specified fromBound) should be returned. But data from other segments should NOT be returned, which is why we need to specify the incremented prefix as the upper bound for the range scan in the physical store. Except in the edge case where incrementing the prefix would cause overflow, in which case using null for the upper bound is correct. Thus, incrementWithoutOverflow() is correct here.

[mjsax]

a range query with toBound == null means

You mean to here, which is the input parameter (not toBound which is the physical bound for the store)?

Except in the edge case where incrementing the prefix would cause overflow, in which case using null for the upper bound is correct.

But if we overflow, and set toBound = null (as returned by incrementWithoutOverflow()) would it not imply we scan the physical store to its end, beyond the current logical segment bound?

[vcrfxia]

You mean to here, which is the input parameter

Yeah sorry, that's what I meant.

But if we overflow, and set toBound = null (as returned by incrementWithoutOverflow()) would it not imply we scan the physical store to its end, beyond the current logical segment bound?

If the prefix overflows then that means that this is the last possible segment in the store, in which case scanning to the end of the physical store is the same as scanning to the end of the segment, so that's actually exactly what we want. (This only works because all prefixes are the same length -- a bunch of the other logic in this class would break too if that were not the case, though, so it's a fine assumption.)

[mjsax]

Ah. Thanks for clarifying -- this make sense!

[mjsax]

Should this not be range(null, null) ?

[vcrfxia]

Good catch! Fixed.

[mjsax]

as above: range(null, null) ?

[mjsax]

Nice side fix.

[mjsax]

Same compile error as on the other PR. Re-triggered Jenkins. Let's see what happens.