Disable pre-fetching of index and filter blocks for sst_dump_tool.

Summary:
BlockBasedTable pre-fetches the filter and index blocks on Open call.
This is an optimistic optimization targeted for runtime scenario. The
optimization is unnecessary for sst_dump_tool

- Added a provision to disable pre-fetching of index and filter blocks
  in BlockBasedTable
- Disabled pre-fetching for the sst_dump tool

Stack for reference :

#1  0x00000000005ed944 in snappy::InternalUncompress<snappy::SnappyArrayWriter> () from /home/engshare/third-party2/snappy/1.0.3/src/snappy-1.0.3/snappy.cc:148
#2  0x00000000005edeee in snappy::RawUncompress () from /home/engshare/third-party2/snappy/1.0.3/src/snappy-1.0.3/snappy.cc:947
#3  0x00000000004e0b4d in rocksdb::UncompressBlockContents () from /data/users/paultuckfield/rocksdb/./util/compression.h:69
#4  0x00000000004e145c in rocksdb::ReadBlockContents () from /data/users/paultuckfield/rocksdb/table/format.cc:334
#5  0x00000000004ca424 in rocksdb::(anonymous namespace)::ReadBlockFromFile () from /data/users/paultuckfield/rocksdb/table/block_based_table_reader.cc:70
#6  0x00000000004cccad in rocksdb::BlockBasedTable::CreateIndexReader () from /data/users/paultuckfield/rocksdb/table/block_based_table_reader.cc:173
#7  0x00000000004d17e5 in rocksdb::BlockBasedTable::Open () from /data/users/paultuckfield/rocksdb/table/block_based_table_reader.cc:553
#8  0x00000000004c8184 in rocksdb::BlockBasedTableFactory::NewTableReader () from /data/users/paultuckfield/rocksdb/table/block_based_table_factory.cc:51
#9  0x0000000000598463 in rocksdb::SstFileReader::NewTableReader () from /data/users/paultuckfield/rocksdb/util/sst_dump_tool.cc:69
#10  0x00000000005986c2 in rocksdb::SstFileReader::SstFileReader () from /data/users/paultuckfield/rocksdb/util/sst_dump_tool.cc:26
#11  0x0000000000599047 in rocksdb::SSTDumpTool::Run () from /data/users/paultuckfield/rocksdb/util/sst_dump_tool.cc:332
#12  0x0000000000409b06 in main () from /data/users/paultuckfield/rocksdb/tools/sst_dump.cc:12

Test Plan:
- Added a unit test to trigger the code.
- Also did some manual verification.
- Passed all unit tests

task #6296048

Reviewers: igor, rven, sdong

Reviewed By: sdong

Subscribers: dhruba, leveldb

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

Author: krad

table/block_based_table_factory.cc

@@ -45,10 +45,10 @@ Status BlockBasedTableFactory::NewTableReader(
     const ImmutableCFOptions& ioptions, const EnvOptions& soptions,
     const InternalKeyComparator& internal_comparator,
     unique_ptr<RandomAccessFile>&& file, uint64_t file_size,
-    unique_ptr<TableReader>* table_reader) const {
+    unique_ptr<TableReader>* table_reader, const bool prefetch_enabled) const {
   return BlockBasedTable::Open(ioptions, soptions, table_options_,
                                internal_comparator, std::move(file), file_size,
-                               table_reader);
+                               table_reader, prefetch_enabled);
 }
 
 TableBuilder* BlockBasedTableFactory::NewTableBuilder(

table/block_based_table_factory.h

@@ -33,11 +33,24 @@ class BlockBasedTableFactory : public TableFactory {
 
   const char* Name() const override { return "BlockBasedTable"; }
 
-  Status NewTableReader(
-      const ImmutableCFOptions& ioptions, const EnvOptions& soptions,
-      const InternalKeyComparator& internal_comparator,
-      unique_ptr<RandomAccessFile>&& file, uint64_t file_size,
-      unique_ptr<TableReader>* table_reader) const override;
+  Status NewTableReader(const ImmutableCFOptions& ioptions,
+                        const EnvOptions& soptions,
+                        const InternalKeyComparator& internal_comparator,
+                        unique_ptr<RandomAccessFile>&& file, uint64_t file_size,
+                        unique_ptr<TableReader>* table_reader) const override {
+    return NewTableReader(ioptions, soptions, internal_comparator,
+                          std::move(file), file_size, table_reader,
+                          /*prefetch_index_and_filter=*/true);
+  }
+
+  // This is a variant of virtual member function NewTableReader function with
+  // added capability to disable pre-fetching of blocks on BlockBasedTable::Open
+  Status NewTableReader(const ImmutableCFOptions& ioptions,
+                        const EnvOptions& soptions,
+                        const InternalKeyComparator& internal_comparator,
+                        unique_ptr<RandomAccessFile>&& file, uint64_t file_size,
+                        unique_ptr<TableReader>* table_reader,
+                        bool prefetch_index_and_filter) const;
 
   TableBuilder* NewTableBuilder(
       const ImmutableCFOptions& ioptions,

table/block_based_table_reader.cc

@@ -463,7 +463,8 @@ Status BlockBasedTable::Open(const ImmutableCFOptions& ioptions,
                              const InternalKeyComparator& internal_comparator,
                              unique_ptr<RandomAccessFile>&& file,
                              uint64_t file_size,
-                             unique_ptr<TableReader>* table_reader) {
+                             unique_ptr<TableReader>* table_reader,
+                             const bool prefetch_index_and_filter) {
   table_reader->reset();
 
   Footer footer;
@@ -537,34 +538,38 @@ Status BlockBasedTable::Open(const ImmutableCFOptions& ioptions,
         BlockBasedTablePropertyNames::kPrefixFiltering, rep->ioptions.info_log);
   }
 
-  // Will use block cache for index/filter blocks access?
-  if (table_options.cache_index_and_filter_blocks) {
-    assert(table_options.block_cache != nullptr);
-    // Hack: Call NewIndexIterator() to implicitly add index to the block_cache
-    unique_ptr<Iterator> iter(new_table->NewIndexIterator(ReadOptions()));
-    s = iter->status();
+  if (prefetch_index_and_filter) {
+    // pre-fetching of blocks is turned on
+    // Will use block cache for index/filter blocks access?
+    if (table_options.cache_index_and_filter_blocks) {
+      assert(table_options.block_cache != nullptr);
+      // Hack: Call NewIndexIterator() to implicitly add index to the
+      // block_cache
+      unique_ptr<Iterator> iter(new_table->NewIndexIterator(ReadOptions()));
+      s = iter->status();
 
-    if (s.ok()) {
-      // Hack: Call GetFilter() to implicitly add filter to the block_cache
-      auto filter_entry = new_table->GetFilter();
-      filter_entry.Release(table_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.
-    IndexReader* index_reader = nullptr;
-    s = new_table->CreateIndexReader(&index_reader, meta_iter.get());
+      if (s.ok()) {
+        // Hack: Call GetFilter() to implicitly add filter to the block_cache
+        auto filter_entry = new_table->GetFilter();
+        filter_entry.Release(table_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.
+      IndexReader* index_reader = nullptr;
+      s = new_table->CreateIndexReader(&index_reader, meta_iter.get());
 
-    if (s.ok()) {
-      rep->index_reader.reset(index_reader);
+      if (s.ok()) {
+        rep->index_reader.reset(index_reader);
 
-      // Set filter block
-      if (rep->filter_policy) {
-        rep->filter.reset(ReadFilter(rep, meta_iter.get(), nullptr));
+        // Set filter block
+        if (rep->filter_policy) {
+          rep->filter.reset(ReadFilter(rep, meta_iter.get(), nullptr));
+        }
+      } else {
+        delete index_reader;
       }
-    } else {
-      delete index_reader;
     }
   }
 

table/block_based_table_reader.h

@@ -63,12 +63,15 @@ class BlockBasedTable : public TableReader {
   // to nullptr and returns a non-ok status.
   //
   // *file must remain live while this Table is in use.
+  // *prefetch_blocks can be used to disable prefetching of index and filter
+  //  blocks at statup
   static Status Open(const ImmutableCFOptions& ioptions,
                      const EnvOptions& env_options,
                      const BlockBasedTableOptions& table_options,
                      const InternalKeyComparator& internal_key_comparator,
                      unique_ptr<RandomAccessFile>&& file, uint64_t file_size,
-                     unique_ptr<TableReader>* table_reader);
+                     unique_ptr<TableReader>* table_reader,
+                     bool prefetch_index_and_filter = true);
 
   bool PrefixMayMatch(const Slice& internal_key);
 

util/sst_dump_test.cc

@@ -147,6 +147,28 @@ TEST(SSTDumpToolTest, FullFilterBlock) {
     delete[] usage[i];
   }
 }
+
+TEST(SSTDumpToolTest, GetProperties) {
+  table_options_.filter_policy.reset(rocksdb::NewBloomFilterPolicy(10, false));
+  std::string file_name = "rocksdb_sst_test.sst";
+  createSST(file_name, table_options_);
+
+  char* usage[3];
+  for (int i = 0; i < 3; i++) {
+    usage[i] = new char[optLength];
+  }
+  snprintf(usage[0], optLength, "./sst_dump");
+  snprintf(usage[1], optLength, "--show_properties");
+  snprintf(usage[2], optLength, "--file=rocksdb_sst_test.sst");
+
+  rocksdb::SSTDumpTool tool;
+  ASSERT_TRUE(!tool.Run(3, usage));
+
+  cleanup(file_name);
+  for (int i = 0; i < 3; i++) {
+    delete[] usage[i];
+  }
+}
 }  // namespace rocksdb
 
 int main(int argc, char** argv) { return rocksdb::test::RunAllTests(); }

util/sst_dump_tool.cc

@@ -15,6 +15,8 @@
 
 namespace rocksdb {
 
+using std::dynamic_pointer_cast;
+
 SstFileReader::SstFileReader(const std::string& file_path,
                              bool verify_checksum,
                              bool output_hex)
@@ -23,15 +25,15 @@ SstFileReader::SstFileReader(const std::string& file_path,
     internal_comparator_(BytewiseComparator()) {
   fprintf(stdout, "Process %s\n", file_path.c_str());
 
-  init_result_ = NewTableReader(file_name_);
+  init_result_ = GetTableReader(file_name_);
 }
 
 extern const uint64_t kBlockBasedTableMagicNumber;
 extern const uint64_t kLegacyBlockBasedTableMagicNumber;
 extern const uint64_t kPlainTableMagicNumber;
 extern const uint64_t kLegacyPlainTableMagicNumber;
 
-Status SstFileReader::NewTableReader(const std::string& file_path) {
+Status SstFileReader::GetTableReader(const std::string& file_path) {
   uint64_t magic_number;
 
   // read table magic number
@@ -66,13 +68,36 @@ Status SstFileReader::NewTableReader(const std::string& file_path) {
   }
 
   if (s.ok()) {
-    s = options_.table_factory->NewTableReader(
-        ioptions_, soptions_, internal_comparator_, std::move(file_), file_size,
-        &table_reader_);
+    s = NewTableReader(ioptions_, soptions_, internal_comparator_,
+                       std::move(file_), file_size, &table_reader_);
   }
   return s;
 }
 
+Status SstFileReader::NewTableReader(
+    const ImmutableCFOptions& ioptions, const EnvOptions& soptions,
+    const InternalKeyComparator& internal_comparator,
+    unique_ptr<RandomAccessFile>&& file, uint64_t file_size,
+    unique_ptr<TableReader>* table_reader) {
+  // We need to turn off pre-fetching of index and filter nodes for
+  // BlockBasedTable
+  shared_ptr<BlockBasedTableFactory> block_table_factory =
+      dynamic_pointer_cast<BlockBasedTableFactory>(options_.table_factory);
+
+  if (block_table_factory) {
+    return block_table_factory->NewTableReader(
+        ioptions_, soptions_, internal_comparator_, std::move(file_), file_size,
+        &table_reader_, /*enable_prefetch=*/false);
+  }
+
+  assert(!block_table_factory);
+
+  // For all other factory implementation
+  return options_.table_factory->NewTableReader(
+      ioptions_, soptions_, internal_comparator_, std::move(file_), file_size,
+      &table_reader_);
+}
+
 Status SstFileReader::DumpTable(const std::string& out_filename) {
   unique_ptr<WritableFile> out_file;
   Env* env = Env::Default();

util/sst_dump_tool_imp.h

@@ -53,12 +53,21 @@ class SstFileReader {
   Status getStatus() { return init_result_; }
 
  private:
-  Status NewTableReader(const std::string& file_path);
+  // Get the TableReader implementation for the sst file
+  Status GetTableReader(const std::string& file_path);
   Status ReadTableProperties(uint64_t table_magic_number,
                              RandomAccessFile* file, uint64_t file_size);
   Status SetTableOptionsByMagicNumber(uint64_t table_magic_number);
   Status SetOldTableOptions();
 
+  // Helper function to call the factory with settings specific to the
+  // factory implementation
+  Status NewTableReader(const ImmutableCFOptions& ioptions,
+                        const EnvOptions& soptions,
+                        const InternalKeyComparator& internal_comparator,
+                        unique_ptr<RandomAccessFile>&& file, uint64_t file_size,
+                        unique_ptr<TableReader>* table_reader);
+
   std::string file_name_;
   uint64_t read_num_;
   bool verify_checksum_;