Allow plain table to store index on file with bloom filter disabled#1525
Allow plain table to store index on file with bloom filter disabled#1525siying wants to merge 1 commit intofacebook:masterfrom
Conversation
IslamAbdelRahman
left a comment
There was a problem hiding this comment.
Thanks @siying for fixing that !
table/plain_table_reader.cc
Outdated
There was a problem hiding this comment.
So now index_in_file will be false even if the index block is in file but the bloom block is not ?
Does that mean that we don't get any benefit from storing the index in file ?
There was a problem hiding this comment.
Good point. Let me fix it.
|
@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
@siying updated the pull request - view changes - changes since last import |
|
@siying updated the pull request - view changes - changes since last import |
|
Some changes to make it the code easier to understand. |
|
@siying updated the pull request - view changes - changes since last import |
|
Revert a previous change in test, which is not needed now. |
Summary: Currently plain table bloom filter is required if storing metadata on file. Remove the constraint. Test Plan: Add a unit test.
|
@siying updated the pull request - view changes - changes since last import |
|
asan, ubsan failure, and the Travis failure don't seem to be related. |
IslamAbdelRahman
left a comment
There was a problem hiding this comment.
LGTM, Thanks @siying
Summary: Currently plain table bloom filter is required if storing metadata on file. Remove the constraint. Closes #1525 Differential Revision: D4190977 Pulled By: siying fbshipit-source-id: be60442
Summary:
Currently plain table bloom filter is required if storing metadata on file. Remove the constraint.
Test Plan: Relax existing unit test to cover this scenario.