From 59de2dbad7b57663cd456c37e3168f5278573fc9 Mon Sep 17 00:00:00 2001
From: Dhruba Borthakur <dhruba@fb.com>
Date: Tue, 27 Aug 2013 14:54:06 -0700
Subject: [PATCH] Cleanup DeleteFile API

Summary:
The DeleteFile API was removing files inside the db-lock. This
is now changed to remove files outside the db-lock.
The GetLiveFilesMetadata() returns the smallest and largest
seqnuence number of each file as well.

Test Plan: deletefile_test

Reviewers: emayanke, haobo

Reviewed By: haobo

CC: leveldb

Maniphest Tasks: T63

Differential Revision: https://reviews.facebook.net/D12567
---
 db/db_impl.cc        | 64 +++++++++++++++++++++++++-------------------
 db/version_set.cc    |  2 ++
 include/rocksdb/db.h | 17 +++++-------
 3 files changed, 45 insertions(+), 38 deletions(-)

diff --git a/db/db_impl.cc b/db/db_impl.cc
index 5034c20bf..d806b2749 100644
--- a/db/db_impl.cc
+++ b/db/db_impl.cc
@@ -2996,40 +2996,48 @@ Status DBImpl::DeleteFile(std::string name) {
   FileMetaData metadata;
   int maxlevel = NumberLevels();
   VersionEdit edit(maxlevel);
-  MutexLock l(&mutex_);
-  Status status =
-    versions_->GetMetadataForFile(number, &level, &metadata);
-  if (!status.ok()) {
-    Log(options_.info_log, "DeleteFile #%lld FAILED. File not found\n",
-        static_cast<unsigned long long>(number));
-    return Status::InvalidArgument("File not found");
-  }
-  assert((level > 0) && (level < maxlevel));
-
-  // If the file is being compacted no need to delete.
-  if (metadata.being_compacted) {
-    Log(options_.info_log,
-        "DeleteFile #%lld Skipped. File about to be compacted\n",
-        static_cast<unsigned long long>(number));
-    return Status::OK();
-  }
+  DeletionState deletion_state;
+  Status status;
+  {
+    MutexLock l(&mutex_);
+    status = versions_->GetMetadataForFile(number, &level, &metadata);
+    if (!status.ok()) {
+      Log(options_.info_log, "DeleteFile #%lld FAILED. File not found\n",
+          static_cast<unsigned long long>(number));
+      return Status::InvalidArgument("File not found");
+    }
+    assert((level > 0) && (level < maxlevel));
 
-  // Only the files in the last level can be deleted externally.
-  // This is to make sure that any deletion tombstones are not
-  // lost. Check that the level passed is the last level.
-  for (int i = level + 1; i < maxlevel; i++) {
-    if (versions_->NumLevelFiles(i) != 0) {
+    // If the file is being compacted no need to delete.
+    if (metadata.being_compacted) {
       Log(options_.info_log,
-          "DeleteFile #%lld FAILED. File not in last level\n",
+          "DeleteFile #%lld Skipped. File about to be compacted\n",
           static_cast<unsigned long long>(number));
-      return Status::InvalidArgument("File not in last level");
+      return Status::OK();
+    }
+
+    // Only the files in the last level can be deleted externally.
+    // This is to make sure that any deletion tombstones are not
+    // lost. Check that the level passed is the last level.
+    for (int i = level + 1; i < maxlevel; i++) {
+      if (versions_->NumLevelFiles(i) != 0) {
+        Log(options_.info_log,
+            "DeleteFile #%lld FAILED. File not in last level\n",
+            static_cast<unsigned long long>(number));
+        return Status::InvalidArgument("File not in last level");
+      }
     }
-  }
+    edit.DeleteFile(level, number);
+    status = versions_->LogAndApply(&edit, &mutex_);
+    if (status.ok()) {
+      FindObsoleteFiles(deletion_state);
+    }
+  } // lock released here
 
-  edit.DeleteFile(level, number);
-  status = versions_->LogAndApply(&edit, &mutex_);
   if (status.ok()) {
-    DeleteObsoleteFiles();
+    // remove files outside the db-lock
+    PurgeObsoleteFiles(deletion_state);
+    EvictObsoleteFiles(deletion_state);
   }
   return status;
 }
diff --git a/db/version_set.cc b/db/version_set.cc
index 54be370bd..531a0b391 100644
--- a/db/version_set.cc
+++ b/db/version_set.cc
@@ -2701,6 +2701,8 @@ void VersionSet::GetLiveFilesMetaData(
       filemetadata.size = files[i]->file_size;
       filemetadata.smallestkey = files[i]->smallest.user_key().ToString();
       filemetadata.largestkey = files[i]->largest.user_key().ToString();
+      filemetadata.smallest_seqno = files[i]->smallest_seqno;
+      filemetadata.largest_seqno = files[i]->largest_seqno;
       metadata->push_back(filemetadata);
     }
   }
diff --git a/include/rocksdb/db.h b/include/rocksdb/db.h
index ca122efb1..d68772e4c 100644
--- a/include/rocksdb/db.h
+++ b/include/rocksdb/db.h
@@ -30,16 +30,13 @@ class WriteBatch;
 
 // Metadata associated with each SST file.
 struct LiveFileMetaData {
-  // Name of the file
-  std::string name;
-  // Level at which this file resides.
-  int level;
-  // File size in bytes.
-  size_t size;
-  // Smallest user defined key in the file.
-  std::string smallestkey;
-  // Largest user defined key in the file.
-  std::string largestkey;
+  std::string name;        // Name of the file
+  int level;               // Level at which this file resides.
+  size_t size;             // File size in bytes.
+  std::string smallestkey; // Smallest user defined key in the file.
+  std::string largestkey;  // Largest user defined key in the file.
+  SequenceNumber smallest_seqno; // smallest seqno in file
+  SequenceNumber largest_seqno;  // largest seqno in file
 };
 
 // Abstract handle to particular state of a DB.