diff --git a/db/db_compaction_test.cc b/db/db_compaction_test.cc index 701b5755c..b5033b66f 100644 --- a/db/db_compaction_test.cc +++ b/db/db_compaction_test.cc @@ -3592,6 +3592,80 @@ TEST_F(DBCompactionTest, LevelPeriodicCompaction) { rocksdb::SyncPoint::GetInstance()->DisableProcessing(); } +TEST_F(DBCompactionTest, LevelPeriodicCompactionWithOldDB) { + // This test makes sure that periodic compactions are working with a DB + // where file_creation_time of some files is 0. + // After compactions the new files are created with a valid file_creation_time + + const int kNumKeysPerFile = 32; + const int kNumFiles = 4; + const int kValueSize = 100; + + Options options = CurrentOptions(); + options.max_open_files = -1; // needed for ttl compaction + env_->time_elapse_only_sleep_ = false; + options.env = env_; + + env_->addon_time_.store(0); + DestroyAndReopen(options); + + int periodic_compactions = 0; + bool set_file_creation_time_to_zero = true; + bool set_creation_time_to_zero = true; + rocksdb::SyncPoint::GetInstance()->SetCallBack( + "LevelCompactionPicker::PickCompaction:Return", [&](void* arg) { + Compaction* compaction = reinterpret_cast(arg); + auto compaction_reason = compaction->compaction_reason(); + if (compaction_reason == CompactionReason::kPeriodicCompaction) { + periodic_compactions++; + } + }); + rocksdb::SyncPoint::GetInstance()->SetCallBack( + "PropertyBlockBuilder::AddTableProperty:Start", [&](void* arg) { + TableProperties* props = reinterpret_cast(arg); + if (set_file_creation_time_to_zero) { + props->file_creation_time = 0; + } + if (set_creation_time_to_zero) { + props->creation_time = 0; + } + }); + rocksdb::SyncPoint::GetInstance()->EnableProcessing(); + + Random rnd(301); + for (int i = 0; i < kNumFiles; ++i) { + for (int j = 0; j < kNumKeysPerFile; ++j) { + ASSERT_OK( + Put(Key(i * kNumKeysPerFile + j), RandomString(&rnd, kValueSize))); + } + Flush(); + // Move the first two files to L2. + if (i == 1) { + MoveFilesToLevel(2); + set_creation_time_to_zero = false; + } + } + ASSERT_OK(dbfull()->TEST_WaitForCompact()); + + ASSERT_EQ("2,0,2", FilesPerLevel()); + ASSERT_EQ(0, periodic_compactions); + + Close(); + + set_file_creation_time_to_zero = false; + // Forward the clock by 2 days. + env_->addon_time_.fetch_add(2 * 24 * 60 * 60); + options.periodic_compaction_seconds = 1 * 24 * 60 * 60; // 1 day + + Reopen(options); + ASSERT_OK(dbfull()->TEST_WaitForCompact()); + ASSERT_EQ("2,0,2", FilesPerLevel()); + // Make sure that all files go through periodic compaction. + ASSERT_EQ(kNumFiles, periodic_compactions); + + rocksdb::SyncPoint::GetInstance()->DisableProcessing(); +} + TEST_F(DBCompactionTest, LevelPeriodicAndTtlCompaction) { const int kNumKeysPerFile = 32; const int kNumLevelFiles = 2; diff --git a/db/version_set.cc b/db/version_set.cc index e916fed6e..fae4c157a 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -2358,9 +2358,32 @@ void VersionStorageInfo::ComputeFilesMarkedForPeriodicCompaction( for (auto f : files_[level]) { if (!f->being_compacted && f->fd.table_reader != nullptr && f->fd.table_reader->GetTableProperties() != nullptr) { - auto file_creation_time = + // Compute a file's modification time in the following order: + // 1. Use file_creation_time table property if it is > 0. + // 2. Use creation_time table property if it is > 0. + // 3. Use file's mtime metadata if the above two table properties are 0. + // Don't consider the file at all if the modification time cannot be + // correctly determined based on the above conditions. + uint64_t file_modification_time = f->fd.table_reader->GetTableProperties()->file_creation_time; - if (file_creation_time > 0 && file_creation_time < allowed_time_limit) { + if (file_modification_time == 0) { + file_modification_time = + f->fd.table_reader->GetTableProperties()->creation_time; + } + if (file_modification_time == 0) { + auto file_path = TableFileName(ioptions.cf_paths, f->fd.GetNumber(), + f->fd.GetPathId()); + status = ioptions.env->GetFileModificationTime( + file_path, &file_modification_time); + if (!status.ok()) { + ROCKS_LOG_WARN(ioptions.info_log, + "Can't get file modification time: %s: %s", + file_path.c_str(), status.ToString().c_str()); + continue; + } + } + if (file_modification_time > 0 && + file_modification_time < allowed_time_limit) { files_marked_for_periodic_compaction_.emplace_back(level, f); } } diff --git a/include/rocksdb/advanced_options.h b/include/rocksdb/advanced_options.h index 74e99b2e0..c88a6c17d 100644 --- a/include/rocksdb/advanced_options.h +++ b/include/rocksdb/advanced_options.h @@ -646,6 +646,12 @@ struct AdvancedColumnFamilyOptions { // Files older than this value will be picked up for compaction, and // re-written to the same level as they were before. + // + // A file's age is computed by looking at file_creation_time or creation_time + // table properties in order, if they have valid non-zero values; if not, the + // age is based on the file's last modified time (given by the underlying + // Env). + // // Only supported in Level compaction. // Pre-req: max_open_file == -1. // unit: seconds. Ex: 7 days = 7 * 24 * 60 * 60 diff --git a/table/meta_blocks.cc b/table/meta_blocks.cc index 21d478bb7..661ea85e6 100644 --- a/table/meta_blocks.cc +++ b/table/meta_blocks.cc @@ -18,6 +18,7 @@ #include "table/table_properties_internal.h" #include "util/coding.h" #include "util/file_reader_writer.h" +#include "util/sync_point.h" namespace rocksdb { @@ -67,6 +68,9 @@ void PropertyBlockBuilder::Add( } void PropertyBlockBuilder::AddTableProperty(const TableProperties& props) { + TEST_SYNC_POINT_CALLBACK("PropertyBlockBuilder::AddTableProperty:Start", + const_cast(&props)); + Add(TablePropertiesNames::kRawKeySize, props.raw_key_size); Add(TablePropertiesNames::kRawValueSize, props.raw_value_size); Add(TablePropertiesNames::kDataSize, props.data_size);