-
Notifications
You must be signed in to change notification settings - Fork 6.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for data block hash index for column families with user-defined timestamps #13283
Open
brennan913
wants to merge
23
commits into
facebook:main
Choose a base branch
from
brennan913:main
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+90
−5
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
… the presence of user-defined timestamps
…efined timestamps
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in 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.
Summary
This patch consists of two changes.
Strip user-defined timestamps (UDTs) from user keys during data block hash index creation and lookup. This ensures that the same key with distinct timestamps will hash to the same location, preventing
Get()
s from retrieving outdated values for a given timestamp. This operation is a noop for column families that don't use UDTs (i.e., for whichts_sz
is 0).Add a new API,
Comparator::KeysAreBytewiseComparableOtherThanTimestamp
to enable use ofBlockBasedTableOptions::kDataBlockBinaryAndHash
for column families that use a timestamp-aware comparator with otherwise bytewise-comparable keys. This API provides semantics similar to the existingComparator::CanKeysWithDifferentByteContentsBeEqual
, except it only requires that the user key itself (not including timestamp) is bytwise comparable. This new API returnsfalse
by default, and should be overridden to returntrue
for comparators that support these semantics. To use the data block hash index, a column family should use a comparator that overrides one of these APIs, and selectBlockBasedTableOptions::kDataBlockBinaryAndHash
.Resolves issue #12100
Proposed Semantics
(Note that only one of
KeysAreBytewiseComparableOtherThanTimestamp
andCanKeysWithDifferentByteContentsBeEqual
needs to be overriden)Test Plan
New unit test in
db_with_timestamp_basic_test.cc
based on repro from originally flagged issuemake clean && make all check -j$(nproc)
make jclean rocksdbjava jtest