kill ReadOptions.prefix and .prefix_seek

Summary:
also add an override option total_order_iteration if you want to use full
iterator with prefix_extractor

Test Plan: make all check

Reviewers: igor, haobo, sdong, yhchiang

Reviewed By: haobo

CC: leveldb, dhruba

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

HISTORY.md

@@ -9,6 +9,7 @@
 * Column family support
 
 ### Public API changes
+* Deprecated ReadOptions.prefix and ReadOptions.prefix_seek. Seek() defaults to prefix-based seek when Options.prefix_extractor is supplied. More detail is documented in https://github.com/facebook/rocksdb/wiki/Prefix-Seek-API-Changes
 
 ## 2.8.0 (04/04/2014)
 

db/c.cc

@@ -1230,23 +1230,12 @@ void rocksdb_readoptions_set_fill_cache(
   opt->rep.fill_cache = v;
 }
 
-void rocksdb_readoptions_set_prefix_seek(
-    rocksdb_readoptions_t* opt, unsigned char v) {
-  opt->rep.prefix_seek = v;
-}
-
 void rocksdb_readoptions_set_snapshot(
     rocksdb_readoptions_t* opt,
     const rocksdb_snapshot_t* snap) {
   opt->rep.snapshot = (snap ? snap->rep : nullptr);
 }
 
-void rocksdb_readoptions_set_prefix(
-    rocksdb_readoptions_t* opt, const char* key, size_t keylen) {
-  Slice prefix = Slice(key, keylen);
-  opt->rep.prefix = &prefix;
-}
-
 void rocksdb_readoptions_set_read_tier(
     rocksdb_readoptions_t* opt, int v) {
   opt->rep.read_tier = static_cast<rocksdb::ReadTier>(v);

db/c_test.c

@@ -461,8 +461,6 @@ int main(int argc, char** argv) {
     rocksdb_put(db, woptions, "bar3", 4, "bar", 3, &err);
     CheckNoError(err);
 
-    rocksdb_readoptions_set_prefix_seek(roptions, 1);
-
     rocksdb_iterator_t* iter = rocksdb_create_iterator(db, roptions);
     CheckCondition(!rocksdb_iter_valid(iter));
 

db/db_bench.cc

@@ -1944,7 +1944,6 @@ class Benchmark {
   void IteratorCreation(ThreadState* thread) {
     Duration duration(FLAGS_duration, reads_);
     ReadOptions options(FLAGS_verify_checksum, true);
-    options.prefix_seek = (FLAGS_prefix_size > 0);
     while (!duration.Done(1)) {
       DB* db = SelectDB(thread);
       Iterator* iter = db->NewIterator(options);
@@ -1966,7 +1965,6 @@ class Benchmark {
     int64_t found = 0;
     ReadOptions options(FLAGS_verify_checksum, true);
     options.tailing = FLAGS_use_tailing_iterator;
-    options.prefix_seek = (FLAGS_prefix_size > 0);
 
     Iterator* single_iter = nullptr;
     std::vector<Iterator*> multi_iters;

db/db_impl.cc

@@ -33,7 +33,6 @@
 #include "db/memtable_list.h"
 #include "db/merge_context.h"
 #include "db/merge_helper.h"
-#include "db/prefix_filter_iterator.h"
 #include "db/table_cache.h"
 #include "db/table_properties_collector.h"
 #include "db/tailing_iter.h"
@@ -1339,7 +1338,7 @@ Status DBImpl::WriteLevel0TableForRecovery(ColumnFamilyData* cfd, MemTable* mem,
   FileMetaData meta;
   meta.number = versions_->NewFileNumber();
   pending_outputs_.insert(meta.number);
-  Iterator* iter = mem->NewIterator();
+  Iterator* iter = mem->NewIterator(ReadOptions(), true);
   const SequenceNumber newest_snapshot = snapshots_.GetNewest();
   const SequenceNumber earliest_seqno_in_memtable =
     mem->GetFirstSequenceNumber();
@@ -1405,7 +1404,7 @@ Status DBImpl::WriteLevel0Table(ColumnFamilyData* cfd,
     for (MemTable* m : mems) {
       Log(options_.info_log, "[%s] Flushing memtable with next log file: %lu\n",
           cfd->GetName().c_str(), (unsigned long)m->GetNextLogNumber());
-      memtables.push_back(m->NewIterator());
+      memtables.push_back(m->NewIterator(ReadOptions(), true));
     }
     Iterator* iter = NewMergingIterator(&cfd->internal_comparator(),
                                         &memtables[0], memtables.size());
@@ -3494,25 +3493,13 @@ Iterator* DBImpl::NewIterator(const ReadOptions& options,
                          cfd->user_comparator(), iter, snapshot);
   }
 
-  if (options.prefix) {
-    // use extra wrapper to exclude any keys from the results which
-    // don't begin with the prefix
-    iter = new PrefixFilterIterator(iter, *options.prefix,
-                                    cfd->options()->prefix_extractor.get());
-  }
   return iter;
 }
 
 Status DBImpl::NewIterators(
     const ReadOptions& options,
     const std::vector<ColumnFamilyHandle*>& column_families,
     std::vector<Iterator*>* iterators) {
-
-  if (options.prefix) {
-    return Status::NotSupported(
-        "NewIterators doesn't support ReadOptions::prefix");
-  }
-
   iterators->clear();
   iterators->reserve(column_families.size());
   SequenceNumber latest_snapshot = 0;

db/db_impl_debug.cc

@@ -33,7 +33,6 @@ Iterator* DBImpl::TEST_NewInternalIterator(ColumnFamilyHandle* column_family) {
   SuperVersion* super_version = cfd->GetSuperVersion()->Ref();
   mutex_.Unlock();
   ReadOptions roptions;
-  roptions.prefix_seek = true;
   return NewInternalIterator(roptions, cfd, super_version);
 }
 

db/db_test.cc

@@ -306,7 +306,8 @@ class DBTest {
     kSkipUniversalCompaction = 2,
     kSkipMergePut = 4,
     kSkipPlainTable = 8,
-    kSkipHashIndex = 16
+    kSkipHashIndex = 16,
+    kSkipNoSeekToLast = 32
   };
 
   DBTest() : option_config_(kDefault),
@@ -341,6 +342,11 @@ class DBTest {
       if ((skip_mask & kSkipMergePut) && option_config_ == kMergePut) {
         continue;
       }
+      if ((skip_mask & kSkipNoSeekToLast) &&
+          (option_config_ == kHashLinkList ||
+           option_config_ == kHashSkipList)) {;
+        continue;
+      }
       if ((skip_mask & kSkipPlainTable)
           && (option_config_ == kPlainTableAllBytesPrefix
               || option_config_ == kPlainTableFirstBytePrefix)) {
@@ -862,10 +868,11 @@ class DBTest {
 
   void VerifyIterLast(std::string expected_key, int cf = 0) {
     Iterator* iter;
+    ReadOptions ro;
     if (cf == 0) {
-      iter = db_->NewIterator(ReadOptions());
+      iter = db_->NewIterator(ro);
     } else {
-      iter = db_->NewIterator(ReadOptions(), handles_[cf]);
+      iter = db_->NewIterator(ro, handles_[cf]);
     }
     iter->SeekToLast();
     ASSERT_EQ(IterStatus(iter), expected_key);
@@ -1463,7 +1470,7 @@ TEST(DBTest, NonBlockingIteration) {
 
     // This test verifies block cache behaviors, which is not used by plain
     // table format.
-  } while (ChangeOptions(kSkipPlainTable));
+  } while (ChangeOptions(kSkipPlainTable | kSkipNoSeekToLast));
 }
 
 // A delete is skipped for key if KeyMayExist(key) returns False
@@ -1907,19 +1914,23 @@ TEST(DBTest, IterSmallAndLargeMix) {
 TEST(DBTest, IterMultiWithDelete) {
   do {
     CreateAndReopenWithCF({"pikachu"});
-    ASSERT_OK(Put(1, "a", "va"));
-    ASSERT_OK(Put(1, "b", "vb"));
-    ASSERT_OK(Put(1, "c", "vc"));
-    ASSERT_OK(Delete(1, "b"));
-    ASSERT_EQ("NOT_FOUND", Get(1, "b"));
+    ASSERT_OK(Put(1, "ka", "va"));
+    ASSERT_OK(Put(1, "kb", "vb"));
+    ASSERT_OK(Put(1, "kc", "vc"));
+    ASSERT_OK(Delete(1, "kb"));
+    ASSERT_EQ("NOT_FOUND", Get(1, "kb"));
 
     Iterator* iter = db_->NewIterator(ReadOptions(), handles_[1]);
-    iter->Seek("c");
-    ASSERT_EQ(IterStatus(iter), "c->vc");
+    iter->Seek("kc");
+    ASSERT_EQ(IterStatus(iter), "kc->vc");
     if (!CurrentOptions().merge_operator) {
       // TODO: merge operator does not support backward iteration yet
-      iter->Prev();
-      ASSERT_EQ(IterStatus(iter), "a->va");
+      if (kPlainTableAllBytesPrefix != option_config_&&
+          kBlockBasedTableWithWholeKeyHashIndex != option_config_ &&
+          kHashLinkList != option_config_) {
+        iter->Prev();
+        ASSERT_EQ(IterStatus(iter), "ka->va");
+      }
     }
     delete iter;
   } while (ChangeOptions());
@@ -1952,7 +1963,7 @@ TEST(DBTest, IterPrevMaxSkip) {
 
     ASSERT_OK(Delete(1, "key1"));
     VerifyIterLast("(invalid)", 1);
-  } while (ChangeOptions(kSkipMergePut));
+  } while (ChangeOptions(kSkipMergePut | kSkipNoSeekToLast));
 }
 
 TEST(DBTest, IterWithSnapshot) {
@@ -1977,15 +1988,19 @@ TEST(DBTest, IterWithSnapshot) {
     ASSERT_EQ(IterStatus(iter), "key5->val5");
     if (!CurrentOptions().merge_operator) {
       // TODO: merge operator does not support backward iteration yet
-      iter->Prev();
-      ASSERT_EQ(IterStatus(iter), "key4->val4");
-      iter->Prev();
-      ASSERT_EQ(IterStatus(iter), "key3->val3");
+      if (kPlainTableAllBytesPrefix != option_config_&&
+        kBlockBasedTableWithWholeKeyHashIndex != option_config_ &&
+        kHashLinkList != option_config_) {
+        iter->Prev();
+        ASSERT_EQ(IterStatus(iter), "key4->val4");
+        iter->Prev();
+        ASSERT_EQ(IterStatus(iter), "key3->val3");
 
-      iter->Next();
-      ASSERT_EQ(IterStatus(iter), "key4->val4");
-      iter->Next();
-      ASSERT_EQ(IterStatus(iter), "key5->val5");
+        iter->Next();
+        ASSERT_EQ(IterStatus(iter), "key4->val4");
+        iter->Next();
+        ASSERT_EQ(IterStatus(iter), "key5->val5");
+      }
       iter->Next();
       ASSERT_TRUE(!iter->Valid());
     }
@@ -5944,7 +5959,7 @@ TEST(DBTest, GroupCommitTest) {
     ASSERT_TRUE(!itr->Valid());
     delete itr;
 
-  } while (ChangeOptions());
+  } while (ChangeOptions(kSkipNoSeekToLast));
 }
 
 namespace {
@@ -6313,7 +6328,7 @@ TEST(DBTest, Randomized) {
     }
     if (model_snap != nullptr) model.ReleaseSnapshot(model_snap);
     if (db_snap != nullptr) db_->ReleaseSnapshot(db_snap);
-  } while (ChangeOptions(kSkipDeletesFilterFirst));
+  } while (ChangeOptions(kSkipDeletesFilterFirst | kSkipNoSeekToLast));
 }
 
 TEST(DBTest, MultiGetSimple) {
@@ -6429,7 +6444,6 @@ void PrefixScanInit(DBTest *dbtest) {
 }  // namespace
 
 TEST(DBTest, PrefixScan) {
-  ReadOptions ro = ReadOptions();
   int count;
   Slice prefix;
   Slice key;
@@ -6450,45 +6464,9 @@ TEST(DBTest, PrefixScan) {
   options.max_background_compactions = 2;
   options.create_if_missing = true;
   options.disable_seek_compaction = true;
-  // Tricky: options.prefix_extractor will be released by
-  // NewHashSkipListRepFactory after use.
   options.memtable_factory.reset(NewHashSkipListRepFactory());
 
-  // prefix specified, with blooms: 2 RAND I/Os
-  // SeekToFirst
-  DestroyAndReopen(&options);
-  PrefixScanInit(this);
-  count = 0;
-  env_->random_read_counter_.Reset();
-  ro.prefix = &prefix;
-  iter = db_->NewIterator(ro);
-  for (iter->SeekToFirst(); iter->Valid(); iter->Next()) {
-    assert(iter->key().starts_with(prefix));
-    count++;
-  }
-  ASSERT_OK(iter->status());
-  delete iter;
-  ASSERT_EQ(count, 2);
-  ASSERT_EQ(env_->random_read_counter_.Read(), 2);
-
-  // prefix specified, with blooms: 2 RAND I/Os
-  // Seek
-  DestroyAndReopen(&options);
-  PrefixScanInit(this);
-  count = 0;
-  env_->random_read_counter_.Reset();
-  ro.prefix = &prefix;
-  iter = db_->NewIterator(ro);
-  for (iter->Seek(key); iter->Valid(); iter->Next()) {
-    assert(iter->key().starts_with(prefix));
-    count++;
-  }
-  ASSERT_OK(iter->status());
-  delete iter;
-  ASSERT_EQ(count, 2);
-  ASSERT_EQ(env_->random_read_counter_.Read(), 2);
-
-  // no prefix specified: 11 RAND I/Os
+  // 11 RAND I/Os
   DestroyAndReopen(&options);
   PrefixScanInit(this);
   count = 0;
@@ -6652,7 +6630,6 @@ TEST(DBTest, TailingIteratorDeletes) {
 TEST(DBTest, TailingIteratorPrefixSeek) {
   ReadOptions read_options;
   read_options.tailing = true;
-  read_options.prefix_seek = true;
 
   Options options = CurrentOptions();
   options.env = env_;

db/memtable.cc

@@ -159,13 +159,12 @@ const char* EncodeKey(std::string* scratch, const Slice& target) {
 
 class MemTableIterator: public Iterator {
  public:
-  MemTableIterator(const MemTable& mem, const ReadOptions& options)
+  MemTableIterator(const MemTable& mem, const ReadOptions& options,
+                   bool enforce_total_order)
       : bloom_(nullptr),
         prefix_extractor_(mem.prefix_extractor_),
         valid_(false) {
-    if (options.prefix) {
-      iter_.reset(mem.table_->GetPrefixIterator(*options.prefix));
-    } else if (options.prefix_seek) {
+    if (prefix_extractor_ != nullptr && !enforce_total_order) {
       bloom_ = mem.prefix_bloom_.get();
       iter_.reset(mem.table_->GetDynamicPrefixIterator());
     } else {
@@ -224,8 +223,9 @@ class MemTableIterator: public Iterator {
   void operator=(const MemTableIterator&);
 };
 
-Iterator* MemTable::NewIterator(const ReadOptions& options) {
-  return new MemTableIterator(*this, options);
+Iterator* MemTable::NewIterator(const ReadOptions& options,
+    bool enforce_total_order) {
+  return new MemTableIterator(*this, options, enforce_total_order);
 }
 
 port::RWMutex* MemTable::GetLock(const Slice& key) {

db/memtable.h

@@ -75,14 +75,10 @@ class MemTable {
   // iterator are internal keys encoded by AppendInternalKey in the
   // db/dbformat.{h,cc} module.
   //
-  // If options.prefix is supplied, it is passed to the underlying MemTableRep
-  // as a hint that the iterator only need to support access to keys with that
-  // specific prefix.
-  // If options.prefix is not supplied and options.prefix_seek is set, the
-  // iterator is not bound to a specific prefix. However, the semantics of
-  // Seek is changed - the result might only include keys with the same prefix
-  // as the seek-key.
-  Iterator* NewIterator(const ReadOptions& options = ReadOptions());
+  // By default, it returns an iterator for prefix seek if prefix_extractor
+  // is configured in Options.
+  Iterator* NewIterator(const ReadOptions& options,
+                        bool enforce_total_order = false);
 
   // Add an entry into memtable that maps key to value at the
   // specified sequence number and with the specified type.