From 5cef458a2c88245fa49b481004c91b170f2daa48 Mon Sep 17 00:00:00 2001
From: sdong <siying.d@fb.com>
Date: Wed, 16 Apr 2014 19:30:33 -0700
Subject: [PATCH] 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 | 31 ++++++++++++++++++++++++-------
 table/block_based_table_reader.h  |  5 +++++
 table/meta_blocks.cc              |  2 ++
 table/table_properties.cc         |  2 ++
 4 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/table/block_based_table_reader.cc b/table/block_based_table_reader.cc
index f686239cb7..4e827bcd09 100644
--- a/table/block_based_table_reader.cc
+++ b/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;
-  auto pos = props.find(BlockBasedTablePropertyNames::kIndexType);
-  if (pos != props.end()) {
-    index_type = static_cast<BlockBasedTableOptions::IndexType>(
-        DecodeFixed32(pos->second.c_str()));
+  if (rep_->table_properties) {
+    auto& props = rep_->table_properties->user_collected_properties;
+    auto pos = props.find(BlockBasedTablePropertyNames::kIndexType);
+    if (pos != props.end()) {
+      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();
diff --git a/table/block_based_table_reader.h b/table/block_based_table_reader.h
index 613460634b..5cff9ce6a6 100644
--- a/table/block_based_table_reader.h
+++ b/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
diff --git a/table/meta_blocks.cc b/table/meta_blocks.cc
index 4465899fba..4e940b97e0 100644
--- a/table/meta_blocks.cc
+++ b/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() &&
diff --git a/table/table_properties.cc b/table/table_properties.cc
index 2da1a975a4..de9eb94771 100644
--- a/table/table_properties.cc
+++ b/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