dolysis/rocksdb
3
0
Fork 0
mirror of https://github.com/facebook/rocksdb.git synced 2024-12-04 20:02:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

Bug fix and test BuildDBOptions (#13038)

Browse source 
Summary:
The following DBOptions were not being propagated through BuildDBOptions, which could at least lead to settings being lost through `GetOptionsFromString()`, possibly elsewhere as well:
* background_close_inactive_wals
* write_dbid_to_manifest
* write_identity_file
* prefix_seek_opt_in_only

Pull Request resolved: https://github.com/facebook/rocksdb/pull/13038

Test Plan:
This problem was not being caught by
OptionsSettableTest.DBOptionsAllFieldsSettable when the option was omitted from both options_helper.cc and options_settable_test.cc. I have added to the test to catch future instances (and the updated test was how I found three of the four missing options).

The same kind of bug seems to be caught by
ColumnFamilyOptionsAllFieldsSettable, and AFAIK analogous code does not exist for BlockBasedTableOptions.

Reviewed By: ltamasi

Differential Revision: D63483779

Pulled By: pdillinger

fbshipit-source-id: a5d5f6e434174bacb8e5d251b767e81e62b7225a
This commit is contained in:
Peter Dillinger 2024-09-26 14:36:29 -07:00
parent ade981b5b4
commit 786ac6a0e9
4 changed files with 31 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

19
options/options_helper.cc
View file

@ -55,7 +55,13 @@ Status ValidateOptions(const DBOptions& db_opts,
DBOptions BuildDBOptions(const ImmutableDBOptions& immutable_db_options,
const MutableDBOptions& mutable_db_options) {
DBOptions options;
BuildDBOptions(immutable_db_options, mutable_db_options, options);
return options;
}
void BuildDBOptions(const ImmutableDBOptions& immutable_db_options,
const MutableDBOptions& mutable_db_options,
DBOptions& options) {
options.create_if_missing = immutable_db_options.create_if_missing;
options.create_missing_column_families =
immutable_db_options.create_missing_column_families;
@ -88,9 +94,6 @@ DBOptions BuildDBOptions(const ImmutableDBOptions& immutable_db_options,
options.max_background_jobs = mutable_db_options.max_background_jobs;
options.max_background_compactions =
mutable_db_options.max_background_compactions;
options.bytes_per_sync = mutable_db_options.bytes_per_sync;
options.wal_bytes_per_sync = mutable_db_options.wal_bytes_per_sync;
options.strict_bytes_per_sync = mutable_db_options.strict_bytes_per_sync;
options.max_subcompactions = mutable_db_options.max_subcompactions;
options.max_background_flushes = mutable_db_options.max_background_flushes;
options.max_log_file_size = immutable_db_options.max_log_file_size;
@ -127,6 +130,9 @@ DBOptions BuildDBOptions(const ImmutableDBOptions& immutable_db_options,
options.writable_file_max_buffer_size =
mutable_db_options.writable_file_max_buffer_size;
options.use_adaptive_mutex = immutable_db_options.use_adaptive_mutex;
options.bytes_per_sync = mutable_db_options.bytes_per_sync;
options.wal_bytes_per_sync = mutable_db_options.wal_bytes_per_sync;
options.strict_bytes_per_sync = mutable_db_options.strict_bytes_per_sync;
options.listeners = immutable_db_options.listeners;
options.enable_thread_tracking = immutable_db_options.enable_thread_tracking;
options.delayed_write_rate = mutable_db_options.delayed_write_rate;
@ -161,9 +167,15 @@ DBOptions BuildDBOptions(const ImmutableDBOptions& immutable_db_options,
options.two_write_queues = immutable_db_options.two_write_queues;
options.manual_wal_flush = immutable_db_options.manual_wal_flush;
options.wal_compression = immutable_db_options.wal_compression;
options.background_close_inactive_wals =
immutable_db_options.background_close_inactive_wals;
options.atomic_flush = immutable_db_options.atomic_flush;
options.avoid_unnecessary_blocking_io =
immutable_db_options.avoid_unnecessary_blocking_io;
options.write_dbid_to_manifest = immutable_db_options.write_dbid_to_manifest;
options.write_identity_file = immutable_db_options.write_identity_file;
options.prefix_seek_opt_in_only =
immutable_db_options.prefix_seek_opt_in_only;
options.log_readahead_size = immutable_db_options.log_readahead_size;
options.file_checksum_gen_factory =
immutable_db_options.file_checksum_gen_factory;
@ -189,7 +201,6 @@ DBOptions BuildDBOptions(const ImmutableDBOptions& immutable_db_options,
options.metadata_write_temperature =
immutable_db_options.metadata_write_temperature;
options.wal_write_temperature = immutable_db_options.wal_write_temperature;
return options;
}
ColumnFamilyOptions BuildColumnFamilyOptions(

4
options/options_helper.h
View file

@ -44,6 +44,10 @@ Status ValidateOptions(const DBOptions& db_opts,
DBOptions BuildDBOptions(const ImmutableDBOptions& immutable_db_options,
const MutableDBOptions& mutable_db_options);
// Overwrites `options`
void BuildDBOptions(const ImmutableDBOptions& immutable_db_options,
const MutableDBOptions& mutable_db_options,
DBOptions& options);
ColumnFamilyOptions BuildColumnFamilyOptions(
const ColumnFamilyOptions& ioptions,

12
options/options_settable_test.cc
View file

@ -271,6 +271,12 @@ TEST_F(OptionsSettableTest, DBOptionsAllFieldsSettable) {
ASSERT_GT(unset_bytes_base, 0);
options->~DBOptions();
// Now also check that BuildDBOptions populates everything
FillWithSpecialChar(options_ptr, sizeof(DBOptions), kDBOptionsExcluded);
BuildDBOptions({}, {}, *options);
ASSERT_EQ(unset_bytes_base,
NumUnsetBytes(options_ptr, sizeof(DBOptions), kDBOptionsExcluded));
options = new (options_ptr) DBOptions();
FillWithSpecialChar(options_ptr, sizeof(DBOptions), kDBOptionsExcluded);
@ -372,7 +378,11 @@ TEST_F(OptionsSettableTest, DBOptionsAllFieldsSettable) {
"follower_catchup_retry_count=456;"
"follower_catchup_retry_wait_ms=789;"
"metadata_write_temperature=kCold;"
"wal_write_temperature=kHot;",
"wal_write_temperature=kHot;"
"background_close_inactive_wals=true;"
"write_dbid_to_manifest=true;"
"write_identity_file=true;"
"prefix_seek_opt_in_only=true;",
new_options));
ASSERT_EQ(unset_bytes_base, NumUnsetBytes(new_options_ptr, sizeof(DBOptions),

1
unreleased_history/bug_fixes/build_db_options.md Normal file
View file

@ -0,0 +1 @@
* Several DB option settings could be lost through `GetOptionsFromString()`, possibly elsewhere as well. Affected options, now fixed:`background_close_inactive_wals`, `write_dbid_to_manifest`, `write_identity_file`, `prefix_seek_opt_in_only`