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

diff --git a/table/block_based_table_reader.cc b/table/block_based_table_reader.cc
index 967836811eba69cb98181dbd762ca25eeeb3aff9..7b72c8f6c913a73b6be3b1d0ec1ca18d66d2aa54 100644 (file)
--- a/table/block_based_table_reader.cc
+++ b/table/block_based_table_reader.cc
@@ -327,8 +327,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, 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()) {
@@ -979,11 +991,13 @@ Status BlockBasedTable::CreateIndexReader(IndexReader** index_reader) const {
   // 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()));
+    }
   }
 
   switch (index_type) {
@@ -1023,7 +1037,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 8b8f09bd39dc0aeb6e82f7ae2fedbf687cca9f80..97493db82b3a8e316ec75a266dc9006be7d42670 100644 (file)
--- 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 4465899fba4ec87609b5b7fddedc3de930d9d07c..4e940b97e03323a7b7650044da14c713075b00b4 100644 (file)
--- 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 2da1a975a486799c0178eab7020cf21754d8dcf4..de9eb94771ed89d45fd370e8e0c561b6cc9ea736 100644 (file)
--- 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