Update Flush policy in PartitionedIndexBuilder on switching from user-key to internal-key mode (#7022)

Summary:
When format_version is high enough to support user-key and there are index entries for same user key that spans multiple data blocks then it changes from user-key mode to internal-key mode. But the flush policy is not reset to point to Block Builder of internal-keys. After this switch, no entries are added to user key index partition result, thus it never triggers flushing the block.

Fix: After adding the entry in sub_builder_index_, if there is a switch from user-key to internal-key, then flush policy is updated to point to Block Builder of internal-keys index partition.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/7022

Test Plan:
1. make check -j64
           2. Added one unit test case

Reviewed By: ajkr

Differential Revision: D22197734

Pulled By: akankshamahajan15

fbshipit-source-id: d87e9e46bccab8e896ee6979d6b79c51f73d479e
This commit is contained in:
Akanksha Mahajan 2020-07-01 14:56:48 -07:00 committed by Facebook GitHub Bot
parent c25a014792
commit 5edfe3a3d8
3 changed files with 50 additions and 4 deletions

View File

@ -22,6 +22,7 @@
### Bug Fixes
* Fail recovery and report once hitting a physical log record checksum mismatch, while reading MANIFEST. RocksDB should not continue processing the MANIFEST any further.
* Fix a bug when index_type == kTwoLevelIndexSearch in PartitionedIndexBuilder to update FlushPolicy to point to internal key partitioner when it changes from user-key mode to internal-key mode in index partition.
## 6.11 (6/12/2020)
### Bug Fixes

View File

@ -144,6 +144,38 @@ INSTANTIATE_TEST_CASE_P(TestReadOnlyWithCompressedCache,
TestReadOnlyWithCompressedCache,
::testing::Combine(::testing::Values(-1, 100),
::testing::Bool()));
class PartitionedIndexTestListener : public EventListener {
public:
void OnFlushCompleted(DB* /*db*/, const FlushJobInfo& info) override {
ASSERT_GT(info.table_properties.index_partitions, 1);
ASSERT_EQ(info.table_properties.index_key_is_user_key, 0);
}
};
TEST_F(DBTest2, PartitionedIndexUserToInternalKey) {
BlockBasedTableOptions table_options;
Options options = CurrentOptions();
table_options.index_type = BlockBasedTableOptions::kTwoLevelIndexSearch;
PartitionedIndexTestListener* listener = new PartitionedIndexTestListener();
options.table_factory.reset(NewBlockBasedTableFactory(table_options));
options.listeners.emplace_back(listener);
std::vector<const Snapshot*> snapshots;
Reopen(options);
Random rnd(301);
for (int i = 0; i < 3000; i++) {
int j = i % 30;
std::string value = RandomString(&rnd, 10500);
ASSERT_OK(Put("keykey_" + std::to_string(j), value));
snapshots.push_back(db_->GetSnapshot());
}
Flush();
for (auto s : snapshots) {
db_->ReleaseSnapshot(s);
}
}
#endif // ROCKSDB_LITE
class PrefixFullBloomWithReverseComparator

View File

@ -112,6 +112,7 @@ void PartitionedIndexBuilder::MakeNewSubIndexBuilder() {
? sub_index_builder_->index_block_builder_
: sub_index_builder_->index_block_builder_without_seq_));
partition_cut_requested_ = false;
seperator_is_key_plus_seq_ = false;
}
void PartitionedIndexBuilder::RequestPartitionCut() {
@ -129,9 +130,15 @@ void PartitionedIndexBuilder::AddIndexEntry(
}
sub_index_builder_->AddIndexEntry(last_key_in_current_block,
first_key_in_next_block, block_handle);
if (sub_index_builder_->seperator_is_key_plus_seq_) {
// then we need to apply it to all sub-index builders
if (!seperator_is_key_plus_seq_ &&
sub_index_builder_->seperator_is_key_plus_seq_) {
// then we need to apply it to all sub-index builders and reset
// flush_policy to point to Block Builder of sub_index_builder_ that store
// internal keys.
seperator_is_key_plus_seq_ = true;
flush_policy_.reset(FlushBlockBySizePolicyFactory::NewFlushBlockPolicy(
table_opt_.metadata_block_size, table_opt_.block_size_deviation,
sub_index_builder_->index_block_builder_));
}
sub_index_last_key_ = std::string(*last_key_in_current_block);
entries_.push_back(
@ -161,8 +168,14 @@ void PartitionedIndexBuilder::AddIndexEntry(
sub_index_builder_->AddIndexEntry(last_key_in_current_block,
first_key_in_next_block, block_handle);
sub_index_last_key_ = std::string(*last_key_in_current_block);
if (sub_index_builder_->seperator_is_key_plus_seq_) {
// then we need to apply it to all sub-index builders
if (!seperator_is_key_plus_seq_ &&
sub_index_builder_->seperator_is_key_plus_seq_) {
// then we need to apply it to all sub-index builders and reset
// flush_policy to point to Block Builder of sub_index_builder_ that store
// internal keys.
flush_policy_.reset(FlushBlockBySizePolicyFactory::NewFlushBlockPolicy(
table_opt_.metadata_block_size, table_opt_.block_size_deviation,
sub_index_builder_->index_block_builder_));
seperator_is_key_plus_seq_ = true;
}
}