From f2f034ef3be52ea48b3b94d50a2d1fee2bf0e2f9 Mon Sep 17 00:00:00 2001
From: Yi Wu <yiwu@fb.com>
Date: Fri, 19 Jan 2018 16:19:34 -0800
Subject: [PATCH] Blob DB: fix crash when DB full but no candidate file to
 evict

Summary:
When blob_files is empty, std::min_element will return blobfiles.end(), which cannot be dereference. Fixing it.
Closes https://github.com/facebook/rocksdb/pull/3387

Differential Revision: D6764927

Pulled By: yiwu-arbug

fbshipit-source-id: 86f78700132be95760d35ac63480dfd3a8bbe17a
---
 utilities/blob_db/blob_db_impl.cc |  4 ++++
 utilities/blob_db/blob_db_test.cc | 32 ++++++++++++++++++++++++++++---
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/utilities/blob_db/blob_db_impl.cc b/utilities/blob_db/blob_db_impl.cc
index 999967c449..4e0352614e 100644
--- a/utilities/blob_db/blob_db_impl.cc
+++ b/utilities/blob_db/blob_db_impl.cc
@@ -859,6 +859,9 @@ std::shared_ptr<BlobFile> BlobDBImpl::GetOldestBlobFile() {
   CopyBlobFiles(&blob_files, [](const std::shared_ptr<BlobFile>& f) {
     return !f->Obsolete() && f->Immutable();
   });
+  if (blob_files.empty()) {
+    return nullptr;
+  }
   blobf_compare_ttl compare;
   return *std::min_element(blob_files.begin(), blob_files.end(), compare);
 }
@@ -889,6 +892,7 @@ bool BlobDBImpl::EvictOldestBlobFile() {
                oldest_file->BlobCount());
     RecordTick(statistics_, BLOB_DB_FIFO_BYTES_EVICTED,
                oldest_file->GetFileSize());
+    TEST_SYNC_POINT("BlobDBImpl::EvictOldestBlobFile:Evicted");
     return true;
   }
 
diff --git a/utilities/blob_db/blob_db_test.cc b/utilities/blob_db/blob_db_test.cc
index c0c0fc4901..2fa8fe12ae 100644
--- a/utilities/blob_db/blob_db_test.cc
+++ b/utilities/blob_db/blob_db_test.cc
@@ -1061,15 +1061,19 @@ TEST_F(BlobDBTest, OutOfSpace) {
 }
 
 TEST_F(BlobDBTest, EvictOldestFileWhenCloseToSpaceLimit) {
-  // Use mock env to stop wall clock.
-  Options options;
   BlobDBOptions bdb_options;
   bdb_options.blob_dir_size = 270;
   bdb_options.blob_file_size = 100;
-  bdb_options.disable_background_tasks = true;
   bdb_options.is_fifo = true;
+  bdb_options.disable_background_tasks = true;
   Open(bdb_options);
 
+  std::atomic<int> evict_count{0};
+  SyncPoint::GetInstance()->SetCallBack(
+      "BlobDBImpl::EvictOldestBlobFile:Evicted",
+      [&](void *) { evict_count++; });
+  SyncPoint::GetInstance()->EnableProcessing();
+
   // Each stored blob has an overhead of 32 bytes currently.
   // So a 100 byte blob should take up 132 bytes.
   std::string value(100, 'v');
@@ -1093,6 +1097,28 @@ TEST_F(BlobDBTest, EvictOldestFileWhenCloseToSpaceLimit) {
   bdb_impl->TEST_DeleteObsoleteFiles();
   obsolete_files = bdb_impl->TEST_GetObsoleteFiles();
   ASSERT_TRUE(obsolete_files.empty());
+  ASSERT_EQ(1, evict_count);
+}
+
+TEST_F(BlobDBTest, NoOldestFileToEvict) {
+  Options options;
+  BlobDBOptions bdb_options;
+  bdb_options.blob_dir_size = 1000;
+  bdb_options.blob_file_size = 5000;
+  bdb_options.is_fifo = true;
+  bdb_options.disable_background_tasks = true;
+  Open(bdb_options);
+
+  std::atomic<int> evict_count{0};
+  SyncPoint::GetInstance()->SetCallBack(
+      "BlobDBImpl::EvictOldestBlobFile:Evicted",
+      [&](void *) { evict_count++; });
+  SyncPoint::GetInstance()->EnableProcessing();
+
+  std::string value(2000, 'v');
+  ASSERT_OK(Put("foo", std::string(2000, 'v')));
+  ASSERT_OK(Put("bar", std::string(2000, 'v')));
+  ASSERT_EQ(0, evict_count);
 }
 
 TEST_F(BlobDBTest, InlineSmallValues) {