-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
fix some rocksdb api will fail in PlainTable #4254
Conversation
if (!isPlainTable_) { | ||
options.total_order_seek = FLAGS_enable_rocksdb_prefix_filtering; | ||
} else { | ||
options.prefix_same_as_start = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So prefix_same_as_start & extractorLen_ together decide the behavior? And only in plaintable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PlainTable must use prefix-based Seek, so we must set prefix_same_as_start = true;
And for BlockBasedTable, if we have prefix extractor, we just use total_order_seek for simplicity, otherwise we need to judge whether the parameter is longer than extractor length (the way we use in prefix function)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GJ
Don't merge it plz, the funny part is that this will fail in ubuntu2004-clang, but other env will pass. I need to check it later |
What type of PR is this?
What problem(s) does this PR solve?
Issue(s) number:
Description:
This problem is found in nebula-ng. Some of our rocksdb api is not well verified in PlainTable, for example, whether the data is in sst will effect the api result. The main problem is that:
prefix_same_as_start
not only inprefix
, but alsorange
andrangeWithPrefix
.prefix_extractor
need to be modified. Because within PlainTable, if the prefix bloom filter is not inserted, you can't be read byprefix
any more. So to make sure every data could be read, we use the minimum length we use inprefix
, which is 4.This is a simple way to fix the problem. In nebula-ng, I will consider refactor it a bit.
How do you solve it?
Special notes for your reviewer, ex. impact of this fix, design document, etc:
Checklist:
Tests:
Affects:
Release notes:
Please confirm whether to be reflected in release notes and how to describe:
Not related.