Introduce the block version ref key #1969
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
poodlewars
force-pushed
the
aseaton/20241031/mega-vref-key
branch
from
619af2a
to
0044bdf
Compare
willdealtry
approved these changes
Nov 4, 2024
cpp/arcticdb/version/block_key.hpp
Outdated
|
|
||
| BlockKey(KeyType key_type, StreamId id, std::unordered_map<StreamId, AtomKey> keys); | ||
|
|
||
| BlockKey block_with_same_keys(StreamId new_id); |
cpp/arcticdb/version/block_key.hpp
Outdated
| bool valid_{true}; | ||
| KeyType block_key_type_; | ||
| StreamId id_; | ||
| KeyType expected_key_type_of_contents_; |
There was a problem hiding this comment.
probably expected_key_type_ is fine
cpp/arcticdb/version/block_key.cpp
Outdated
|
|
||
| namespace arcticdb::version_store { | ||
|
|
||
| BlockKey::BlockKey(KeyType key_type, StreamId id, SegmentInMemory &&segment) { |
There was a problem hiding this comment.
I'm not totally sold on the name, I don't think it really reflects what the class does. Even KeyBlock might be better as it expresses what it is - i.e. a block of keys.
poodlewars
force-pushed
the
aseaton/20241031/mega-vref-key
branch
from
efd06a8
to
4a691ba
Compare
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Useful for background jobs.
See design doc shared internally - https://manwiki.maninvestments.com/x/n4CsFg .
test_snapshot.cpphas rotted and wasn't included in any cmake targets (so hasn't compiled for months or ever). Since it didn't check anything very useful, I've just removed it here.An alternative design would be to store the segment sorted by stream ID, like we do for snapshots, and work with it directly rather than transforming it in to an
unordered_map. This would save us the cost of transforming the segment to theunordered_mapand back again. I plan to see how the current implementation performs in replication and we can swap out to the other idea at any time until we release this (the point of no return is once unordered block ref keys are serialized in production).