From 106be9f824b72a65ab30dbbf115b4a0466244b8e Mon Sep 17 00:00:00 2001 From: Yi Wu Date: Sun, 24 Jun 2018 11:40:09 -0700 Subject: [PATCH 1/3] BlobDB: is_fifo=true also evict non-TTL blob files --- utilities/blob_db/blob_db_impl.cc | 59 +++++++++++++++---------------- utilities/blob_db/blob_db_impl.h | 4 +-- utilities/blob_db/blob_db_test.cc | 27 +++++++++++--- 3 files changed, 51 insertions(+), 39 deletions(-) diff --git a/utilities/blob_db/blob_db_impl.cc b/utilities/blob_db/blob_db_impl.cc index 164178f1757..9e36de3200a 100644 --- a/utilities/blob_db/blob_db_impl.cc +++ b/utilities/blob_db/blob_db_impl.cc @@ -54,14 +54,13 @@ WalFilter::WalProcessingOption BlobReconcileWalFilter::LogRecordFound( bool blobf_compare_ttl::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; - } - if (lhs->expiration_range_.first > rhs->expiration_range_.first) { - return false; + if (lhs->HasTTL() && rhs->HasTTL()) { + return lhs->expiration_range_.second > rhs->expiration_range_.second; + } else if (!lhs->HasTTL() && !rhs->HasTTL()) { + return lhs->BlobFileNumber() > rhs->BlobFileNumber(); + } else { + return !lhs->HasTTL(); } - return lhs->BlobFileNumber() < rhs->BlobFileNumber(); } BlobDBImpl::BlobDBImpl(const std::string& dbname, @@ -852,14 +851,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()); fifo_eviction_seq_ = GetLatestSequenceNumber(); WriteLock l(&mutex_); @@ -883,14 +877,25 @@ Status BlobDBImpl::CheckSizeAndEvictBlobFiles(uint64_t blob_size, } assert(blob_file->Immutable()); auto expiration_range = blob_file->GetExpirationRange(); - ROCKS_LOG_INFO(db_options_.info_log, - "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); + if (blob_file->HasTTL()) { + ROCKS_LOG_INFO(db_options_.info_log, + "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); + } else { + ROCKS_LOG_INFO(db_options_.info_log, + "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 " without TTL.", + live_sst_size, total_blob_size_.load(), + 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 +1746,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 d3e810deb0c..ccd2f43a2f6 100644 --- a/utilities/blob_db/blob_db_impl.h +++ b/utilities/blob_db/blob_db_impl.h @@ -315,9 +315,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; } diff --git a/utilities/blob_db/blob_db_test.cc b/utilities/blob_db/blob_db_test.cc index 653b0accad6..ff3fde7889d 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,35 @@ TEST_F(BlobDBTest, FIFOEviction) { // key1 will exist until corresponding file be deleted. VerifyDB({{"key1", value}, {"key2", value}}); + // Adding another 100 bytes blob without TTL. Data with TTL will get + // evicted first. + 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) { From f1ddde4744bf0cfc547724df24b5156a3a2696bd Mon Sep 17 00:00:00 2001 From: Yi Wu Date: Mon, 25 Jun 2018 16:35:04 -0700 Subject: [PATCH 2/3] is_fifo=true evict files purely based on file number --- utilities/blob_db/blob_db.h | 4 +--- utilities/blob_db/blob_db_impl.cc | 8 +------- utilities/blob_db/blob_db_test.cc | 3 +-- 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/utilities/blob_db/blob_db.h b/utilities/blob_db/blob_db.h index 183d23a8cd8..de175df05e5 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 9e36de3200a..600a7f83738 100644 --- a/utilities/blob_db/blob_db_impl.cc +++ b/utilities/blob_db/blob_db_impl.cc @@ -54,13 +54,7 @@ WalFilter::WalProcessingOption BlobReconcileWalFilter::LogRecordFound( bool blobf_compare_ttl::operator()(const std::shared_ptr& lhs, const std::shared_ptr& rhs) const { - if (lhs->HasTTL() && rhs->HasTTL()) { - return lhs->expiration_range_.second > rhs->expiration_range_.second; - } else if (!lhs->HasTTL() && !rhs->HasTTL()) { - return lhs->BlobFileNumber() > rhs->BlobFileNumber(); - } else { - return !lhs->HasTTL(); - } + return lhs->BlobFileNumber() > rhs->BlobFileNumber(); } BlobDBImpl::BlobDBImpl(const std::string& dbname, diff --git a/utilities/blob_db/blob_db_test.cc b/utilities/blob_db/blob_db_test.cc index ff3fde7889d..f0b2b1a4766 100644 --- a/utilities/blob_db/blob_db_test.cc +++ b/utilities/blob_db/blob_db_test.cc @@ -1128,8 +1128,7 @@ TEST_F(BlobDBTest, FIFOEviction) { // key1 will exist until corresponding file be deleted. VerifyDB({{"key1", value}, {"key2", value}}); - // Adding another 100 bytes blob without TTL. Data with TTL will get - // evicted first. + // 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. From 23ca34586de25a528e5c6a9c58ad28aa74f83589 Mon Sep 17 00:00:00 2001 From: Yi Wu Date: Mon, 25 Jun 2018 17:03:58 -0700 Subject: [PATCH 3/3] address comment. Separate blobf_compare into two comparators --- utilities/blob_db/blob_db_impl.cc | 46 ++++++++++++++++--------------- utilities/blob_db/blob_db_impl.h | 9 ++++-- utilities/blob_db/blob_db_test.cc | 2 +- utilities/blob_db/blob_file.h | 3 +- 4 files changed, 34 insertions(+), 26 deletions(-) diff --git a/utilities/blob_db/blob_db_impl.cc b/utilities/blob_db/blob_db_impl.cc index 600a7f83738..5aa1ce092f4 100644 --- a/utilities/blob_db/blob_db_impl.cc +++ b/utilities/blob_db/blob_db_impl.cc @@ -52,11 +52,25 @@ 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; + } + if (lhs->expiration_range_.first > rhs->expiration_range_.first) { + return false; + } + return lhs->BlobFileNumber() < rhs->BlobFileNumber(); +} + BlobDBImpl::BlobDBImpl(const std::string& dbname, const BlobDBOptions& blob_db_options, const DBOptions& db_options, @@ -847,7 +861,7 @@ Status BlobDBImpl::CheckSizeAndEvictBlobFiles(uint64_t blob_size, std::vector> candidate_files; CopyBlobFiles(&candidate_files); std::sort(candidate_files.begin(), candidate_files.end(), - blobf_compare_ttl()); + BlobFileComparator()); fifo_eviction_seq_ = GetLatestSequenceNumber(); WriteLock l(&mutex_); @@ -871,25 +885,13 @@ Status BlobDBImpl::CheckSizeAndEvictBlobFiles(uint64_t blob_size, } assert(blob_file->Immutable()); auto expiration_range = blob_file->GetExpirationRange(); - if (blob_file->HasTTL()) { - ROCKS_LOG_INFO(db_options_.info_log, - "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); - } else { - ROCKS_LOG_INFO(db_options_.info_log, - "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 " without TTL.", - live_sst_size, total_blob_size_.load(), - bdb_options_.max_db_size, blob_file->BlobFileNumber()); - } + ROCKS_LOG_INFO(db_options_.info_log, + "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 + ".", + live_sst_size, total_blob_size_.load(), + 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); diff --git a/utilities/blob_db/blob_db_impl.h b/utilities/blob_db/blob_db_impl.h index ccd2f43a2f6..3fe8683866c 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; }; @@ -371,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 f0b2b1a4766..5ab2656b93a 100644 --- a/utilities/blob_db/blob_db_test.cc +++ b/utilities/blob_db/blob_db_test.cc @@ -1128,7 +1128,7 @@ TEST_F(BlobDBTest, FIFOEviction) { // key1 will exist until corresponding file be deleted. VerifyDB({{"key1", value}, {"key2", value}}); - // Adding another 100 bytes blob without TTL. + // 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. diff --git a/utilities/blob_db/blob_file.h b/utilities/blob_db/blob_file.h index 0e41a129d0b..288523e7738 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