From 6d454d7376b835c1ec668ee152fba4752603a3c8 Mon Sep 17 00:00:00 2001 From: Yi Wu Date: Mon, 25 Jun 2018 22:32:29 -0700 Subject: [PATCH] BlobDB: is_fifo=true also evict non-TTL blob files (#4049) Summary: Previously with is_fifo=true we only evict TTL file. Changing it to also evict non-TTL files from oldest to newest, after exhausted TTL files. Closes https://github.com/facebook/rocksdb/pull/4049 Differential Revision: D8604597 Pulled By: yiwu-arbug fbshipit-source-id: bc4209ee27c1528ce4b72833e6f1e1bff80082c1 --- utilities/blob_db/blob_db.h | 4 +--- utilities/blob_db/blob_db_impl.cc | 37 +++++++++++++------------------ utilities/blob_db/blob_db_impl.h | 13 ++++++----- utilities/blob_db/blob_db_test.cc | 26 +++++++++++++++++----- utilities/blob_db/blob_file.h | 3 ++- 5 files changed, 47 insertions(+), 36 deletions(-) diff --git a/utilities/blob_db/blob_db.h b/utilities/blob_db/blob_db.h index 183d23a8cd..de175df05e 100644 --- a/utilities/blob_db/blob_db.h +++ b/utilities/blob_db/blob_db.h @@ -38,9 +38,7 @@ struct BlobDBOptions { // When max_db_size is reached, evict blob files to free up space // instead of returnning NoSpace error on write. Blob files will be - // evicted in this order until enough space is free up: - // * the TTL blob file cloeset to expire, - // * the oldest non-TTL blob file. + // evicted from oldest to newest, based on file creation time. bool is_fifo = false; // Maximum size of the database (including SST files and blob files). diff --git a/utilities/blob_db/blob_db_impl.cc b/utilities/blob_db/blob_db_impl.cc index 164178f175..5aa1ce092f 100644 --- a/utilities/blob_db/blob_db_impl.cc +++ b/utilities/blob_db/blob_db_impl.cc @@ -52,8 +52,15 @@ WalFilter::WalProcessingOption BlobReconcileWalFilter::LogRecordFound( return WalFilter::WalProcessingOption::kContinueProcessing; } -bool blobf_compare_ttl::operator()(const std::shared_ptr& lhs, - const std::shared_ptr& rhs) const { +bool BlobFileComparator::operator()( + const std::shared_ptr& lhs, + const std::shared_ptr& rhs) const { + return lhs->BlobFileNumber() > rhs->BlobFileNumber(); +} + +bool BlobFileComparatorTTL::operator()( + const std::shared_ptr& lhs, + const std::shared_ptr& rhs) const { assert(lhs->HasTTL() && rhs->HasTTL()); if (lhs->expiration_range_.first < rhs->expiration_range_.first) { return true; @@ -852,14 +859,9 @@ Status BlobDBImpl::CheckSizeAndEvictBlobFiles(uint64_t blob_size, } std::vector> candidate_files; - CopyBlobFiles(&candidate_files, - [&](const std::shared_ptr& blob_file) { - // Only evict TTL files - return blob_file->HasTTL(); - }); + CopyBlobFiles(&candidate_files); std::sort(candidate_files.begin(), candidate_files.end(), - blobf_compare_ttl()); - std::reverse(candidate_files.begin(), candidate_files.end()); + BlobFileComparator()); fifo_eviction_seq_ = GetLatestSequenceNumber(); WriteLock l(&mutex_); @@ -887,10 +889,9 @@ Status BlobDBImpl::CheckSizeAndEvictBlobFiles(uint64_t blob_size, "Evict oldest blob file since DB out of space. Current " "live SST file size: %" PRIu64 ", total blob size: %" PRIu64 ", max db size: %" PRIu64 ", evicted blob file #%" PRIu64 - " with expiration range (%" PRIu64 ", %" PRIu64 ").", + ".", live_sst_size, total_blob_size_.load(), - bdb_options_.max_db_size, blob_file->BlobFileNumber(), - expiration_range.first, expiration_range.second); + bdb_options_.max_db_size, blob_file->BlobFileNumber()); ObsoleteBlobFile(blob_file, fifo_eviction_seq_, true /*update_size*/); evict_expiration_up_to_ = expiration_range.first; RecordTick(statistics_, BLOB_DB_FIFO_NUM_FILES_EVICTED); @@ -1741,18 +1742,10 @@ std::pair BlobDBImpl::DeleteObsoleteFiles(bool aborted) { } void BlobDBImpl::CopyBlobFiles( - std::vector>* bfiles_copy, - std::function&)> predicate) { + std::vector>* bfiles_copy) { ReadLock rl(&mutex_); - for (auto const& p : blob_files_) { - bool pred_value = true; - if (predicate) { - pred_value = predicate(p.second); - } - if (pred_value) { - bfiles_copy->push_back(p.second); - } + bfiles_copy->push_back(p.second); } } diff --git a/utilities/blob_db/blob_db_impl.h b/utilities/blob_db/blob_db_impl.h index d3e810deb0..3fe8683866 100644 --- a/utilities/blob_db/blob_db_impl.h +++ b/utilities/blob_db/blob_db_impl.h @@ -64,7 +64,12 @@ class BlobReconcileWalFilter : public WalFilter { // Comparator to sort "TTL" aware Blob files based on the lower value of // TTL range. -struct blobf_compare_ttl { +struct BlobFileComparatorTTL { + bool operator()(const std::shared_ptr& lhs, + const std::shared_ptr& rhs) const; +}; + +struct BlobFileComparator { bool operator()(const std::shared_ptr& lhs, const std::shared_ptr& rhs) const; }; @@ -315,9 +320,7 @@ class BlobDBImpl : public BlobDB { bool VisibleToActiveSnapshot(const std::shared_ptr& file); bool FileDeleteOk_SnapshotCheckLocked(const std::shared_ptr& bfile); - void CopyBlobFiles( - std::vector>* bfiles_copy, - std::function&)> predicate = {}); + void CopyBlobFiles(std::vector>* bfiles_copy); uint64_t EpochNow() { return env_->NowMicros() / 1000000; } @@ -373,7 +376,7 @@ class BlobDBImpl : public BlobDB { // all the blob files which are currently being appended to based // on variety of incoming TTL's - std::set, blobf_compare_ttl> open_ttl_files_; + std::set, BlobFileComparatorTTL> open_ttl_files_; // Flag to check whether Close() has been called on this DB bool closed_; diff --git a/utilities/blob_db/blob_db_test.cc b/utilities/blob_db/blob_db_test.cc index 653b0accad..5ab2656b93 100644 --- a/utilities/blob_db/blob_db_test.cc +++ b/utilities/blob_db/blob_db_test.cc @@ -1120,7 +1120,7 @@ TEST_F(BlobDBTest, FIFOEviction) { ASSERT_EQ(1, blob_db_impl()->TEST_GetBlobFiles().size()); - // Adding another 100 byte blob would take the total size to 264 bytes + // Adding another 100 bytes blob would take the total size to 264 bytes // (2*132). max_db_size will be exceeded // than max_db_size and trigger FIFO eviction. ASSERT_OK(blob_db_->PutWithTTL(WriteOptions(), "key2", value, 60)); @@ -1128,18 +1128,34 @@ TEST_F(BlobDBTest, FIFOEviction) { // key1 will exist until corresponding file be deleted. VerifyDB({{"key1", value}, {"key2", value}}); + // Adding another 100 bytes blob without TTL. + ASSERT_OK(blob_db_->Put(WriteOptions(), "key3", value)); + ASSERT_EQ(2, evict_count); + // key1 and key2 will exist until corresponding file be deleted. + VerifyDB({{"key1", value}, {"key2", value}, {"key3", value}}); + + // The fourth blob file, without TTL. + ASSERT_OK(blob_db_->Put(WriteOptions(), "key4", value)); + ASSERT_EQ(3, evict_count); + VerifyDB( + {{"key1", value}, {"key2", value}, {"key3", value}, {"key4", value}}); + auto blob_files = blob_db_impl()->TEST_GetBlobFiles(); - ASSERT_EQ(2, blob_files.size()); + ASSERT_EQ(4, blob_files.size()); ASSERT_TRUE(blob_files[0]->Obsolete()); - ASSERT_FALSE(blob_files[1]->Obsolete()); + ASSERT_TRUE(blob_files[1]->Obsolete()); + ASSERT_TRUE(blob_files[2]->Obsolete()); + ASSERT_FALSE(blob_files[3]->Obsolete()); auto obsolete_files = blob_db_impl()->TEST_GetObsoleteFiles(); - ASSERT_EQ(1, obsolete_files.size()); + ASSERT_EQ(3, obsolete_files.size()); ASSERT_EQ(blob_files[0], obsolete_files[0]); + ASSERT_EQ(blob_files[1], obsolete_files[1]); + ASSERT_EQ(blob_files[2], obsolete_files[2]); blob_db_impl()->TEST_DeleteObsoleteFiles(); obsolete_files = blob_db_impl()->TEST_GetObsoleteFiles(); ASSERT_TRUE(obsolete_files.empty()); - VerifyDB({{"key2", value}}); + VerifyDB({{"key4", value}}); } TEST_F(BlobDBTest, FIFOEviction_NoOldestFileToEvict) { diff --git a/utilities/blob_db/blob_file.h b/utilities/blob_db/blob_file.h index 0e41a129d0..288523e773 100644 --- a/utilities/blob_db/blob_file.h +++ b/utilities/blob_db/blob_file.h @@ -23,7 +23,8 @@ class BlobDBImpl; class BlobFile { friend class BlobDBImpl; - friend struct blobf_compare_ttl; + friend struct BlobFileComparator; + friend struct BlobFileComparatorTTL; private: // access to parent