Merge branch 'master' into performance

Conflicts:
	Makefile
	db/db_impl.cc
	db/db_test.cc
	db/memtable_list.cc
	db/memtable_list.h
	table/block_based_table_reader.cc
	table/table_test.cc
	util/cache.cc
	util/coding.cc

table/block_based_table_factory.cc

@@ -20,10 +20,10 @@ namespace rocksdb {
 
 Status BlockBasedTableFactory::GetTableReader(
     const Options& options, const EnvOptions& soptions,
-    unique_ptr<RandomAccessFile> && file, uint64_t file_size,
+    unique_ptr<RandomAccessFile>&& file, uint64_t file_size,
     unique_ptr<TableReader>* table_reader) const {
-  return BlockBasedTable::Open(options, soptions, std::move(file), file_size,
-                               table_reader);
+  return BlockBasedTable::Open(options, soptions, table_options_,
+                               std::move(file), file_size, table_reader);
 }
 
 TableBuilder* BlockBasedTableFactory::GetTableBuilder(

table/block_based_table_factory.h

@@ -14,6 +14,7 @@
 #include "rocksdb/flush_block_policy.h"
 #include "rocksdb/options.h"
 #include "rocksdb/table.h"
+#include "table/block_based_table_options.h"
 
 namespace rocksdb {
 
@@ -30,40 +31,25 @@ class BlockBasedTable;
 class BlockBasedTableBuilder;
 
 class BlockBasedTableFactory: public TableFactory {
-public:
-  struct TableOptions {
-    // @flush_block_policy_factory creates the instances of flush block policy.
-    // which provides a configurable way to determine when to flush a block in
-    // the block based tables.  If not set, table builder will use the default
-    // block flush policy, which cut blocks by block size (please refer to
-    // `FlushBlockBySizePolicy`).
-    std::shared_ptr<FlushBlockPolicyFactory> flush_block_policy_factory;
-  };
+ public:
+  BlockBasedTableFactory() : BlockBasedTableFactory(BlockBasedTableOptions()) {}
+  explicit BlockBasedTableFactory(const BlockBasedTableOptions& table_options)
+      : table_options_(table_options) {}
 
-  BlockBasedTableFactory() : BlockBasedTableFactory(TableOptions()) { }
-  BlockBasedTableFactory(const TableOptions& table_options): 
-      table_options_(table_options) { 
-  }
+  ~BlockBasedTableFactory() {}
 
-  ~BlockBasedTableFactory() {
-  }
-
-  const char* Name() const override {
-    return "BlockBasedTable";
-  }
+  const char* Name() const override { return "BlockBasedTable"; }
 
   Status GetTableReader(const Options& options, const EnvOptions& soptions,
-                        unique_ptr<RandomAccessFile> && file,
-                        uint64_t file_size,
+                        unique_ptr<RandomAccessFile>&& file, uint64_t file_size,
                         unique_ptr<TableReader>* table_reader) const override;
 
   TableBuilder* GetTableBuilder(const Options& options, WritableFile* file,
-                                CompressionType compression_type) const
-                                    override;
+                                CompressionType compression_type)
+      const override;
 
  private:
-  TableOptions table_options_;
+  BlockBasedTableOptions table_options_;
 };
 
-
 }  // namespace rocksdb

table/block_based_table_options.h

@@ -0,0 +1,31 @@
+//  Copyright (c) 2013, Facebook, Inc.  All rights reserved.
+//  This source code is licensed under the BSD-style license found in the
+//  LICENSE file in the root directory of this source tree. An additional grant
+//  of patent rights can be found in the PATENTS file in the same directory.
+
+#pragma once
+#include <memory>
+
+namespace rocksdb {
+
+class FlushBlockPolicyFactory;
+
+struct BlockBasedTableOptions {
+  // @flush_block_policy_factory creates the instances of flush block policy.
+  // which provides a configurable way to determine when to flush a block in
+  // the block based tables.  If not set, table builder will use the default
+  // block flush policy, which cut blocks by block size (please refer to
+  // `FlushBlockBySizePolicy`).
+  std::shared_ptr<FlushBlockPolicyFactory> flush_block_policy_factory;
+
+  // TODO(kailiu) Temporarily disable this feature by making the default value
+  // to be false. Also in master branch, this file is non-public so no user
+  // will be able to change the value of `cache_index_and_filter_blocks`.
+  //
+  // Indicating if we'd put index/filter blocks to the block cache.
+  // If not specified, each "table reader" object will pre-load index/filter
+  // block during table initialization.
+  bool cache_index_and_filter_blocks = false;
+};
+
+}  // namespace rocksdb

table/block_based_table_reader.cc

@@ -27,6 +27,7 @@
 #include "util/coding.h"
 #include "util/perf_context_imp.h"
 #include "util/stop_watch.h"
+#include "table/block_based_table_options.h"
 
 namespace rocksdb {
 
@@ -48,9 +49,9 @@ struct BlockBasedTable::Rep {
   Status status;
   unique_ptr<RandomAccessFile> file;
   char cache_key_prefix[kMaxCacheKeyPrefixSize];
-  size_t cache_key_prefix_size;
+  size_t cache_key_prefix_size = 0;
   char compressed_cache_key_prefix[kMaxCacheKeyPrefixSize];
-  size_t compressed_cache_key_prefix_size;
+  size_t compressed_cache_key_prefix_size = 0;
 
   // Handle to metaindex_block: saved from footer
   BlockHandle metaindex_handle;
@@ -223,15 +224,15 @@ Cache::Handle* GetFromBlockCache(
 
 } // end of anonymous namespace
 
-Status BlockBasedTable::Open(const Options& options,
-                             const EnvOptions& soptions,
-                             unique_ptr<RandomAccessFile> && file,
-                             uint64_t size,
+Status BlockBasedTable::Open(const Options& options, const EnvOptions& soptions,
+                             const BlockBasedTableOptions& table_options,
+                             unique_ptr<RandomAccessFile>&& file,
+                             uint64_t file_size,
                              unique_ptr<TableReader>* table_reader) {
   table_reader->reset();
 
   Footer footer(kBlockBasedTableMagicNumber);
-  auto s = ReadFooterFromFile(file.get(), size, &footer);
+  auto s = ReadFooterFromFile(file.get(), file_size, &footer);
   if (!s.ok()) return s;
 
   // We've successfully read the footer and the index block: we're
@@ -254,13 +255,8 @@ Status BlockBasedTable::Open(const Options& options,
   if (meta_iter->Valid() && meta_iter->key() == kPropertiesBlock) {
     s = meta_iter->status();
     if (s.ok()) {
-      s = ReadProperties(
-          meta_iter->value(),
-          rep->file.get(),
-          rep->options.env,
-          rep->options.info_log.get(),
-          &rep->table_properties
-      );
+      s = ReadProperties(meta_iter->value(), rep->file.get(), rep->options.env,
+                         rep->options.info_log.get(), &rep->table_properties);
     }
 
     if (!s.ok()) {
@@ -271,11 +267,21 @@ Status BlockBasedTable::Open(const Options& options,
     }
   }
 
-  // Initialize index/filter blocks. If block cache is not specified,
-  // these blocks will be kept in member variables in Rep, which will
-  // reside in the memory as long as this table object is alive; otherwise
-  // they will be added to block cache.
-  if (!options.block_cache) {
+  // Will use block cache for index/filter blocks access?
+  if (options.block_cache && table_options.cache_index_and_filter_blocks) {
+    // Call IndexBlockReader() to implicitly add index to the block_cache
+    unique_ptr<Iterator> iter(new_table->IndexBlockReader(ReadOptions()));
+    s = iter->status();
+
+    if (s.ok()) {
+      // Call GetFilter() to implicitly add filter to the block_cache
+      auto filter_entry = new_table->GetFilter();
+      filter_entry.Release(options.block_cache.get());
+    }
+  } else {
+    // If we don't use block cache for index/filter blocks access, we'll
+    // pre-load these blocks, which will kept in member variables in Rep
+    // and with a same life-time as this table object.
     Block* index_block = nullptr;
     // TODO: we never really verify check sum for index block
     s = ReadBlockFromFile(
@@ -303,18 +309,7 @@ Status BlockBasedTable::Open(const Options& options,
     } else {
       delete index_block;
     }
-  } else {
-    // Call IndexBlockReader() to implicitly add index to the block_cache
-    unique_ptr<Iterator> iter(
-        new_table->IndexBlockReader(ReadOptions())
-    );
-    s = iter->status();
 
-    if (s.ok()) {
-      // Call GetFilter() to implicitly add filter to the block_cache
-      auto filter_entry = new_table->GetFilter();
-      filter_entry.Release(options.block_cache.get());
-    }
   }
 
   if (s.ok()) {
@@ -740,7 +735,6 @@ BlockBasedTable::GetFilter(bool no_io) const {
 // Get the iterator from the index block.
 Iterator* BlockBasedTable::IndexBlockReader(const ReadOptions& options) const {
   if (rep_->index_block) {
-    assert (!rep_->options.block_cache);
     return rep_->index_block->NewIterator(rep_->options.comparator);
   }
 

table/block_based_table_reader.h

@@ -29,6 +29,7 @@ struct ReadOptions;
 class TableCache;
 class TableReader;
 class FilterBlockReader;
+struct BlockBasedTableOptions;
 
 using std::unique_ptr;
 
@@ -49,10 +50,9 @@ class BlockBasedTable : public TableReader {
   // to nullptr and returns a non-ok status.
   //
   // *file must remain live while this Table is in use.
-  static Status Open(const Options& options,
-                     const EnvOptions& soptions,
-                     unique_ptr<RandomAccessFile>&& file,
-                     uint64_t file_size,
+  static Status Open(const Options& db_options, const EnvOptions& env_options,
+                     const BlockBasedTableOptions& table_options,
+                     unique_ptr<RandomAccessFile>&& file, uint64_t file_size,
                      unique_ptr<TableReader>* table_reader);
 
   bool PrefixMayMatch(const Slice& internal_prefix) override;

table/merger.h

@@ -23,7 +23,8 @@ class Env;
 // key is present in K child iterators, it will be yielded K times.
 //
 // REQUIRES: n >= 0
-extern Iterator* NewMergingIterator(
-    Env* const env, const Comparator* comparator, Iterator** children, int n);
+extern Iterator* NewMergingIterator(Env* const env,
+                                    const Comparator* comparator,
+                                    Iterator** children, int n);
 
 }  // namespace rocksdb

table/meta_blocks.h

@@ -11,6 +11,7 @@
 #include "rocksdb/comparator.h"
 #include "rocksdb/options.h"
 #include "rocksdb/slice.h"
+#include "rocksdb/table_properties.h"
 #include "table/block_builder.h"
 
 namespace rocksdb {

table/plain_table_reader.h

@@ -10,6 +10,7 @@
 #include "rocksdb/env.h"
 #include "rocksdb/iterator.h"
 #include "rocksdb/table.h"
+#include "rocksdb/slice_transform.h"
 #include "rocksdb/plain_table_factory.h"
 
 namespace rocksdb {
@@ -38,7 +39,7 @@ using std::unordered_map;
 class PlainTableReader: public TableReader {
  public:
   static Status Open(const Options& options, const EnvOptions& soptions,
-                     unique_ptr<RandomAccessFile> && file, uint64_t file_size,
+                     unique_ptr<RandomAccessFile>&& file, uint64_t file_size,
                      unique_ptr<TableReader>* table, const int bloom_num_bits,
                      double hash_table_ratio);
 
@@ -196,7 +197,7 @@ class PlainTableReader: public TableReader {
                    uint32_t& ret_offset);
 
   Slice GetPrefix(const Slice& target) {
-    assert(target.size() >= 8); // target is internal key
+    assert(target.size() >= 8);  // target is internal key
     return options_.prefix_extractor->Transform(
         Slice(target.data(), target.size() - 8));
   }

table/table_test.cc

@@ -293,14 +293,11 @@ class KeyConvertingIterator: public Iterator {
 
 class TableConstructor: public Constructor {
  public:
-  explicit TableConstructor(
-      const Comparator* cmp, bool convert_to_internal_key = false)
-      : Constructor(cmp),
-        convert_to_internal_key_(convert_to_internal_key)  {
-  }
-  ~TableConstructor() {
-    Reset();
-  }
+  explicit TableConstructor(const Comparator* cmp,
+                            bool convert_to_internal_key = false)
+      : Constructor(cmp), convert_to_internal_key_(convert_to_internal_key) {}
+  ~TableConstructor() { Reset(); }
+
   virtual Status FinishImpl(const Options& options, const KVMap& data) {
     Reset();
     sink_.reset(new StringSink());
@@ -329,14 +326,11 @@ class TableConstructor: public Constructor {
 
     // Open the table
     uniq_id_ = cur_uniq_id_++;
-    source_.reset(
-        new StringSource(sink_->contents(), uniq_id_,
-                         options.allow_mmap_reads));
-    unique_ptr<TableFactory> table_factory;
-    return options.table_factory->GetTableReader(options, soptions,
-                                                 std::move(source_),
-                                                 sink_->contents().size(),
-                                                 &table_reader_);
+    source_.reset(new StringSource(sink_->contents(), uniq_id_,
+                                   options.allow_mmap_reads));
+    return options.table_factory->GetTableReader(
+        options, soptions, std::move(source_), sink_->contents().size(),
+        &table_reader_);
   }
 
   virtual Iterator* NewIterator() const {
@@ -630,7 +624,7 @@ class Harness {
     internal_comparator_.reset(new InternalKeyComparator(options_.comparator));
     support_prev_ = true;
     only_support_prefix_seek_ = false;
-    BlockBasedTableFactory::TableOptions table_options;
+    BlockBasedTableOptions table_options;
     switch (args.type) {
       case BLOCK_BASED_TABLE_TEST:
         table_options.flush_block_policy_factory.reset(
@@ -1053,6 +1047,11 @@ TEST(BlockBasedTableTest, BlockCacheTest) {
   options.create_if_missing = true;
   options.statistics = CreateDBStatistics();
   options.block_cache = NewLRUCache(1024);
+
+  // Enable the cache for index/filter blocks
+  BlockBasedTableOptions table_options;
+  table_options.cache_index_and_filter_blocks = true;
+  options.table_factory.reset(new BlockBasedTableFactory(table_options));
   std::vector<std::string> keys;
   KVMap kvmap;