From 9aa1b1dc199e173c36fb4f7b6102f9898077b6a9 Mon Sep 17 00:00:00 2001 From: Yanqin Jin Date: Fri, 13 Nov 2020 11:51:24 -0800 Subject: [PATCH] Hack to load OPTIONS file for read_amp_bytes_per_bit (#7659) Summary: A temporary hack to work around a bug in 6.10, 6.11, 6.12, 6.13 and 6.14. The bug will write out 8 bytes to OPTIONS file from the starting address of BlockBasedTableOptions.read_amp_bytes_per_bit which is actually a uint32. Consequently, the value of read_amp_bytes_per_bit written in the OPTIONS file is wrong. From 6.15, RocksDB will try to parse the read_amp_bytes_per_bit from OPTIONS file as a uint32. To be able to load OPTIONS file generated by affected releases before the fix, we need to manually parse read_amp_bytes_per_bit with this hack. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7659 Test Plan: Generate a db with current 6.14.fb (head at https://github.com/facebook/rocksdb/commit/b6db05dbb5364c658c5401a8078d73697bb5f31d). Maybe use db_stress. Checkout this PR, run ``` ~/rocksdb/ldb --db=. --try_load_options --ignore_unknown_options idump --count_only ``` Expect success, and should not see ``` Failed: Invalid argument: Error parsing read_amp_bytes_per_bit:17179869184 ``` Also make check Reviewed By: anand1976 Differential Revision: D24954752 Pulled By: riversand963 fbshipit-source-id: c7b802fc3e52acd050a4fc1cd475016122234394 --- options/options_test.cc | 10 +++++++++- table/block_based/block_based_table_factory.cc | 17 ++++++++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/options/options_test.cc b/options/options_test.cc index ba39b622f..a916e8568 100644 --- a/options/options_test.cc +++ b/options/options_test.cc @@ -846,7 +846,12 @@ TEST_F(OptionsTest, GetBlockBasedTableOptionsFromString) { "block_cache=1M;block_cache_compressed=1k;block_size=1024;" "block_size_deviation=8;block_restart_interval=4;" "format_version=5;whole_key_filtering=1;" - "filter_policy=bloomfilter:4.567:false;", + "filter_policy=bloomfilter:4.567:false;" + // A bug caused read_amp_bytes_per_bit to be a large integer in OPTIONS + // file generated by 6.10 to 6.14. Though bug is fixed in these releases, + // we need to handle the case of loading OPTIONS file generated before the + // fix. + "read_amp_bytes_per_bit=17179869185;", &new_opt)); ASSERT_TRUE(new_opt.cache_index_and_filter_blocks); ASSERT_EQ(new_opt.index_type, BlockBasedTableOptions::kHashSearch); @@ -867,6 +872,9 @@ TEST_F(OptionsTest, GetBlockBasedTableOptionsFromString) { EXPECT_EQ(bfp->GetMillibitsPerKey(), 4567); EXPECT_EQ(bfp->GetWholeBitsPerKey(), 5); EXPECT_EQ(bfp->GetMode(), BloomFilterPolicy::kAutoBloom); + // Verify that only the lower 32bits are stored in + // new_opt.read_amp_bytes_per_bit. + EXPECT_EQ(1U, new_opt.read_amp_bytes_per_bit); // unknown option Status s = GetBlockBasedTableOptionsFromString( diff --git a/table/block_based/block_based_table_factory.cc b/table/block_based/block_based_table_factory.cc index ca6a27f40..1fe4ca58a 100644 --- a/table/block_based/block_based_table_factory.cc +++ b/table/block_based/block_based_table_factory.cc @@ -360,7 +360,22 @@ static std::unordered_map {"read_amp_bytes_per_bit", {offsetof(struct BlockBasedTableOptions, read_amp_bytes_per_bit), OptionType::kUInt32T, OptionVerificationType::kNormal, - OptionTypeFlags::kNone}}, + OptionTypeFlags::kNone, + [](const ConfigOptions& /*opts*/, const std::string& /*name*/, + const std::string& value, char* addr) { + // A workaround to fix a bug in 6.10, 6.11, 6.12, 6.13 + // and 6.14. The bug will write out 8 bytes to OPTIONS file from the + // starting address of BlockBasedTableOptions.read_amp_bytes_per_bit + // which is actually a uint32. Consequently, the value of + // read_amp_bytes_per_bit written in the OPTIONS file is wrong. + // From 6.15, RocksDB will try to parse the read_amp_bytes_per_bit + // from OPTIONS file as a uint32. To be able to load OPTIONS file + // generated by affected releases before the fix, we need to + // manually parse read_amp_bytes_per_bit with this special hack. + uint64_t read_amp_bytes_per_bit = ParseUint64(value); + EncodeFixed32(addr, static_cast(read_amp_bytes_per_bit)); + return Status::OK(); + }}}, {"enable_index_compression", {offsetof(struct BlockBasedTableOptions, enable_index_compression), OptionType::kBoolean, OptionVerificationType::kNormal,