diff --git a/HISTORY.md b/HISTORY.md index e4b49f044..a65500b1e 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -3,6 +3,9 @@ ### New Features * Added support for pipelined & parallel compression optimization for `BlockBasedTableBuilder`. This optimization makes block building, block compression and block appending a pipeline, and uses multiple threads to accelerate block compression. Users can set `CompressionOptions::parallel_threads` greater than 1 to enable compression parallelism. +### Bug Fixes +* Fix a bug when making options.bottommost_compression, options.compression_opts and options.bottommost_compression_opts dynamically changeable: the modified values are not written to option files or returned back to users when being queried. + ## 6.9.0 (03/29/2020) ### Behavior changes * Since RocksDB 6.8, ttl-based FIFO compaction can drop a file whose oldest key becomes older than options.ttl while others have not. This fix reverts this and makes ttl-based FIFO compaction use the file's flush time as the criterion. This fix also requires that max_open_files = -1 and compaction_options_fifo.allow_compaction = false to function properly. diff --git a/options/options_helper.cc b/options/options_helper.cc index f3ee13ad8..d844888bf 100644 --- a/options/options_helper.cc +++ b/options/options_helper.cc @@ -208,6 +208,10 @@ ColumnFamilyOptions BuildColumnFamilyOptions( cf_opts.paranoid_file_checks = mutable_cf_options.paranoid_file_checks; cf_opts.report_bg_io_stats = mutable_cf_options.report_bg_io_stats; cf_opts.compression = mutable_cf_options.compression; + cf_opts.compression_opts = mutable_cf_options.compression_opts; + cf_opts.bottommost_compression = mutable_cf_options.bottommost_compression; + cf_opts.bottommost_compression_opts = + mutable_cf_options.bottommost_compression_opts; cf_opts.sample_for_compression = mutable_cf_options.sample_for_compression; cf_opts.table_factory = options.table_factory; diff --git a/options/options_settable_test.cc b/options/options_settable_test.cc index 12a0ec9d8..94ca5be1e 100644 --- a/options/options_settable_test.cc +++ b/options/options_settable_test.cc @@ -42,13 +42,14 @@ const char kSpecialChar = 'z'; typedef std::vector> OffsetGap; void FillWithSpecialChar(char* start_ptr, size_t total_size, - const OffsetGap& blacklist) { + const OffsetGap& blacklist, + char special_char = kSpecialChar) { size_t offset = 0; for (auto& pair : blacklist) { - std::memset(start_ptr + offset, kSpecialChar, pair.first - offset); + std::memset(start_ptr + offset, special_char, pair.first - offset); offset = pair.first + pair.second; } - std::memset(start_ptr + offset, kSpecialChar, total_size - offset); + std::memset(start_ptr + offset, special_char, total_size - offset); } int NumUnsetBytes(char* start_ptr, size_t total_size, @@ -71,6 +72,26 @@ int NumUnsetBytes(char* start_ptr, size_t total_size, return total_unset_bytes_base; } +// Return true iff two structs are the same except blacklist fields. +bool CompareBytes(char* start_ptr1, char* start_ptr2, size_t total_size, + const OffsetGap& blacklist) { + size_t offset = 0; + for (auto& pair : blacklist) { + for (; offset < pair.first; offset++) { + if (*(start_ptr1 + offset) != *(start_ptr2 + offset)) { + return false; + } + } + offset = pair.first + pair.second; + } + for (; offset < total_size; offset++) { + if (*(start_ptr1 + offset) != *(start_ptr2 + offset)) { + return false; + } + } + return true; +} + // If the test fails, likely a new option is added to BlockBasedTableOptions // but it cannot be set through GetBlockBasedTableOptionsFromString(), or the // test is not updated accordingly. @@ -373,6 +394,7 @@ TEST_F(OptionsSettableTest, ColumnFamilyOptionsAllFieldsSettable) { ColumnFamilyOptions* options = new (options_ptr) ColumnFamilyOptions(); FillWithSpecialChar(options_ptr, sizeof(ColumnFamilyOptions), kColumnFamilyOptionsBlacklist); + // It based on the behavior of compiler that padding bytes are not changed // when copying the struct. It's prone to failure when compiler behavior // changes. We verify there is unset bytes to detect the case. @@ -395,8 +417,6 @@ TEST_F(OptionsSettableTest, ColumnFamilyOptionsAllFieldsSettable) { // GetColumnFamilyOptionsFromString(): options->rate_limit_delay_max_milliseconds = 33; options->compaction_options_universal = CompactionOptionsUniversal(); - options->compression_opts = CompressionOptions(); - options->bottommost_compression_opts = CompressionOptions(); options->hard_rate_limit = 0; options->soft_rate_limit = 0; options->purge_redundant_kvs_while_flush = false; @@ -434,6 +454,8 @@ TEST_F(OptionsSettableTest, ColumnFamilyOptionsAllFieldsSettable) { "max_bytes_for_level_multiplier=60;" "memtable_factory=SkipListFactory;" "compression=kNoCompression;" + "compression_opts=5:6:7:8:9:10:true;" + "bottommost_compression_opts=4:5:6:7:8:9:true;" "bottommost_compression=kDisableCompressionOption;" "level0_stop_writes_trigger=33;" "num_levels=99;" @@ -470,11 +492,46 @@ TEST_F(OptionsSettableTest, ColumnFamilyOptionsAllFieldsSettable) { NumUnsetBytes(new_options_ptr, sizeof(ColumnFamilyOptions), kColumnFamilyOptionsBlacklist)); + ColumnFamilyOptions rnd_filled_options = *new_options; + options->~ColumnFamilyOptions(); new_options->~ColumnFamilyOptions(); delete[] options_ptr; delete[] new_options_ptr; + + // Test copying to mutabable and immutable options and copy back the mutable + // part. + const OffsetGap kMutableCFOptionsBlacklist = { + {offset_of(&MutableCFOptions::prefix_extractor), + sizeof(std::shared_ptr)}, + {offset_of(&MutableCFOptions::max_bytes_for_level_multiplier_additional), + sizeof(std::vector)}, + {offset_of(&MutableCFOptions::max_file_size), + sizeof(std::vector)}, + }; + + // Construct two pieces of memory with controlled base. This is needed as + // otherwise padding bytes might not be filled. + char* mcfo1_ptr = new char[sizeof(MutableCFOptions)]; + MutableCFOptions* mcfo1 = new (mcfo1_ptr) MutableCFOptions(); + FillWithSpecialChar(mcfo1_ptr, sizeof(MutableCFOptions), + kMutableCFOptionsBlacklist, 'x'); + char* mcfo2_ptr = new char[sizeof(MutableCFOptions)]; + MutableCFOptions* mcfo2 = new (mcfo2_ptr) MutableCFOptions(); + FillWithSpecialChar(mcfo2_ptr, sizeof(MutableCFOptions), + kMutableCFOptionsBlacklist, 'x'); + + rnd_filled_options.num_levels = 66; + *mcfo1 = MutableCFOptions(rnd_filled_options); + ColumnFamilyOptions cfo_back = + BuildColumnFamilyOptions(ColumnFamilyOptions(), *mcfo1); + *mcfo2 = MutableCFOptions(cfo_back); + + ASSERT_TRUE(CompareBytes(mcfo1_ptr, mcfo2_ptr, sizeof(MutableCFOptions), + kMutableCFOptionsBlacklist)); + delete[] mcfo1; + delete[] mcfo2; } #endif // !__clang__ #endif // OS_LINUX || OS_WIN