From 4a776d81cc2b3d7ee1c1bb5fda4233f1ef50d494 Mon Sep 17 00:00:00 2001 From: Hui Xiao Date: Fri, 4 Mar 2022 10:35:08 -0800 Subject: [PATCH] Dynamic toggling of BlockBasedTableOptions::detect_filter_construct_corruption (#9654) Summary: **Context/Summary:** As requested, `BlockBasedTableOptions::detect_filter_construct_corruption` can now be dynamically configured using `DB::SetOptions` after this PR Pull Request resolved: https://github.com/facebook/rocksdb/pull/9654 Test Plan: - New unit test Reviewed By: pdillinger Differential Revision: D34622609 Pulled By: hx235 fbshipit-source-id: c06773ef3d029e6bf1724d3a72dffd37a8ec66d9 --- HISTORY.md | 1 + db/db_bloom_filter_test.cc | 87 +++++++++++++++++++ include/rocksdb/table.h | 4 + .../block_based/block_based_table_factory.cc | 2 +- 4 files changed, 93 insertions(+), 1 deletion(-) diff --git a/HISTORY.md b/HISTORY.md index bf6bf618a..01cda51a6 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -3,6 +3,7 @@ ### New Features * Allow WriteBatchWithIndex to index a WriteBatch that includes keys with user-defined timestamps. The index itself does not have timestamp. * Added BlobDB options to `ldb` +* `BlockBasedTableOptions::detect_filter_construct_corruption` can now be dynamically configured using `DB::SetOptions`. ### Bug Fixes * Fixed a data race on `versions_` between `DBImpl::ResumeImpl()` and threads waiting for recovery to complete (#9496) diff --git a/db/db_bloom_filter_test.cc b/db/db_bloom_filter_test.cc index d4e576c6b..9129ca180 100644 --- a/db/db_bloom_filter_test.cc +++ b/db/db_bloom_filter_test.cc @@ -1540,6 +1540,93 @@ TEST_P(DBFilterConstructionCorruptionTestWithParam, DetectCorruption) { "TamperFilter"); } +// RocksDB lite does not support dynamic options +#ifndef ROCKSDB_LITE +TEST_P(DBFilterConstructionCorruptionTestWithParam, + DynamicallyTurnOnAndOffDetectConstructCorruption) { + Options options = CurrentOptions(); + BlockBasedTableOptions table_options = GetBlockBasedTableOptions(); + // We intend to turn on + // table_options.detect_filter_construct_corruption dynamically + // therefore we override this test parmater's value + table_options.detect_filter_construct_corruption = false; + + options.table_factory.reset(NewBlockBasedTableFactory(table_options)); + options.create_if_missing = true; + + int num_key = static_cast(GetNumKey()); + Status s; + + DestroyAndReopen(options); + + // Case 1: !table_options.detect_filter_construct_corruption + for (int i = 0; i < num_key; i++) { + ASSERT_OK(Put(Key(i), Key(i))); + } + + SyncPoint::GetInstance()->SetCallBack( + "XXPH3FilterBitsBuilder::Finish::TamperHashEntries", [&](void* arg) { + std::deque* hash_entries_to_corrupt = + (std::deque*)arg; + assert(!hash_entries_to_corrupt->empty()); + *(hash_entries_to_corrupt->begin()) = + *(hash_entries_to_corrupt->begin()) ^ uint64_t { 1 }; + }); + SyncPoint::GetInstance()->EnableProcessing(); + + s = Flush(); + + SyncPoint::GetInstance()->DisableProcessing(); + SyncPoint::GetInstance()->ClearCallBack( + "XXPH3FilterBitsBuilder::Finish::" + "TamperHashEntries"); + + ASSERT_FALSE(table_options.detect_filter_construct_corruption); + EXPECT_TRUE(s.ok()); + + // Case 2: dynamically turn on + // table_options.detect_filter_construct_corruption + ASSERT_OK(db_->SetOptions({{"block_based_table_factory", + "{detect_filter_construct_corruption=true;}"}})); + + for (int i = 0; i < num_key; i++) { + ASSERT_OK(Put(Key(i), Key(i))); + } + + SyncPoint::GetInstance()->SetCallBack( + "XXPH3FilterBitsBuilder::Finish::TamperHashEntries", [&](void* arg) { + std::deque* hash_entries_to_corrupt = + (std::deque*)arg; + assert(!hash_entries_to_corrupt->empty()); + *(hash_entries_to_corrupt->begin()) = + *(hash_entries_to_corrupt->begin()) ^ uint64_t { 1 }; + }); + SyncPoint::GetInstance()->EnableProcessing(); + + s = Flush(); + + SyncPoint::GetInstance()->DisableProcessing(); + SyncPoint::GetInstance()->ClearCallBack( + "XXPH3FilterBitsBuilder::Finish::" + "TamperHashEntries"); + + auto updated_table_options = + db_->GetOptions().table_factory->GetOptions(); + EXPECT_TRUE(updated_table_options->detect_filter_construct_corruption); + EXPECT_TRUE(s.IsCorruption()); + EXPECT_TRUE(s.ToString().find("Filter's hash entries checksum mismatched") != + std::string::npos); + + // Case 3: dynamically turn off + // table_options.detect_filter_construct_corruption + ASSERT_OK(db_->SetOptions({{"block_based_table_factory", + "{detect_filter_construct_corruption=false;}"}})); + updated_table_options = + db_->GetOptions().table_factory->GetOptions(); + EXPECT_FALSE(updated_table_options->detect_filter_construct_corruption); +} +#endif // ROCKSDB_LITE + namespace { // NOTE: This class is referenced by HISTORY.md as a model for a wrapper // FilterPolicy selecting among configurations based on context. diff --git a/include/rocksdb/table.h b/include/rocksdb/table.h index 8782e94b0..ffb8bd26a 100644 --- a/include/rocksdb/table.h +++ b/include/rocksdb/table.h @@ -371,6 +371,10 @@ struct BlockBasedTableOptions { // useful in detecting software bugs or CPU+memory malfunction. // Turning on this feature increases filter construction time by 30%. // + // This parameter can be changed dynamically by + // DB::SetOptions({{"block_based_table_factory", + // "{detect_filter_construct_corruption=true;}"}}); + // // TODO: optimize this performance bool detect_filter_construct_corruption = false; diff --git a/table/block_based/block_based_table_factory.cc b/table/block_based/block_based_table_factory.cc index 5f13b245b..a391fa06a 100644 --- a/table/block_based/block_based_table_factory.cc +++ b/table/block_based/block_based_table_factory.cc @@ -325,7 +325,7 @@ static std::unordered_map {offsetof(struct BlockBasedTableOptions, detect_filter_construct_corruption), OptionType::kBoolean, OptionVerificationType::kNormal, - OptionTypeFlags::kNone}}, + OptionTypeFlags::kMutable}}, {"reserve_table_builder_memory", {offsetof(struct BlockBasedTableOptions, reserve_table_builder_memory), OptionType::kBoolean, OptionVerificationType::kNormal,