From 6504ae0c4edc5535e0f4176bef66d1581e2c6604 Mon Sep 17 00:00:00 2001 From: sdong Date: Thu, 30 Apr 2020 16:59:16 -0700 Subject: [PATCH] Remove the support of setting CompressionOptions.parallel_threads from 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: https://github.com/facebook/rocksdb/pull/6782 Test Plan: Run all existing tests. Reviewed By: zhichao-cao Differential Revision: D21338614 fbshipit-source-id: bca2dac3cb37d4e6e64b52cbbe8ea749cd848685 --- db/db_options_test.cc | 4 ++-- options/cf_options.cc | 15 ++++----------- options/options_settable_test.cc | 4 ++-- options/options_test.cc | 22 +++++++++++++--------- 4 files changed, 21 insertions(+), 24 deletions(-) diff --git a/db/db_options_test.cc b/db/db_options_test.cc index 23be9cda3..cc9182461 100644 --- a/db/db_options_test.cc +++ b/db/db_options_test.cc @@ -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()); @@ -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(); } diff --git a/options/cf_options.cc b/options/cf_options.cc index 74c13aba4..3466b33e5 100644 --- a/options/cf_options.cc +++ b/options/cf_options.cc @@ -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; diff --git a/options/options_settable_test.cc b/options/options_settable_test.cc index 55d144d26..f386671cf 100644 --- a/options/options_settable_test.cc +++ b/options/options_settable_test.cc @@ -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;" diff --git a/options/options_test.cc b/options/options_test.cc index a60794315..ca59b5ca1 100644 --- a/options/options_test.cc +++ b/options/options_test.cc @@ -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"}, @@ -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); @@ -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"}, @@ -1402,7 +1404,8 @@ 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); @@ -1410,7 +1413,8 @@ TEST_F(OptionsOldApiTest, GetOptionsFromMapTest) { 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);