andreacavalli/rocksdb
0
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

fix calling SetOptions on deprecated options

Browse Source 
Summary:
In `cf_options_type_info`, the deprecated options are all considered to have offset zero in the `MutableCFOptions` struct. Previously we weren't checking in `GetMutableOptionsFromStrings` whether the provided option was deprecated or not and simply writing the provided value to the offset specified by `cf_options_type_info`. That meant setting any deprecated option would overwrite the first element in the struct, which is `write_buffer_size`. `db_stress` hit this often since it calls `SetOptions` with `soft_rate_limit=0` and `hard_rate_limit=0`, which are both deprecated so cause `write_buffer_size` to be set to zero, which causes it to crash on the following assertion:

```
db_stress: db/memtable.cc:106: rocksdb::MemTable::MemTable(const rocksdb::InternalKeyComparator&, const rocksdb::ImmutableCFOptions&, const rocksdb::MutableCFOptions&, rocksdb::WriteBufferManager*, rocksdb::SequenceNumber, uint32_t): Assertion `!ShouldScheduleFlush()' failed.
```

We fix it by skipping deprecated options (and logging a warning) when users provide them to `SetOptions`. I didn't want to fail the call for compatibility reasons.
Closes https://github.com/facebook/rocksdb/pull/3700

Differential Revision: D7572596

Pulled By: ajkr

fbshipit-source-id: bd5d84e14c0c39f30c5d4c6df7c1503d2c28ecf1
This commit is contained in:
Andrew Kryczka 2018-04-10 18:48:49 -07:00 committed by Facebook Github Bot
parent d95014b9df
commit 019d7894eb
3 changed files with 12 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
db/column_family.cc
View File

@ -1148,8 +1148,9 @@ void ColumnFamilyData::ResetThreadLocalSuperVersions() {
Status ColumnFamilyData::SetOptions(
const std::unordered_map<std::string, std::string>& options_map) {
MutableCFOptions new_mutable_cf_options;
Status s = GetMutableOptionsFromStrings(mutable_cf_options_, options_map,
&new_mutable_cf_options);
Status s =
GetMutableOptionsFromStrings(mutable_cf_options_, options_map,
ioptions_.info_log, &new_mutable_cf_options);
if (s.ok()) {
mutable_cf_options_ = new_mutable_cf_options;
mutable_cf_options_.RefreshDerivedOptions(ioptions_);

9
options/options_helper.cc
View File

@ -704,7 +704,7 @@ bool SerializeSingleOptionHelper(const char* opt_address,
Status GetMutableOptionsFromStrings(
const MutableCFOptions& base_options,
const std::unordered_map<std::string, std::string>& options_map,
MutableCFOptions* new_options) {
Logger* info_log, MutableCFOptions* new_options) {
assert(new_options);
*new_options = base_options;
for (const auto& o : options_map) {
@ -717,6 +717,13 @@ Status GetMutableOptionsFromStrings(
if (!opt_info.is_mutable) {
return Status::InvalidArgument("Option not changeable: " + o.first);
}
if (opt_info.verification == OptionVerificationType::kDeprecated) {
// log warning when user tries to set a deprecated option but don't fail
// the call for compatibility.
ROCKS_LOG_WARN(info_log, "%s is a deprecated option and cannot be set",
o.first.c_str());
continue;
}
bool is_ok = ParseOptionHelper(
reinterpret_cast<char*>(new_options) + opt_info.mutable_offset,
opt_info.type, o.second);

2
options/options_helper.h
View File

@ -31,7 +31,7 @@ ColumnFamilyOptions BuildColumnFamilyOptions(
Status GetMutableOptionsFromStrings(
const MutableCFOptions& base_options,
const std::unordered_map<std::string, std::string>& options_map,
MutableCFOptions* new_options);
Logger* info_log, MutableCFOptions* new_options);
Status GetMutableDBOptionsFromStrings(
const MutableDBOptions& base_options,