[RocksDB] Make block based table hash index more adaptivev3.2rocksdb-3.2

Summary: Currently, RocksDB returns error if a db written with prefix hash index, is later opened without providing a prefix extractor. This is uncessarily harsh. Without a prefix extractor, we could always fallback to the normal binary index.

Test Plan: unit test, also manually veried LOG that fallback did occur.

Reviewers: sdong, ljin

Reviewed By: ljin

Subscribers: leveldb

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

2 files changed, 41 insertions, 7 deletions

diff --git a/db/db_test.cc b/db/db_test.cc
index e3f0a47cc..c998cca87 100644
--- a/db/db_test.cc
+++ b/db/db_test.cc
@@ -6822,6 +6822,31 @@ TEST(DBTest, TailingIteratorPrefixSeek) {
   ASSERT_TRUE(!iter->Valid());
 }
 
+TEST(DBTest, BlockBasedTablePrefixIndexTest) {
+  // create a DB with block prefix index
+  BlockBasedTableOptions table_options;
+  Options options = CurrentOptions();
+  table_options.index_type = BlockBasedTableOptions::kHashSearch;
+  options.table_factory.reset(NewBlockBasedTableFactory(table_options));
+  options.prefix_extractor.reset(NewFixedPrefixTransform(1));
+
+
+  Reopen(&options);
+  ASSERT_OK(Put("k1", "v1"));
+  Flush();
+  ASSERT_OK(Put("k2", "v2"));
+
+  // Reopen it without prefix extractor, make sure everything still works.
+  // RocksDB should just fall back to the binary index.
+  table_options.index_type = BlockBasedTableOptions::kBinarySearch;
+  options.table_factory.reset(NewBlockBasedTableFactory(table_options));
+  options.prefix_extractor.reset();
+
+  Reopen(&options);
+  ASSERT_EQ("v1", Get("k1"));
+  ASSERT_EQ("v2", Get("k2"));
+}
+
 TEST(DBTest, ChecksumTest) {
   BlockBasedTableOptions table_options;
   Options options = CurrentOptions();
diff --git a/table/block_based_table_reader.cc b/table/block_based_table_reader.cc
index 2420ad03f..25de3ca63 100644
--- a/table/block_based_table_reader.cc
+++ b/table/block_based_table_reader.cc
@@ -1132,6 +1132,15 @@ Status BlockBasedTable::CreateIndexReader(IndexReader** index_reader,
   auto comparator = &rep_->internal_comparator;
   const Footer& footer = rep_->footer;
 
+  if (index_type_on_file == BlockBasedTableOptions::kHashSearch &&
+      rep_->options.prefix_extractor == nullptr) {
+    Log(rep_->options.info_log,
+        "BlockBasedTableOptions::kHashSearch requires "
+        "options.prefix_extractor to be set."
+        " Fall back to binary seach index.");
+    index_type_on_file = BlockBasedTableOptions::kBinarySearch;
+  }
+
   switch (index_type_on_file) {
     case BlockBasedTableOptions::kBinarySearch: {
       return BinarySearchIndexReader::Create(
@@ -1144,19 +1153,19 @@ Status BlockBasedTable::CreateIndexReader(IndexReader** index_reader,
       if (meta_index_iter == nullptr) {
         auto s = ReadMetaBlock(rep_, &meta_guard, &meta_iter_guard);
         if (!s.ok()) {
-          return Status::Corruption("Unable to read the metaindex block");
+          // we simply fall back to binary search in case there is any
+          // problem with prefix hash index loading.
+          Log(rep_->options.info_log,
+              "Unable to read the metaindex block."
+              " Fall back to binary seach index.");
+          return BinarySearchIndexReader::Create(
+            file, footer, footer.index_handle(), env, comparator, index_reader);
         }
         meta_index_iter = meta_iter_guard.get();
       }
 
       // We need to wrap data with internal_prefix_transform to make sure it can
       // handle prefix correctly.
-      if (rep_->options.prefix_extractor == nullptr) {
-        return Status::InvalidArgument(
-            "BlockBasedTableOptions::kHashSearch requires "
-            "options.prefix_extractor to be set.");
-      }
-
       rep_->internal_prefix_transform.reset(
           new InternalKeySliceTransform(rep_->options.prefix_extractor.get()));
       return HashIndexReader::Create(