Add a whitebox test for deleted file iterators.

Summary:
We have earlier added a feature to delete file iterators when the
current key is over the iterate upper bound. We now add a whitebox test
to check if the file iterators were actually deleted.

Test Plan: Add check for a range which has deleted iterators.

Reviewers: sdong

Reviewed By: sdong

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D45321

db/db_tailing_iter_test.cc

@@ -12,6 +12,7 @@
 // which is a pity, it is a good test
 #if !(defined NDEBUG) || !defined(OS_WIN)
 
+#include "db/forward_iterator.h"
 #include "port/stack_trace.h"
 #include "util/db_test_util.h"
 
@@ -96,7 +97,10 @@ TEST_F(DBTestTailingIterator, TailingIteratorSeekToNext) {
     }
     ASSERT_TRUE(itern->Valid());
     ASSERT_EQ(itern->key().compare(key), 0);
+    file_iters_deleted = false;
   }
+  rocksdb::SyncPoint::GetInstance()->ClearAllCallBacks();
+  rocksdb::SyncPoint::GetInstance()->DisableProcessing();
   for (int i = 2 * num_records; i > 0; --i) {
     char buf1[32];
     char buf2[32];
@@ -130,7 +134,18 @@ TEST_F(DBTestTailingIterator, TailingIteratorTrimSeekToNext) {
   std::unique_ptr<Iterator> itern(db_->NewIterator(read_options, handles_[1]));
   std::unique_ptr<Iterator> iterh(db_->NewIterator(read_options, handles_[1]));
   std::string value(1024, 'a');
-
+  bool file_iters_deleted = false;
+  rocksdb::SyncPoint::GetInstance()->SetCallBack(
+      "ForwardIterator::SeekInternal:Return", [&](void* arg) {
+        ForwardIterator* fiter = reinterpret_cast<ForwardIterator*>(arg);
+        ASSERT_TRUE(!file_iters_deleted || fiter->TEST_CheckDeletedIters());
+      });
+  rocksdb::SyncPoint::GetInstance()->SetCallBack(
+      "ForwardIterator::Next:Return", [&](void* arg) {
+        ForwardIterator* fiter = reinterpret_cast<ForwardIterator*>(arg);
+        ASSERT_TRUE(!file_iters_deleted || fiter->TEST_CheckDeletedIters());
+      });
+  rocksdb::SyncPoint::GetInstance()->EnableProcessing();
   const int num_records = 1000;
   for (int i = 1; i < num_records; ++i) {
     char buf1[32];
@@ -148,6 +163,9 @@ TEST_F(DBTestTailingIterator, TailingIteratorTrimSeekToNext) {
     if (i % 100 == 99) {
       ASSERT_OK(Flush(1));
       dbfull()->TEST_WaitForCompact();
+      if (i == 299) {
+        file_iters_deleted = true;
+      }
       snprintf(buf4, sizeof(buf1), "00a0%016d", i * 5 / 2);
       Slice target(buf4, 20);
       iterh->Seek(target);
@@ -156,8 +174,12 @@ TEST_F(DBTestTailingIterator, TailingIteratorTrimSeekToNext) {
         iterh->Next();
         ASSERT_TRUE(iterh->Valid());
       }
+      if (i == 299) {
+        file_iters_deleted = false;
+      }
     }
 
+    file_iters_deleted = ((i > 99) && (i % 400 != 399));
     snprintf(buf2, sizeof(buf2), "00a0%016d", i * 5 - 2);
     Slice target(buf2, 20);
     iter->Seek(target);

db/forward_iterator.cc

@@ -19,6 +19,7 @@
 #include "rocksdb/slice_transform.h"
 #include "table/merger.h"
 #include "db/dbformat.h"
+#include "util/sync_point.h"
 
 namespace rocksdb {
 
@@ -389,6 +390,7 @@ void ForwardIterator::SeekInternal(const Slice& internal_key,
   }
 
   UpdateCurrent();
+  TEST_SYNC_POINT_CALLBACK("ForwardIterator::SeekInternal:Return", this);
 }
 
 void ForwardIterator::Next() {
@@ -443,6 +445,7 @@ void ForwardIterator::Next() {
     }
   }
   UpdateCurrent();
+  TEST_SYNC_POINT_CALLBACK("ForwardIterator::Next:Return", this);
 }
 
 Slice ForwardIterator::key() const {
@@ -474,6 +477,7 @@ void ForwardIterator::RebuildIterators(bool refresh_sv) {
   }
   mutable_iter_ = sv_->mem->NewIterator(read_options_, &arena_);
   sv_->imm->AddIterators(read_options_, &imm_iters_, &arena_);
+  has_iter_trimmed_for_upper_bound_ = false;
 
   const auto* vstorage = sv_->current->storage_info();
   const auto& l0_files = vstorage->LevelFiles(0);
@@ -499,6 +503,9 @@ void ForwardIterator::RebuildIterators(bool refresh_sv) {
                                     level_files[0]->smallest.user_key()) <
           0))) {
       level_iters_.push_back(nullptr);
+      if (!level_files.empty()) {
+        has_iter_trimmed_for_upper_bound_ = true;
+      }
     } else {
       level_iters_.push_back(
           new LevelIterator(cfd_, read_options_, level_files));
@@ -507,7 +514,6 @@ void ForwardIterator::RebuildIterators(bool refresh_sv) {
 
   current_ = nullptr;
   is_prev_set_ = false;
-  has_iter_trimmed_for_upper_bound_ = false;
 }
 
 void ForwardIterator::ResetIncompleteIterators() {
@@ -657,6 +663,33 @@ void ForwardIterator::DeleteCurrentIter() {
   }
 }
 
+bool ForwardIterator::TEST_CheckDeletedIters() {
+  if (!has_iter_trimmed_for_upper_bound_) {
+    return false;
+  }
+  for (size_t i = 0; i < imm_iters_.size(); i++) {
+    auto& m = imm_iters_[i];
+    if (!m) {
+      return true;
+    }
+  }
+
+  const VersionStorageInfo* vstorage = sv_->current->storage_info();
+  const std::vector<FileMetaData*>& l0 = vstorage->LevelFiles(0);
+  for (uint32_t i = 0; i < l0.size(); ++i) {
+    if (!l0_iters_[i]) {
+      return true;
+    }
+  }
+
+  for (int32_t level = 1; level < vstorage->num_levels(); ++level) {
+    if ((level_iters_[level - 1] == nullptr) &&
+        (!vstorage->LevelFiles(level).empty())) {
+      return true;
+    }
+  }
+  return false;
+}
 uint32_t ForwardIterator::FindFileInRange(
     const std::vector<FileMetaData*>& files, const Slice& internal_key,
     uint32_t left, uint32_t right) {

db/forward_iterator.h

@@ -70,6 +70,7 @@ class ForwardIterator : public Iterator {
   virtual Slice key() const override;
   virtual Slice value() const override;
   virtual Status status() const override;
+  bool TEST_CheckDeletedIters();
 
  private:
   void Cleanup(bool release_sv);