diff --git a/HISTORY.md b/HISTORY.md index 1affbf25b..340590d49 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,4 +1,8 @@ # Rocksdb Change Log +## 6.11.3 (7/9/2020) +### Bug Fixes +* 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.1 (6/23/2020) ### Bug Fixes * Best-efforts recovery ignores CURRENT file completely. If CURRENT file is missing during recovery, best-efforts recovery still proceeds with MANIFEST file(s). diff --git a/db/db_test2.cc b/db/db_test2.cc index 58503ff9f..07485760b 100644 --- a/db/db_test2.cc +++ b/db/db_test2.cc @@ -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 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 diff --git a/table/block_based/index_builder.cc b/table/block_based/index_builder.cc index 277bec61d..a2c074c76 100644 --- a/table/block_based/index_builder.cc +++ b/table/block_based/index_builder.cc @@ -104,6 +104,15 @@ void PartitionedIndexBuilder::MakeNewSubIndexBuilder() { comparator_, table_opt_.index_block_restart_interval, table_opt_.format_version, use_value_delta_encoding_, table_opt_.index_shortening, /* include_first_key */ false); + + // Set sub_index_builder_->seperator_is_key_plus_seq_ to true if + // seperator_is_key_plus_seq_ is true (internal-key mode) (set to false by + // default on Creation) so that flush policy can point to + // sub_index_builder_->index_block_builder_ + if (seperator_is_key_plus_seq_) { + sub_index_builder_->seperator_is_key_plus_seq_ = true; + } + flush_policy_.reset(FlushBlockBySizePolicyFactory::NewFlushBlockPolicy( table_opt_.metadata_block_size, table_opt_.block_size_deviation, // Note: this is sub-optimal since sub_index_builder_ could later reset @@ -129,9 +138,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,9 +176,15 @@ 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. 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_)); } } }