Make max_dict_bytes optional in options string

Summary:
For backwards compatibility with older option strings, the parser needs
to treat this argument as optional.

Test Plan:
Updated unit test to cover case where compression_opts is present but
max_dict_bytes is omitted.

Reviewers: MarkCallaghan, sdong, IslamAbdelRahman

Reviewed By: IslamAbdelRahman

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D57759
This commit is contained in:
Andrew Kryczka 2016-05-06 11:27:28 -07:00
parent 8a5ec0ec6f
commit 53a86bf61e
2 changed files with 18 additions and 10 deletions

View File

@ -806,20 +806,23 @@ Status ParseColumnFamilyOption(const std::string& name,
new_options->compression_opts.level = new_options->compression_opts.level =
ParseInt(value.substr(start, end - start)); ParseInt(value.substr(start, end - start));
start = end + 1; start = end + 1;
end = value.find(':', start);
if (end == std::string::npos) {
return Status::InvalidArgument(
"unable to parse the specified CF option " + name);
}
new_options->compression_opts.strategy =
ParseInt(value.substr(start, value.size() - start));
start = end + 1;
if (start >= value.size()) { if (start >= value.size()) {
return Status::InvalidArgument( return Status::InvalidArgument(
"unable to parse the specified CF option " + name); "unable to parse the specified CF option " + name);
} }
new_options->compression_opts.max_dict_bytes = end = value.find(':', start);
new_options->compression_opts.strategy =
ParseInt(value.substr(start, value.size() - start)); ParseInt(value.substr(start, value.size() - start));
// max_dict_bytes is optional for backwards compatibility
if (end != std::string::npos) {
start = end + 1;
if (start >= value.size()) {
return Status::InvalidArgument(
"unable to parse the specified CF option " + name);
}
new_options->compression_opts.max_dict_bytes =
ParseInt(value.substr(start, value.size() - start));
}
} else if (name == "compaction_options_fifo") { } else if (name == "compaction_options_fifo") {
new_options->compaction_options_fifo.max_table_files_size = new_options->compaction_options_fifo.max_table_files_size =
ParseUint64(value); ParseUint64(value);

View File

@ -600,9 +600,14 @@ TEST_F(OptionsTest, GetOptionsFromStringTest) {
base_options, base_options,
"write_buffer_size=10;max_write_buffer_number=16;" "write_buffer_size=10;max_write_buffer_number=16;"
"block_based_table_factory={block_cache=1M;block_size=4;};" "block_based_table_factory={block_cache=1M;block_size=4;};"
"create_if_missing=true;max_open_files=1;rate_limiter_bytes_per_sec=1024", "compression_opts=4:5:6;create_if_missing=true;max_open_files=1;"
"rate_limiter_bytes_per_sec=1024",
&new_options)); &new_options));
ASSERT_EQ(new_options.compression_opts.window_bits, 4);
ASSERT_EQ(new_options.compression_opts.level, 5);
ASSERT_EQ(new_options.compression_opts.strategy, 6);
ASSERT_EQ(new_options.compression_opts.max_dict_bytes, 0);
ASSERT_EQ(new_options.write_buffer_size, 10U); ASSERT_EQ(new_options.write_buffer_size, 10U);
ASSERT_EQ(new_options.max_write_buffer_number, 16); ASSERT_EQ(new_options.max_write_buffer_number, 16);
BlockBasedTableOptions new_block_based_table_options = BlockBasedTableOptions new_block_based_table_options =