Skip to content

Commit

Permalink
Make option level_compaction_dynamic_level_bytes true by default (#…
Browse files Browse the repository at this point in the history
…11525)

Summary:
after #11321 and #11340 (both included in RocksDB v8.2), migration from `level_compaction_dynamic_level_bytes=false` to `level_compaction_dynamic_level_bytes=true` is automatic by RocksDB and requires no manual compaction from user. Making the option true by default as it has several advantages: 1. better space amplification guarantee (a more stable LSM shape). 2. compaction is more adaptive to write traffic. 3. automatic draining of unneeded levels. Wiki is updated with more detail: https://github.com/facebook/rocksdb/wiki/Leveled-Compaction#option-level_compaction_dynamic_level_bytes-and-levels-target-size.

The PR mostly contains fixes for unit tests as they assumed `level_compaction_dynamic_level_bytes=false`. Most notable change is commit f742be3 and b1928e4 which override the default option in DBTestBase to still set `level_compaction_dynamic_level_bytes=false` by default. This helps to reduce the change needed for unit tests. I think this default option override in unit tests is okay since the behavior of `level_compaction_dynamic_level_bytes=true` is tested by explicitly setting this option. Also, `level_compaction_dynamic_level_bytes=false` may be more desired in unit tests as it makes it easier to create a desired LSM shape.

Comment for option `level_compaction_dynamic_level_bytes` is updated to reflect this change and change made in #10057.

Pull Request resolved: #11525

Test Plan: `make -j32 J=32 check` several times to try to catch flaky tests due to this option change.

Reviewed By: ajkr

Differential Revision: D46654256

Pulled By: cbi42

fbshipit-source-id: 6b5827dae124f6f1fdc8cca2ac6f6fcd878830e1
  • Loading branch information
cbi42 authored and facebook-github-bot committed Jun 16, 2023
1 parent 253bc91 commit bc04ec8
Show file tree
Hide file tree
Showing 23 changed files with 170 additions and 135 deletions.
12 changes: 10 additions & 2 deletions db/column_family_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1255,6 +1255,7 @@ TEST_P(ColumnFamilyTest, DifferentCompactionStyles) {
ColumnFamilyOptions default_cf, one, two;
db_options_.max_open_files = 20; // only 10 files in file cache

default_cf.level_compaction_dynamic_level_bytes = false;
default_cf.compaction_style = kCompactionStyleLevel;
default_cf.num_levels = 3;
default_cf.write_buffer_size = 64 << 10; // 64KB
Expand All @@ -1272,6 +1273,7 @@ TEST_P(ColumnFamilyTest, DifferentCompactionStyles) {
one.level0_file_num_compaction_trigger = 4;
one.write_buffer_size = 120000;

two.level_compaction_dynamic_level_bytes = false;
two.compaction_style = kCompactionStyleLevel;
two.num_levels = 4;
two.level0_file_num_compaction_trigger = 3;
Expand Down Expand Up @@ -1326,6 +1328,7 @@ TEST_P(ColumnFamilyTest, MultipleManualCompactions) {
db_options_.max_open_files = 20; // only 10 files in file cache
db_options_.max_background_compactions = 3;

default_cf.level_compaction_dynamic_level_bytes = false;
default_cf.compaction_style = kCompactionStyleLevel;
default_cf.num_levels = 3;
default_cf.write_buffer_size = 64 << 10; // 64KB
Expand All @@ -1342,6 +1345,7 @@ TEST_P(ColumnFamilyTest, MultipleManualCompactions) {
one.level0_file_num_compaction_trigger = 4;
one.write_buffer_size = 120000;

two.level_compaction_dynamic_level_bytes = false;
two.compaction_style = kCompactionStyleLevel;
two.num_levels = 4;
two.level0_file_num_compaction_trigger = 3;
Expand Down Expand Up @@ -1424,13 +1428,14 @@ TEST_P(ColumnFamilyTest, AutomaticAndManualCompactions) {
db_options_.max_open_files = 20; // only 10 files in file cache
db_options_.max_background_compactions = 3;

default_cf.level_compaction_dynamic_level_bytes = false;
default_cf.compaction_style = kCompactionStyleLevel;
default_cf.num_levels = 3;
default_cf.write_buffer_size = 64 << 10; // 64KB
default_cf.target_file_size_base = 30 << 10;
default_cf.max_compaction_bytes = default_cf.target_file_size_base * 1100;
BlockBasedTableOptions table_options = GetBlockBasedTableOptions();
;

table_options.no_block_cache = true;
default_cf.table_factory.reset(NewBlockBasedTableFactory(table_options));

Expand All @@ -1441,6 +1446,7 @@ TEST_P(ColumnFamilyTest, AutomaticAndManualCompactions) {
one.level0_file_num_compaction_trigger = 4;
one.write_buffer_size = 120000;

two.level_compaction_dynamic_level_bytes = false;
two.compaction_style = kCompactionStyleLevel;
two.num_levels = 4;
two.level0_file_num_compaction_trigger = 3;
Expand Down Expand Up @@ -1519,13 +1525,14 @@ TEST_P(ColumnFamilyTest, ManualAndAutomaticCompactions) {
db_options_.max_open_files = 20; // only 10 files in file cache
db_options_.max_background_compactions = 3;

default_cf.level_compaction_dynamic_level_bytes = false;
default_cf.compaction_style = kCompactionStyleLevel;
default_cf.num_levels = 3;
default_cf.write_buffer_size = 64 << 10; // 64KB
default_cf.target_file_size_base = 30 << 10;
default_cf.max_compaction_bytes = default_cf.target_file_size_base * 1100;
BlockBasedTableOptions table_options = GetBlockBasedTableOptions();
;

table_options.no_block_cache = true;
default_cf.table_factory.reset(NewBlockBasedTableFactory(table_options));

Expand All @@ -1536,6 +1543,7 @@ TEST_P(ColumnFamilyTest, ManualAndAutomaticCompactions) {
one.level0_file_num_compaction_trigger = 4;
one.write_buffer_size = 120000;

two.level_compaction_dynamic_level_bytes = false;
two.compaction_style = kCompactionStyleLevel;
two.num_levels = 4;
two.level0_file_num_compaction_trigger = 3;
Expand Down
3 changes: 3 additions & 0 deletions db/compact_files_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ TEST_F(CompactFilesTest, L0ConflictsFiles) {
const int kWriteBufferSize = 10000;
const int kLevel0Trigger = 2;
options.create_if_missing = true;
options.level_compaction_dynamic_level_bytes = false;
options.compaction_style = kCompactionStyleLevel;
// Small slowdown and stop trigger for experimental purpose.
options.level0_slowdown_writes_trigger = 20;
Expand Down Expand Up @@ -359,6 +360,7 @@ TEST_F(CompactFilesTest, CompactionFilterWithGetSv) {
std::shared_ptr<FilterWithGet> cf(new FilterWithGet());

Options options;
options.level_compaction_dynamic_level_bytes = false;
options.create_if_missing = true;
options.compaction_filter = cf.get();

Expand Down Expand Up @@ -401,6 +403,7 @@ TEST_F(CompactFilesTest, SentinelCompressionType) {
CompactionStyle::kCompactionStyleNone}) {
ASSERT_OK(DestroyDB(db_name_, Options()));
Options options;
options.level_compaction_dynamic_level_bytes = false;
options.compaction_style = compaction_style;
// L0: Snappy, L1: ZSTD, L2: Snappy
options.compression_per_level = {CompressionType::kSnappyCompression,
Expand Down
2 changes: 2 additions & 0 deletions db/compaction/compaction_job_stats_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,7 @@ TEST_P(CompactionJobStatsTest, CompactionJobStatsTest) {
// via AddExpectedStats().
auto* stats_checker = new CompactionJobStatsChecker();
Options options;
options.level_compaction_dynamic_level_bytes = false;
options.listeners.emplace_back(stats_checker);
options.create_if_missing = true;
// just enough setting to hold off auto-compaction.
Expand Down Expand Up @@ -815,6 +816,7 @@ TEST_P(CompactionJobStatsTest, DeletionStatsTest) {
// what we expect.
auto* stats_checker = new CompactionJobDeletionStatsChecker();
Options options;
options.level_compaction_dynamic_level_bytes = false;
options.listeners.emplace_back(stats_checker);
options.create_if_missing = true;
options.level0_file_num_compaction_trigger = kTestScale + 1;
Expand Down
5 changes: 5 additions & 0 deletions db/compaction/compaction_picker_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ class CompactionPickerTestBase : public testing::Test {
mutable_cf_options_.RefreshDerivedOptions(ioptions_);
ioptions_.cf_paths.emplace_back("dummy",
std::numeric_limits<uint64_t>::max());
// When the default value of this option is true, universal compaction
// tests can encounter assertion failure since SanitizeOption() is
// not run to set this option to false. So we do the sanitization
// here. Tests that test this option set this option to true explicitly.
ioptions_.level_compaction_dynamic_level_bytes = false;
}

~CompactionPickerTestBase() override {}
Expand Down
17 changes: 17 additions & 0 deletions db/corruption_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,7 @@ TEST_F(CorruptionTest, TableFile) {

TEST_F(CorruptionTest, VerifyChecksumReadahead) {
Options options;
options.level_compaction_dynamic_level_bytes = false;
SpecialEnv senv(base_env_);
options.env = &senv;
// Disable block cache as we are going to check checksum for
Expand Down Expand Up @@ -503,6 +504,7 @@ TEST_F(CorruptionTest, VerifyChecksumReadahead) {

TEST_F(CorruptionTest, TableFileIndexData) {
Options options;
options.level_compaction_dynamic_level_bytes = false;
// very big, we'll trigger flushes manually
options.write_buffer_size = 100 * 1024 * 1024;
Reopen(&options);
Expand Down Expand Up @@ -659,6 +661,7 @@ TEST_F(CorruptionTest, CorruptedDescriptor) {

TEST_F(CorruptionTest, CompactionInputError) {
Options options;
options.level_compaction_dynamic_level_bytes = false;
options.env = env_.get();
Reopen(&options);
Build(10);
Expand All @@ -680,6 +683,7 @@ TEST_F(CorruptionTest, CompactionInputError) {

TEST_F(CorruptionTest, CompactionInputErrorParanoid) {
Options options;
options.level_compaction_dynamic_level_bytes = false;
options.env = env_.get();
options.paranoid_checks = true;
options.write_buffer_size = 131072;
Expand Down Expand Up @@ -777,6 +781,7 @@ TEST_F(CorruptionTest, RangeDeletionCorrupted) {
TEST_F(CorruptionTest, FileSystemStateCorrupted) {
for (int iter = 0; iter < 2; ++iter) {
Options options;
options.level_compaction_dynamic_level_bytes = false;
options.env = env_.get();
options.paranoid_checks = true;
options.create_if_missing = true;
Expand Down Expand Up @@ -816,6 +821,7 @@ static const auto& corruption_modes = {

TEST_F(CorruptionTest, ParanoidFileChecksOnFlush) {
Options options;
options.level_compaction_dynamic_level_bytes = false;
options.env = env_.get();
options.check_flush_compaction_key_order = false;
options.paranoid_file_checks = true;
Expand Down Expand Up @@ -844,6 +850,7 @@ TEST_F(CorruptionTest, ParanoidFileChecksOnFlush) {

TEST_F(CorruptionTest, ParanoidFileChecksOnCompact) {
Options options;
options.level_compaction_dynamic_level_bytes = false;
options.env = env_.get();
options.paranoid_file_checks = true;
options.create_if_missing = true;
Expand Down Expand Up @@ -877,6 +884,7 @@ TEST_F(CorruptionTest, ParanoidFileChecksOnCompact) {

TEST_F(CorruptionTest, ParanoidFileChecksWithDeleteRangeFirst) {
Options options;
options.level_compaction_dynamic_level_bytes = false;
options.env = env_.get();
options.check_flush_compaction_key_order = false;
options.paranoid_file_checks = true;
Expand Down Expand Up @@ -913,6 +921,7 @@ TEST_F(CorruptionTest, ParanoidFileChecksWithDeleteRangeFirst) {

TEST_F(CorruptionTest, ParanoidFileChecksWithDeleteRange) {
Options options;
options.level_compaction_dynamic_level_bytes = false;
options.env = env_.get();
options.check_flush_compaction_key_order = false;
options.paranoid_file_checks = true;
Expand Down Expand Up @@ -952,6 +961,7 @@ TEST_F(CorruptionTest, ParanoidFileChecksWithDeleteRange) {

TEST_F(CorruptionTest, ParanoidFileChecksWithDeleteRangeLast) {
Options options;
options.level_compaction_dynamic_level_bytes = false;
options.env = env_.get();
options.check_flush_compaction_key_order = false;
options.paranoid_file_checks = true;
Expand Down Expand Up @@ -988,6 +998,7 @@ TEST_F(CorruptionTest, ParanoidFileChecksWithDeleteRangeLast) {

TEST_F(CorruptionTest, LogCorruptionErrorsInCompactionIterator) {
Options options;
options.level_compaction_dynamic_level_bytes = false;
options.env = env_.get();
options.create_if_missing = true;
options.allow_data_in_errors = true;
Expand Down Expand Up @@ -1017,6 +1028,7 @@ TEST_F(CorruptionTest, LogCorruptionErrorsInCompactionIterator) {

TEST_F(CorruptionTest, CompactionKeyOrderCheck) {
Options options;
options.level_compaction_dynamic_level_bytes = false;
options.env = env_.get();
options.paranoid_file_checks = false;
options.create_if_missing = true;
Expand Down Expand Up @@ -1044,6 +1056,7 @@ TEST_F(CorruptionTest, CompactionKeyOrderCheck) {

TEST_F(CorruptionTest, FlushKeyOrderCheck) {
Options options;
options.level_compaction_dynamic_level_bytes = false;
options.env = env_.get();
options.paranoid_file_checks = false;
options.create_if_missing = true;
Expand Down Expand Up @@ -1097,6 +1110,7 @@ TEST_F(CorruptionTest, DisableKeyOrderCheck) {
TEST_F(CorruptionTest, VerifyWholeTableChecksum) {
CloseDb();
Options options;
options.level_compaction_dynamic_level_bytes = false;
options.env = env_.get();
ASSERT_OK(DestroyDB(dbname_, options));
options.create_if_missing = true;
Expand Down Expand Up @@ -1182,6 +1196,7 @@ INSTANTIATE_TEST_CASE_P(CorruptionTest, CrashDuringRecoveryWithCorruptionTest,
TEST_P(CrashDuringRecoveryWithCorruptionTest, CrashDuringRecovery) {
CloseDb();
Options options;
options.level_compaction_dynamic_level_bytes = false;
options.track_and_verify_wals_in_manifest =
track_and_verify_wals_in_manifest_;
options.wal_recovery_mode = WALRecoveryMode::kPointInTimeRecovery;
Expand Down Expand Up @@ -1354,6 +1369,7 @@ TEST_P(CrashDuringRecoveryWithCorruptionTest, CrashDuringRecovery) {
TEST_P(CrashDuringRecoveryWithCorruptionTest, TxnDbCrashDuringRecovery) {
CloseDb();
Options options;
options.level_compaction_dynamic_level_bytes = false;
options.wal_recovery_mode = WALRecoveryMode::kPointInTimeRecovery;
options.track_and_verify_wals_in_manifest =
track_and_verify_wals_in_manifest_;
Expand Down Expand Up @@ -1551,6 +1567,7 @@ TEST_P(CrashDuringRecoveryWithCorruptionTest, TxnDbCrashDuringRecovery) {
TEST_P(CrashDuringRecoveryWithCorruptionTest, CrashDuringRecoveryWithFlush) {
CloseDb();
Options options;
options.level_compaction_dynamic_level_bytes = false;
options.wal_recovery_mode = WALRecoveryMode::kPointInTimeRecovery;
options.avoid_flush_during_recovery = false;
options.env = env_.get();
Expand Down
1 change: 1 addition & 0 deletions db/cuckoo_table_db_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ class CuckooTableDBTest : public testing::Test {

Options CurrentOptions() {
Options options;
options.level_compaction_dynamic_level_bytes = false;
options.table_factory.reset(NewCuckooTableFactory());
options.memtable_factory.reset(NewHashLinkListRepFactory(4, 0, 3, true));
options.allow_mmap_reads = true;
Expand Down
11 changes: 3 additions & 8 deletions db/db_compaction_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8001,10 +8001,8 @@ TEST_F(DBCompactionTest, ChangeLevelErrorPathTest) {
}

TEST_F(DBCompactionTest, CompactionWithBlob) {
Options options;
options.env = env_;
Options options = CurrentOptions();
options.disable_auto_compactions = true;

Reopen(options);

constexpr char first_key[] = "first_key";
Expand Down Expand Up @@ -8096,10 +8094,8 @@ INSTANTIATE_TEST_CASE_P(DBCompactionTestBlobError, DBCompactionTestBlobError,
"BlobFileBuilder::WriteBlobToFile:AppendFooter"}));

TEST_P(DBCompactionTestBlobError, CompactionError) {
Options options;
Options options = CurrentOptions();
options.disable_auto_compactions = true;
options.env = env_;

Reopen(options);

constexpr char first_key[] = "first_key";
Expand Down Expand Up @@ -8265,8 +8261,7 @@ TEST_P(DBCompactionTestBlobGC, CompactionWithBlobGCOverrides) {
}

TEST_P(DBCompactionTestBlobGC, CompactionWithBlobGC) {
Options options;
options.env = env_;
Options options = CurrentOptions();
options.disable_auto_compactions = true;
options.enable_blob_files = true;
options.blob_file_size = 32; // one blob per file
Expand Down
15 changes: 4 additions & 11 deletions db/db_merge_operand_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,8 @@ TEST_F(DBMergeOperandTest, CacheEvictedMergeOperandReadAfterFreeBug) {
// There was a bug of reading merge operands after they are mistakely freed
// in DB::GetMergeOperands, which is surfaced by cache full.
// See PR#9507 for more.
Options options;
options.create_if_missing = true;
Options options = CurrentOptions();
options.merge_operator = MergeOperators::CreateStringAppendOperator();
options.env = env_;
BlockBasedTableOptions table_options;

// Small cache to simulate cache full
Expand Down Expand Up @@ -121,11 +119,9 @@ TEST_F(DBMergeOperandTest, FlushedMergeOperandReadAfterFreeBug) {
}

TEST_F(DBMergeOperandTest, GetMergeOperandsBasic) {
Options options;
options.create_if_missing = true;
Options options = CurrentOptions();
// Use only the latest two merge operands.
options.merge_operator = std::make_shared<LimitedStringAppendMergeOp>(2, ',');
options.env = env_;
Reopen(options);
int num_records = 4;
int number_of_operands = 0;
Expand Down Expand Up @@ -309,13 +305,11 @@ TEST_F(DBMergeOperandTest, GetMergeOperandsBasic) {
}

TEST_F(DBMergeOperandTest, BlobDBGetMergeOperandsBasic) {
Options options;
options.create_if_missing = true;
Options options = CurrentOptions();
options.enable_blob_files = true;
options.min_blob_size = 0;
// Use only the latest two merge operands.
options.merge_operator = std::make_shared<LimitedStringAppendMergeOp>(2, ',');
options.env = env_;
Reopen(options);
int num_records = 4;
int number_of_operands = 0;
Expand Down Expand Up @@ -401,8 +395,7 @@ TEST_F(DBMergeOperandTest, GetMergeOperandsLargeResultOptimization) {
const int kNumOperands = 1024;
const int kOperandLen = 1024;

Options options;
options.create_if_missing = true;
Options options = CurrentOptions();
options.merge_operator = MergeOperators::CreateStringAppendOperator();
DestroyAndReopen(options);

Expand Down
Loading

0 comments on commit bc04ec8

Please sign in to comment.