From beb19ad0dd41ee6d36572648e9983faae9df0f29 Mon Sep 17 00:00:00 2001 From: Poornima Chozhiyath Raman Date: Wed, 15 Jul 2015 12:28:22 -0700 Subject: [PATCH] Fixing delete files in Trivial move of universal compaction Summary: Trvial move in universal compaction was failing when trying to move files from levels other than 0. This was because the DeleteFile while trivially moving, was only deleting files of level 0 which caused duplication of same file in different levels. This is fixed by passing the right level as argument in the call of DeleteFile while doing trivial move. Test Plan: ./db_test ran successfully with the new test cases. Reviewers: sdong Reviewed By: sdong Subscribers: leveldb, dhruba Differential Revision: https://reviews.facebook.net/D42135 --- db/db_impl.cc | 5 ++- db/db_test.cc | 88 ++++++++++++++++++++++++++++++++++++++++++++ util/db_test_util.cc | 2 +- 3 files changed, 92 insertions(+), 3 deletions(-) diff --git a/db/db_impl.cc b/db/db_impl.cc index afae447df..c626813c3 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -2542,12 +2542,13 @@ Status DBImpl::BackgroundCompaction(bool* madeProgress, JobContext* job_context, int32_t moved_files = 0; int64_t moved_bytes = 0; for (unsigned int l = 0; l < c->num_input_levels(); l++) { - if (l == static_cast(c->output_level())) { + if (l == static_cast(c->output_level()) || + (c->output_level() == 0)) { continue; } for (size_t i = 0; i < c->num_input_files(l); i++) { FileMetaData* f = c->input(l, i); - c->edit()->DeleteFile(c->level(), f->fd.GetNumber()); + c->edit()->DeleteFile(c->level(l), f->fd.GetNumber()); c->edit()->AddFile(c->output_level(), f->fd.GetNumber(), f->fd.GetPathId(), f->fd.GetFileSize(), f->smallest, f->largest, f->smallest_seqno, f->largest_seqno, diff --git a/db/db_test.cc b/db/db_test.cc index 86d743787..7a965cdf8 100644 --- a/db/db_test.cc +++ b/db/db_test.cc @@ -2729,6 +2729,94 @@ TEST_F(DBTest, TrivialMoveToLastLevelWithFiles) { rocksdb::SyncPoint::GetInstance()->DisableProcessing(); } +// Test that checks trivial move in universal compaction +TEST_F(DBTest, UniversalCompactionTrivialMoveTest1) { + int32_t trivial_move = 0; + int32_t non_trivial_move = 0; + rocksdb::SyncPoint::GetInstance()->SetCallBack( + "DBImpl::BackgroundCompaction:TrivialMove", + [&](void* arg) { trivial_move++; }); + rocksdb::SyncPoint::GetInstance()->SetCallBack( + "DBImpl::BackgroundCompaction:NonTrivial", + [&](void* arg) { non_trivial_move++; }); + rocksdb::SyncPoint::GetInstance()->EnableProcessing(); + + Options options; + options.compaction_style = kCompactionStyleUniversal; + options.compaction_options_universal.allow_trivial_move = true; + options.num_levels = 3; + options.write_buffer_size = 100 << 10; // 100KB + options.level0_file_num_compaction_trigger = 3; + options.max_background_compactions = 1; + options.target_file_size_base = 32 * 1024; + options = CurrentOptions(options); + DestroyAndReopen(options); + CreateAndReopenWithCF({"pikachu"}, options); + + // Trigger compaction if size amplification exceeds 110% + options.compaction_options_universal.max_size_amplification_percent = 110; + options = CurrentOptions(options); + ReopenWithColumnFamilies({"default", "pikachu"}, options); + + Random rnd(301); + int num_keys = 150000; + for (int i = 0; i < num_keys; i++) { + ASSERT_OK(Put(1, Key(i), Key(i))); + } + std::vector values; + + ASSERT_OK(Flush(1)); + dbfull()->TEST_WaitForCompact(); + + ASSERT_GT(trivial_move, 0); + ASSERT_EQ(non_trivial_move, 0); + + rocksdb::SyncPoint::GetInstance()->DisableProcessing(); +} +// Test that checks trivial move in universal compaction +TEST_F(DBTest, UniversalCompactionTrivialMoveTest2) { + int32_t trivial_move = 0; + int32_t non_trivial_move = 0; + rocksdb::SyncPoint::GetInstance()->SetCallBack( + "DBImpl::BackgroundCompaction:TrivialMove", + [&](void* arg) { trivial_move++; }); + rocksdb::SyncPoint::GetInstance()->SetCallBack( + "DBImpl::BackgroundCompaction:NonTrivial", + [&](void* arg) { non_trivial_move++; }); + rocksdb::SyncPoint::GetInstance()->EnableProcessing(); + + Options options; + options.compaction_style = kCompactionStyleUniversal; + options.compaction_options_universal.allow_trivial_move = true; + options.num_levels = 15; + options.write_buffer_size = 100 << 10; // 100KB + options.level0_file_num_compaction_trigger = 8; + options.max_background_compactions = 4; + options.target_file_size_base = 64 * 1024; + options = CurrentOptions(options); + DestroyAndReopen(options); + CreateAndReopenWithCF({"pikachu"}, options); + + // Trigger compaction if size amplification exceeds 110% + options.compaction_options_universal.max_size_amplification_percent = 110; + options = CurrentOptions(options); + ReopenWithColumnFamilies({"default", "pikachu"}, options); + + Random rnd(301); + int num_keys = 500000; + for (int i = 0; i < num_keys; i++) { + ASSERT_OK(Put(1, Key(i), Key(i))); + } + std::vector values; + + ASSERT_OK(Flush(1)); + dbfull()->TEST_WaitForCompact(); + + ASSERT_GT(trivial_move, 0); + ASSERT_EQ(non_trivial_move, 0); + + rocksdb::SyncPoint::GetInstance()->DisableProcessing(); +} TEST_F(DBTest, CompactionTrigger) { Options options; diff --git a/util/db_test_util.cc b/util/db_test_util.cc index 68959fc65..909ee8973 100644 --- a/util/db_test_util.cc +++ b/util/db_test_util.cc @@ -239,7 +239,7 @@ Options DBTestBase::CurrentOptions( break; case kManifestFileSize: options.max_manifest_file_size = 50; // 50 bytes - case kPerfOptions: + case kPerfOptions: options.soft_rate_limit = 2.0; options.delayed_write_rate = 8 * 1024 * 1024; // TODO(3.13) -- test more options