Fix SIGFAULT when running sst_dump on v2.6 db

Summary: Fix the sigfault when running sst_dump on v2.6 db.

Test Plan:
    git checkout bba6595b1f
    make clean
    make db_bench
    ./db_bench --db=/tmp/some/db --benchmarks=fillseq
    arc patch D18039
    make clean
    make sst_dump
    ./sst_dump --file=/tmp/some/db --command=check

Reviewers: igor, haobo, sdong

Reviewed By: sdong

CC: leveldb

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

table/block_based_table_reader.cc

@@ -22,6 +22,7 @@
 #include "rocksdb/options.h"
 #include "rocksdb/statistics.h"
 #include "rocksdb/table.h"
+#include "rocksdb/table_properties.h"
 
 #include "table/block.h"
 #include "table/filter_block.h"
@@ -366,17 +367,7 @@ Status BlockBasedTable::Open(const Options& options, const EnvOptions& soptions,
 
   // Read the properties
   bool found_properties_block = true;
-  meta_iter->Seek(kPropertiesBlock);
+  s = SeekToPropertiesBlock(meta_iter.get(), &found_properties_block);
-  if (meta_iter->status().ok() &&
-      (!meta_iter->Valid() || meta_iter->key() != kPropertiesBlock)) {
-    meta_iter->Seek(kPropertiesBlockOldName);
-    if (meta_iter->status().ok() &&
-        (!meta_iter->Valid() || meta_iter->key() != kPropertiesBlockOldName)) {
-      found_properties_block = false;
-      Log(WARN_LEVEL, rep->options.info_log,
-          "Cannot find Properties block from file.");
-    }
-  }
 
   if (found_properties_block) {
     s = meta_iter->status();
@@ -394,6 +385,9 @@ Status BlockBasedTable::Open(const Options& options, const EnvOptions& soptions,
     } else {
       rep->table_properties.reset(table_properties);
     }
+  } else {
+    Log(WARN_LEVEL, rep->options.info_log,
+        "Cannot find Properties block from file.");
   }
 
   // Will use block cache for index/filter blocks access?

table/block_based_table_reader.h

@@ -199,8 +199,4 @@ class BlockBasedTable : public TableReader {
   void operator=(const TableReader&) = delete;
 };
 
-// Backward compatible properties block name. Limited in block based
-// table.
-extern const std::string kPropertiesBlockOldName;
-
 }  // namespace rocksdb

table/meta_blocks.cc

@@ -244,17 +244,19 @@ Status ReadTableProperties(RandomAccessFile* file, uint64_t file_size,
       metaindex_block.NewIterator(BytewiseComparator()));
 
   // -- Read property block
-  // This function is not used by BlockBasedTable, so we don't have to
+  bool found_properties_block = true;
-  // worry about old properties block name.
+  s = SeekToPropertiesBlock(meta_iter.get(), &found_properties_block);
-  meta_iter->Seek(kPropertiesBlock);
+  if (!s.ok()) {
+    return s;
+  }
+
   TableProperties table_properties;
-  if (meta_iter->Valid() &&
+  if (found_properties_block == true) {
-      meta_iter->key() == kPropertiesBlock &&
-      meta_iter->status().ok()) {
     s = ReadProperties(meta_iter->value(), file, env, info_log, properties);
   } else {
-    s = Status::Corruption(
+    s = Status::Corruption("Unable to read the property block.");
-        "Unable to read the property block from the plain table");
+    Log(WARN_LEVEL, info_log,
+        "Cannot find Properties block from file.");
   }
 
   return s;

table/meta_blocks.h

@@ -129,4 +129,10 @@ Status ReadTableMagicNumber(RandomAccessFile* file, uint64_t file_size,
                             const Options& options,
                             const EnvOptions& env_options,
                             uint64_t* table_magic_number);
+
+// Seek to the properties block.
+// If it successfully seeks to the properties block, "is_found" will be
+// set to true.
+extern Status SeekToPropertiesBlock(Iterator* meta_iter, bool* is_found);
+
 }  // namespace rocksdb

table/table_properties.cc

@@ -4,6 +4,8 @@
 //  of patent rights can be found in the PATENTS file in the same directory.
 
 #include "rocksdb/table_properties.h"
+#include "rocksdb/iterator.h"
+#include "rocksdb/env.h"
 
 namespace rocksdb {
 
@@ -94,4 +96,20 @@ extern const std::string kPropertiesBlock = "rocksdb.properties";
 // Old property block name for backward compatibility
 extern const std::string kPropertiesBlockOldName = "rocksdb.stats";
 
+// Seek to the properties block.
+// Return true if it successfully seeks to the properties block.
+Status SeekToPropertiesBlock(Iterator* meta_iter, bool* is_found) {
+  *is_found = true;
+  meta_iter->Seek(kPropertiesBlock);
+  if (meta_iter->status().ok() &&
+      (!meta_iter->Valid() || meta_iter->key() != kPropertiesBlock)) {
+    meta_iter->Seek(kPropertiesBlockOldName);
+    if (meta_iter->status().ok() &&
+        (!meta_iter->Valid() || meta_iter->key() != kPropertiesBlockOldName)) {
+      *is_found = false;
+    }
+  }
+  return meta_iter->status();
+}
+
 }  // namespace rocksdb

tools/sst_dump.cc

@@ -118,10 +118,10 @@ Status SstFileReader::SetTableOptionsByMagicNumber(uint64_t table_magic_number,
   Status s = rocksdb::ReadTableProperties(file, file_size, table_magic_number,
                                           options_.env, options_.info_log.get(),
                                           &table_properties);
-  std::unique_ptr<TableProperties> props_guard(table_properties);
   if (!s.ok()) {
     return s;
   }
+  std::unique_ptr<TableProperties> props_guard(table_properties);
 
   if (table_magic_number == kBlockBasedTableMagicNumber) {
     options_.table_factory = std::make_shared<BlockBasedTableFactory>();