RocksDB 2.8 to be able to read files generated by 2.6

Summary:
From 2.6 to 2.7, property block name is renamed from rocksdb.stats to rocksdb.properties. Older properties were not able to be loaded. In 2.8, we seem to have added some logic that uses property block without checking null pointers, which create segment faults.

In this patch, we fix it by:
(1) try rocksdb.stats if rocksdb.properties is not found
(2) add some null checking before consuming rep->table_properties

Test Plan: make sure a file generated in 2.7 couldn't be opened now can be opened.

Reviewers: haobo, igor, yhchiang

Reviewed By: igor

CC: ljin, xjin, dhruba, kailiu, leveldb

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

table/block_based_table_reader.cc

@@ -365,8 +365,20 @@ Status BlockBasedTable::Open(const Options& options, const EnvOptions& soptions,
   s = ReadMetaBlock(rep, &meta, &meta_iter);
 
   // Read the properties
+  bool found_properties_block = true;
   meta_iter->Seek(kPropertiesBlock);
-  if (meta_iter->Valid() && meta_iter->key() == 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)) {
+      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();
     TableProperties* table_properties = nullptr;
     if (s.ok()) {
@@ -1019,11 +1031,13 @@ Status BlockBasedTable::CreateIndexReader(IndexReader** index_reader) {
   // Some old version of block-based tables don't have index type present in
   // table properties. If that's the case we can safely use the kBinarySearch.
   auto index_type = BlockBasedTableOptions::kBinarySearch;
-  auto& props = rep_->table_properties->user_collected_properties;
+  if (rep_->table_properties) {
-  auto pos = props.find(BlockBasedTablePropertyNames::kIndexType);
+    auto& props = rep_->table_properties->user_collected_properties;
-  if (pos != props.end()) {
+    auto pos = props.find(BlockBasedTablePropertyNames::kIndexType);
-    index_type = static_cast<BlockBasedTableOptions::IndexType>(
+    if (pos != props.end()) {
-        DecodeFixed32(pos->second.c_str()));
+      index_type = static_cast<BlockBasedTableOptions::IndexType>(
+          DecodeFixed32(pos->second.c_str()));
+    }
   }
 
   auto file = rep_->file.get();
@@ -1082,7 +1096,10 @@ uint64_t BlockBasedTable::ApproximateOffsetOf(const Slice& key) {
     // key is past the last key in the file. If table_properties is not
     // available, approximate the offset by returning the offset of the
     // metaindex block (which is right near the end of the file).
-    result = rep_->table_properties->data_size;
+    result = 0;
+    if (rep_->table_properties) {
+      result = rep_->table_properties->data_size;
+    }
     // table_properties is not present in the table.
     if (result == 0) {
       result = rep_->metaindex_handle.offset();

table/block_based_table_reader.h

@@ -12,6 +12,7 @@
 #include <stdint.h>
 #include <memory>
 #include <utility>
+#include <string>
 
 #include "rocksdb/statistics.h"
 #include "rocksdb/status.h"
@@ -198,4 +199,8 @@ 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,6 +244,8 @@ 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
+  // worry about old properties block name.
   meta_iter->Seek(kPropertiesBlock);
   TableProperties table_properties;
   if (meta_iter->Valid() &&

table/table_properties.cc

@@ -91,5 +91,7 @@ const std::string TablePropertiesNames::kFixedKeyLen =
     "rocksdb.fixed.key.length";
 
 extern const std::string kPropertiesBlock = "rocksdb.properties";
+// Old property block name for backward compatibility
+extern const std::string kPropertiesBlockOldName = "rocksdb.stats";
 
 }  // namespace rocksdb