Skip to content

Commit

Permalink
Remove the support of setting CompressionOptions.parallel_threads fro…
Browse files Browse the repository at this point in the history 
…m string for now (#6782)

Summary:
The current way of implementing CompressionOptions.parallel_threads introduces a format change. We plan to change CompressionOptions's serailization format to a new JSON-like format, which would be another format change. We would like to consolidate the two format changes into one, rather than making some users to change twice. Hold CompressionOptions.parallel_threads from being supported by option string for now. Will add it back after the general CompressionOptions's format change.
Pull Request resolved: #6782

Test Plan: Run all existing tests.

Reviewed By: zhichao-cao

Differential Revision: D21338614

fbshipit-source-id: bca2dac3cb37d4e6e64b52cbbe8ea749cd848685
  • Loading branch information
@siying @facebook-github-bot
siying authored and facebook-github-bot committed May 1, 2020
1 parent ef0c3ed commit 6504ae0
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 24 deletions.
4 changes: 2 additions & 2 deletions db/db_options_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -920,7 +920,7 @@ TEST_F(DBOptionsTest, ChangeCompression) {
compacted = false;
ASSERT_OK(dbfull()->SetOptions(
{{"bottommost_compression", "kSnappyCompression"},
{"bottommost_compression_opts", "0:6:0:0:0:4:true"}}));
{"bottommost_compression_opts", "0:6:0:0:4:true"}}));
ASSERT_OK(Put("foo", "foofoofoo"));
ASSERT_OK(Put("bar", "foofoofoo"));
ASSERT_OK(Flush());
Expand All @@ -931,7 +931,7 @@ TEST_F(DBOptionsTest, ChangeCompression) {
ASSERT_TRUE(compacted);
ASSERT_EQ(CompressionType::kSnappyCompression, compression_used);
ASSERT_EQ(6, compression_opt_used.level);
ASSERT_EQ(4u, compression_opt_used.parallel_threads);
// Right now parallel_level is not yet allowed to be changed.

SyncPoint::GetInstance()->DisableProcessing();
}
Expand Down
15 changes: 4 additions & 11 deletions options/cf_options.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,17 +94,10 @@ static Status ParseCompressionOptions(const std::string& value,
ParseInt(value.substr(start, value.size() - start));
end = value.find(':', start);
}
// parallel_threads 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);
}
compression_opts.parallel_threads =
ParseInt(value.substr(start, value.size() - start));
end = value.find(':', start);
}
// parallel_threads is not serialized with this format.
// We plan to upgrade the format to a JSON-like format.
compression_opts.parallel_threads = CompressionOptions().parallel_threads;

// enabled is optional for backwards compatibility
if (end != std::string::npos) {
start = end + 1;
Expand Down
4 changes: 2 additions & 2 deletions options/options_settable_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -454,8 +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;"
"compression_opts=5:6:7:8:9:true;"
"bottommost_compression_opts=4:5:6:7:8:true;"
"bottommost_compression=kDisableCompressionOption;"
"level0_stop_writes_trigger=33;"
"num_levels=99;"
Expand Down
22 changes: 13 additions & 9 deletions options/options_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ TEST_F(OptionsTest, GetOptionsFromMapTest) {
"kZSTD:"
"kZSTDNotFinalCompression"},
{"bottommost_compression", "kLZ4Compression"},
{"bottommost_compression_opts", "5:6:7:8:9:10:true"},
{"compression_opts", "4:5:6:7:8:9:true"},
{"bottommost_compression_opts", "5:6:7:8:10:true"},
{"compression_opts", "4:5:6:7:8:true"},
{"num_levels", "8"},
{"level0_file_num_compaction_trigger", "8"},
{"level0_slowdown_writes_trigger", "9"},
Expand Down Expand Up @@ -175,15 +175,17 @@ TEST_F(OptionsTest, GetOptionsFromMapTest) {
ASSERT_EQ(new_cf_opt.compression_opts.strategy, 6);
ASSERT_EQ(new_cf_opt.compression_opts.max_dict_bytes, 7u);
ASSERT_EQ(new_cf_opt.compression_opts.zstd_max_train_bytes, 8u);
ASSERT_EQ(new_cf_opt.compression_opts.parallel_threads, 9u);
ASSERT_EQ(new_cf_opt.compression_opts.parallel_threads,
CompressionOptions().parallel_threads);
ASSERT_EQ(new_cf_opt.compression_opts.enabled, true);
ASSERT_EQ(new_cf_opt.bottommost_compression, kLZ4Compression);
ASSERT_EQ(new_cf_opt.bottommost_compression_opts.window_bits, 5);
ASSERT_EQ(new_cf_opt.bottommost_compression_opts.level, 6);
ASSERT_EQ(new_cf_opt.bottommost_compression_opts.strategy, 7);
ASSERT_EQ(new_cf_opt.bottommost_compression_opts.max_dict_bytes, 8u);
ASSERT_EQ(new_cf_opt.bottommost_compression_opts.zstd_max_train_bytes, 9u);
ASSERT_EQ(new_cf_opt.bottommost_compression_opts.parallel_threads, 10u);
ASSERT_EQ(new_cf_opt.bottommost_compression_opts.zstd_max_train_bytes, 10u);
ASSERT_EQ(new_cf_opt.bottommost_compression_opts.parallel_threads,
CompressionOptions().parallel_threads);
ASSERT_EQ(new_cf_opt.bottommost_compression_opts.enabled, true);
ASSERT_EQ(new_cf_opt.num_levels, 8);
ASSERT_EQ(new_cf_opt.level0_file_num_compaction_trigger, 8);
Expand Down Expand Up @@ -1297,8 +1299,8 @@ TEST_F(OptionsOldApiTest, GetOptionsFromMapTest) {
"kZSTD:"
"kZSTDNotFinalCompression"},
{"bottommost_compression", "kLZ4Compression"},
{"bottommost_compression_opts", "5:6:7:8:9:10:true"},
{"compression_opts", "4:5:6:7:8:9:true"},
{"bottommost_compression_opts", "5:6:7:8:9:true"},
{"compression_opts", "4:5:6:7:8:true"},
{"num_levels", "8"},
{"level0_file_num_compaction_trigger", "8"},
{"level0_slowdown_writes_trigger", "9"},
Expand Down Expand Up @@ -1402,15 +1404,17 @@ TEST_F(OptionsOldApiTest, GetOptionsFromMapTest) {
ASSERT_EQ(new_cf_opt.compression_opts.strategy, 6);
ASSERT_EQ(new_cf_opt.compression_opts.max_dict_bytes, 7u);
ASSERT_EQ(new_cf_opt.compression_opts.zstd_max_train_bytes, 8u);
ASSERT_EQ(new_cf_opt.compression_opts.parallel_threads, 9u);
ASSERT_EQ(new_cf_opt.compression_opts.parallel_threads,
CompressionOptions().parallel_threads);
ASSERT_EQ(new_cf_opt.compression_opts.enabled, true);
ASSERT_EQ(new_cf_opt.bottommost_compression, kLZ4Compression);
ASSERT_EQ(new_cf_opt.bottommost_compression_opts.window_bits, 5);
ASSERT_EQ(new_cf_opt.bottommost_compression_opts.level, 6);
ASSERT_EQ(new_cf_opt.bottommost_compression_opts.strategy, 7);
ASSERT_EQ(new_cf_opt.bottommost_compression_opts.max_dict_bytes, 8u);
ASSERT_EQ(new_cf_opt.bottommost_compression_opts.zstd_max_train_bytes, 9u);
ASSERT_EQ(new_cf_opt.bottommost_compression_opts.parallel_threads, 10u);
ASSERT_EQ(new_cf_opt.bottommost_compression_opts.parallel_threads,
CompressionOptions().parallel_threads);
ASSERT_EQ(new_cf_opt.bottommost_compression_opts.enabled, true);
ASSERT_EQ(new_cf_opt.num_levels, 8);
ASSERT_EQ(new_cf_opt.level0_file_num_compaction_trigger, 8);
Expand Down

0 comments on commit 6504ae0

Please sign in to comment.