Fix db_test and DBIter

Summary: Fix old issue with DBTest.Randomized with BlockBasedTableWithWholeKeyHashIndex + added printing in DBTest.Randomized.

Test Plan: make all check

Reviewers: zagfox, igor, ljin, yhchiang, sdong

Reviewed By: sdong

Subscribers: leveldb

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

diff --git a/db/db_iter.cc b/db/db_iter.cc
index 7681a3df8a449022994c3b1e0bd0bf8a5f0f9692..1381f24cf6dada8b4a7ef1a7502411159f68a803 100644 (file)
--- a/db/db_iter.cc
+++ b/db/db_iter.cc
@@ -11,6 +11,7 @@
 #include <stdexcept>
 #include <deque>
 #include <string>
+#include <limits>
 
 #include "db/filename.h"
 #include "db/dbformat.h"
@@ -71,6 +72,7 @@ class DBIter: public Iterator {
         current_entry_is_merged_(false),
         statistics_(options.statistics.get()) {
     RecordTick(statistics_, NO_ITERATORS);
+    has_prefix_extractor_ = (options.prefix_extractor.get() != nullptr);
     max_skip_ = options.max_sequential_skip_in_iterations;
   }
   virtual ~DBIter() {
@@ -130,6 +132,7 @@ class DBIter: public Iterator {
     }
   }
 
+  bool has_prefix_extractor_;
   bool arena_mode_;
   Env* const env_;
   Logger* logger_;
@@ -565,6 +568,11 @@ void DBIter::Seek(const Slice& target) {
 }
 
 void DBIter::SeekToFirst() {
+  // Don't use iter_::Seek() if we set a prefix extractor
+  // because prefix seek wiil be used.
+  if (has_prefix_extractor_) {
+    max_skip_ = std::numeric_limits<uint64_t>::max();
+  }
   direction_ = kForward;
   ClearSavedValue();
   PERF_TIMER_AUTO(seek_internal_seek_time);
@@ -578,6 +586,11 @@ void DBIter::SeekToFirst() {
 }
 
 void DBIter::SeekToLast() {
+  // Don't use iter_::Seek() if we set a prefix extractor
+  // because prefix seek wiil be used.
+  if (has_prefix_extractor_) {
+    max_skip_ = std::numeric_limits<uint64_t>::max();
+  }
   direction_ = kReverse;
   ClearSavedValue();
   PERF_TIMER_AUTO(seek_internal_seek_time);

diff --git a/db/db_test.cc b/db/db_test.cc
index c56bbfe994e083daf50956588fb28be3347472d2..9fb10335ba74e60095d0a522ba0439065181cdea 100644 (file)
--- a/db/db_test.cc
+++ b/db/db_test.cc
@@ -309,31 +309,31 @@ class DBTest {
  protected:
   // Sequence of option configurations to try
   enum OptionConfig {
-    kBlockBasedTableWithWholeKeyHashIndex,
-    kDefault,
-    kBlockBasedTableWithPrefixHashIndex,
-    kPlainTableFirstBytePrefix,
-    kPlainTableAllBytesPrefix,
-    kVectorRep,
-    kHashLinkList,
-    kHashCuckoo,
-    kMergePut,
-    kFilter,
-    kUncompressed,
-    kNumLevel_3,
-    kDBLogDir,
-    kWalDir,
-    kManifestFileSize,
-    kCompactOnFlush,
-    kPerfOptions,
-    kDeletesFilterFirst,
-    kHashSkipList,
-    kUniversalCompaction,
-    kCompressedBlockCache,
-    kInfiniteMaxOpenFiles,
-    kxxHashChecksum,
-    kFIFOCompaction,
-    kEnd
+    kDefault = 0,
+    kBlockBasedTableWithPrefixHashIndex = 1,
+    kBlockBasedTableWithWholeKeyHashIndex = 2,
+    kPlainTableFirstBytePrefix = 3,
+    kPlainTableAllBytesPrefix = 4,
+    kVectorRep = 5,
+    kHashLinkList = 6,
+    kHashCuckoo = 7,
+    kMergePut = 8,
+    kFilter = 9,
+    kUncompressed = 10,
+    kNumLevel_3 = 11,
+    kDBLogDir = 12,
+    kWalDir = 13,
+    kManifestFileSize = 14,
+    kCompactOnFlush = 15,
+    kPerfOptions = 16,
+    kDeletesFilterFirst = 17,
+    kHashSkipList = 18,
+    kUniversalCompaction = 19,
+    kCompressedBlockCache = 20,
+    kInfiniteMaxOpenFiles = 21,
+    kxxHashChecksum = 22,
+    kFIFOCompaction = 23,
+    kEnd = 24
   };
   int option_config_;
 
@@ -405,7 +405,7 @@ class DBTest {
               || option_config_ == kPlainTableFirstBytePrefix)) {
         continue;
       }
-      if ((skip_mask & kSkipPlainTable) &&
+      if ((skip_mask & kSkipHashIndex) &&
           (option_config_ == kBlockBasedTableWithPrefixHashIndex ||
            option_config_ == kBlockBasedTableWithWholeKeyHashIndex)) {
         continue;
@@ -4809,7 +4809,7 @@ TEST(DBTest, ApproximateSizes) {
     }
     // ApproximateOffsetOf() is not yet implemented in plain table format.
   } while (ChangeOptions(kSkipUniversalCompaction | kSkipFIFOCompaction |
-                         kSkipPlainTable));
+                         kSkipPlainTable | kSkipHashIndex));
 }
 
 TEST(DBTest, ApproximateSizes_MixOfSmallAndLarge) {
@@ -6759,7 +6759,14 @@ class ModelDB: public DB {
       iter_ = map_->lower_bound(k.ToString());
     }
     virtual void Next() { ++iter_; }
-    virtual void Prev() { --iter_; }
+    virtual void Prev() {
+      if (iter_ == map_->begin()) {
+        iter_ = map_->end();
+        return;
+      }
+      --iter_;
+    }
+
     virtual Slice key() const { return iter_->first; }
     virtual Slice value() const { return iter_->second; }
     virtual Status status() const { return Status::OK(); }
@@ -6887,8 +6894,14 @@ TEST(DBTest, Randomized) {
       }
 
       if ((step % 100) == 0) {
-        ASSERT_TRUE(CompareIterators(step, &model, db_, nullptr, nullptr));
-        ASSERT_TRUE(CompareIterators(step, &model, db_, model_snap, db_snap));
+        // For DB instances that use the hash index + block-based table, the
+        // iterator will be invalid right when seeking a non-existent key, right
+        // than return a key that is close to it.
+        if (option_config_ != kBlockBasedTableWithWholeKeyHashIndex &&
+            option_config_ != kBlockBasedTableWithPrefixHashIndex) {
+          ASSERT_TRUE(CompareIterators(step, &model, db_, nullptr, nullptr));
+          ASSERT_TRUE(CompareIterators(step, &model, db_, model_snap, db_snap));
+        }
 
         // Save a snapshot from each DB this time that we'll use next
         // time we compare things, to make sure the current state is
@@ -6902,12 +6915,18 @@ TEST(DBTest, Randomized) {
         model_snap = model.GetSnapshot();
         db_snap = db_->GetSnapshot();
       }
+
+      if ((step % 2000) == 0) {
+        fprintf(stdout,
+                "DBTest.Randomized, option ID: %d, step: %d out of %d\n",
+                option_config_, step, N);
+      }
     }
     if (model_snap != nullptr) model.ReleaseSnapshot(model_snap);
     if (db_snap != nullptr) db_->ReleaseSnapshot(db_snap);
     // skip cuckoo hash as it does not support snapshot.
-  } while (ChangeOptions(kSkipDeletesFilterFirst |
-                         kSkipNoSeekToLast | kSkipHashCuckoo));
+  } while (ChangeOptions(kSkipDeletesFilterFirst | kSkipNoSeekToLast |
+                         kSkipHashCuckoo));
 }
 
 TEST(DBTest, MultiGetSimple) {