Make sure (Shared)BlobFileMetaData are owned by shared_ptrs (#6749)

Summary:
The patch makes a couple of small cleanups to `SharedBlobFileMetaData` and `BlobFileMetaData`:
* It makes the constructors private and introduces factory methods to ensure these objects are always owned by `shared_ptr`s. Note that `SharedBlobFileMetaData` has an additional factory that takes a deleter object; we can utilize this to e.g. notify `VersionSet` when a blob file becomes obsolete (which is exactly when `SharedBlobFileMetaData` is destroyed).
* It disables move operations explicitly instead of relying on them being suppressed because of a user-declared destructor.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6749

Test Plan: `make check`

Reviewed By: riversand963

Differential Revision: D21206947

Pulled By: ltamasi

fbshipit-source-id: 9094c14cc335b3e226f883e5a0df4f87a5cdeb95

db/blob/blob_file_meta.cc

@@ -10,10 +10,6 @@
 
 namespace ROCKSDB_NAMESPACE {
 
-SharedBlobFileMetaData::~SharedBlobFileMetaData() {
-  // TODO: add the blob file to the list of obsolete files here
-}
-
 std::string SharedBlobFileMetaData::DebugString() const {
   std::ostringstream oss;
   oss << (*this);

db/blob/blob_file_meta.h

@@ -23,22 +23,33 @@ namespace ROCKSDB_NAMESPACE {
 
 class SharedBlobFileMetaData {
  public:
-  SharedBlobFileMetaData(uint64_t blob_file_number, uint64_t total_blob_count,
+  static std::shared_ptr<SharedBlobFileMetaData> Create(
+      uint64_t blob_file_number, uint64_t total_blob_count,
       uint64_t total_blob_bytes, std::string checksum_method,
-                         std::string checksum_value)
-      : blob_file_number_(blob_file_number),
-        total_blob_count_(total_blob_count),
-        total_blob_bytes_(total_blob_bytes),
-        checksum_method_(std::move(checksum_method)),
-        checksum_value_(std::move(checksum_value)) {
-    assert(checksum_method_.empty() == checksum_value_.empty());
+      std::string checksum_value) {
+    return std::shared_ptr<SharedBlobFileMetaData>(new SharedBlobFileMetaData(
+        blob_file_number, total_blob_count, total_blob_bytes,
+        std::move(checksum_method), std::move(checksum_value)));
   }
 
-  ~SharedBlobFileMetaData();
+  template <typename Deleter>
+  static std::shared_ptr<SharedBlobFileMetaData> Create(
+      uint64_t blob_file_number, uint64_t total_blob_count,
+      uint64_t total_blob_bytes, std::string checksum_method,
+      std::string checksum_value, Deleter deleter) {
+    return std::shared_ptr<SharedBlobFileMetaData>(
+        new SharedBlobFileMetaData(blob_file_number, total_blob_count,
+                                   total_blob_bytes, std::move(checksum_method),
+                                   std::move(checksum_value)),
+        deleter);
+  }
 
   SharedBlobFileMetaData(const SharedBlobFileMetaData&) = delete;
   SharedBlobFileMetaData& operator=(const SharedBlobFileMetaData&) = delete;
 
+  SharedBlobFileMetaData(SharedBlobFileMetaData&&) = delete;
+  SharedBlobFileMetaData& operator=(SharedBlobFileMetaData&&) = delete;
+
   uint64_t GetBlobFileNumber() const { return blob_file_number_; }
   uint64_t GetTotalBlobCount() const { return total_blob_count_; }
   uint64_t GetTotalBlobBytes() const { return total_blob_bytes_; }
@@ -48,6 +59,17 @@ class SharedBlobFileMetaData {
   std::string DebugString() const;
 
  private:
+  SharedBlobFileMetaData(uint64_t blob_file_number, uint64_t total_blob_count,
+                         uint64_t total_blob_bytes, std::string checksum_method,
+                         std::string checksum_value)
+      : blob_file_number_(blob_file_number),
+        total_blob_count_(total_blob_count),
+        total_blob_bytes_(total_blob_bytes),
+        checksum_method_(std::move(checksum_method)),
+        checksum_value_(std::move(checksum_value)) {
+    assert(checksum_method_.empty() == checksum_value_.empty());
+  }
+
   uint64_t blob_file_number_;
   uint64_t total_blob_count_;
   uint64_t total_blob_bytes_;
@@ -68,21 +90,19 @@ std::ostream& operator<<(std::ostream& os,
 
 class BlobFileMetaData {
  public:
-  BlobFileMetaData(std::shared_ptr<SharedBlobFileMetaData> shared_meta,
-                   uint64_t garbage_blob_count, uint64_t garbage_blob_bytes)
-      : shared_meta_(std::move(shared_meta)),
-        garbage_blob_count_(garbage_blob_count),
-        garbage_blob_bytes_(garbage_blob_bytes) {
-    assert(shared_meta_);
-    assert(garbage_blob_count_ <= shared_meta_->GetTotalBlobCount());
-    assert(garbage_blob_bytes_ <= shared_meta_->GetTotalBlobBytes());
+  static std::shared_ptr<BlobFileMetaData> Create(
+      std::shared_ptr<SharedBlobFileMetaData> shared_meta,
+      uint64_t garbage_blob_count, uint64_t garbage_blob_bytes) {
+    return std::shared_ptr<BlobFileMetaData>(new BlobFileMetaData(
+        std::move(shared_meta), garbage_blob_count, garbage_blob_bytes));
   }
 
-  ~BlobFileMetaData() = default;
-
   BlobFileMetaData(const BlobFileMetaData&) = delete;
   BlobFileMetaData& operator=(const BlobFileMetaData&) = delete;
 
+  BlobFileMetaData(BlobFileMetaData&&) = delete;
+  BlobFileMetaData& operator=(BlobFileMetaData&&) = delete;
+
   const std::shared_ptr<SharedBlobFileMetaData>& GetSharedMeta() const {
     return shared_meta_;
   }
@@ -114,6 +134,16 @@ class BlobFileMetaData {
   std::string DebugString() const;
 
  private:
+  BlobFileMetaData(std::shared_ptr<SharedBlobFileMetaData> shared_meta,
+                   uint64_t garbage_blob_count, uint64_t garbage_blob_bytes)
+      : shared_meta_(std::move(shared_meta)),
+        garbage_blob_count_(garbage_blob_count),
+        garbage_blob_bytes_(garbage_blob_bytes) {
+    assert(shared_meta_);
+    assert(garbage_blob_count_ <= shared_meta_->GetTotalBlobCount());
+    assert(garbage_blob_bytes_ <= shared_meta_->GetTotalBlobBytes());
+  }
+
   std::shared_ptr<SharedBlobFileMetaData> shared_meta_;
   uint64_t garbage_blob_count_;
   uint64_t garbage_blob_bytes_;

db/version_builder.cc

@@ -388,7 +388,7 @@ class VersionBuilder::Rep {
       return Status::Corruption("VersionBuilder", oss.str());
     }
 
-    auto shared_meta = std::make_shared<SharedBlobFileMetaData>(
+    auto shared_meta = SharedBlobFileMetaData::Create(
         blob_file_number, blob_file_addition.GetTotalBlobCount(),
         blob_file_addition.GetTotalBlobBytes(),
         blob_file_addition.GetChecksumMethod(),
@@ -396,7 +396,7 @@ class VersionBuilder::Rep {
 
     constexpr uint64_t garbage_blob_count = 0;
     constexpr uint64_t garbage_blob_bytes = 0;
-    auto new_meta = std::make_shared<BlobFileMetaData>(
+    auto new_meta = BlobFileMetaData::Create(
         std::move(shared_meta), garbage_blob_count, garbage_blob_bytes);
 
     changed_blob_files_.emplace(blob_file_number, std::move(new_meta));
@@ -417,7 +417,7 @@ class VersionBuilder::Rep {
 
     assert(meta->GetBlobFileNumber() == blob_file_number);
 
-    auto new_meta = std::make_shared<BlobFileMetaData>(
+    auto new_meta = BlobFileMetaData::Create(
         meta->GetSharedMeta(),
         meta->GetGarbageBlobCount() + blob_file_garbage.GetGarbageBlobCount(),
         meta->GetGarbageBlobBytes() + blob_file_garbage.GetGarbageBlobBytes());

db/version_builder_test.cc

@@ -82,10 +82,10 @@ class VersionBuilderTest : public testing::Test {
                uint64_t total_blob_bytes, std::string checksum_method,
                std::string checksum_value, uint64_t garbage_blob_count,
                uint64_t garbage_blob_bytes) {
-    auto shared_meta = std::make_shared<SharedBlobFileMetaData>(
+    auto shared_meta = SharedBlobFileMetaData::Create(
         blob_file_number, total_blob_count, total_blob_bytes,
         std::move(checksum_method), std::move(checksum_value));
-    auto meta = std::make_shared<BlobFileMetaData>(
+    auto meta = BlobFileMetaData::Create(
         std::move(shared_meta), garbage_blob_count, garbage_blob_bytes);
 
     vstorage_.AddBlobFile(std::move(meta));