diff --git a/HISTORY.md b/HISTORY.md index 9d084f6c6..c645a3552 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -13,6 +13,7 @@ * Compressed block cache was automatically disabled with read-only DBs by mistake. Now it is fixed: compressed block cache will be in effective with read-only DB too. * Fix a bug of wrong iterator result if another thread finishes an update and a DB flush between two statement. * Disable file deletion after MANIFEST write/sync failure until db re-open or Resume() so that subsequent re-open will not see MANIFEST referencing deleted SSTs. +* 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. ### Public API Change * `DB::GetDbSessionId(std::string& session_id)` is added. `session_id` stores a unique identifier that gets reset every time the DB is opened. This DB session ID should be unique among all open DB instances on all hosts, and should be unique among re-openings of the same or other DBs. This identifier is recorded in the LOG file on the line starting with "DB Session ID:". diff --git a/db/db_test2.cc b/db/db_test2.cc index e00bf4fb4..95a7f557a 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_)); } } }