From af6ad113a857a6ab9593063330219a0ce4605956 Mon Sep 17 00:00:00 2001 From: Yueh-Hsuan Chiang Date: Mon, 21 Apr 2014 17:49:47 -0700 Subject: [PATCH] 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 bba6595b1f3f42cf79bb21c2d5b981ede1cc0063 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 | 16 +++++----------- table/block_based_table_reader.h | 4 ---- table/meta_blocks.cc | 18 ++++++++++-------- table/meta_blocks.h | 6 ++++++ table/table_properties.cc | 18 ++++++++++++++++++ tools/sst_dump.cc | 2 +- 6 files changed, 40 insertions(+), 24 deletions(-) diff --git a/table/block_based_table_reader.cc b/table/block_based_table_reader.cc index b44fab155..35f6a194c 100644 --- a/table/block_based_table_reader.cc +++ b/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); - 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."); - } - } + s = SeekToPropertiesBlock(meta_iter.get(), &found_properties_block); 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? diff --git a/table/block_based_table_reader.h b/table/block_based_table_reader.h index 5cff9ce6a..d48c5d2c7 100644 --- a/table/block_based_table_reader.h +++ b/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 diff --git a/table/meta_blocks.cc b/table/meta_blocks.cc index 4e940b97e..5775c2a14 100644 --- a/table/meta_blocks.cc +++ b/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 - // worry about old properties block name. - meta_iter->Seek(kPropertiesBlock); + bool found_properties_block = true; + s = SeekToPropertiesBlock(meta_iter.get(), &found_properties_block); + if (!s.ok()) { + return s; + } + TableProperties table_properties; - if (meta_iter->Valid() && - meta_iter->key() == kPropertiesBlock && - meta_iter->status().ok()) { + if (found_properties_block == true) { s = ReadProperties(meta_iter->value(), file, env, info_log, properties); } else { - s = Status::Corruption( - "Unable to read the property block from the plain table"); + s = Status::Corruption("Unable to read the property block."); + Log(WARN_LEVEL, info_log, + "Cannot find Properties block from file."); } return s; diff --git a/table/meta_blocks.h b/table/meta_blocks.h index a355905a1..9371409cb 100644 --- a/table/meta_blocks.h +++ b/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 diff --git a/table/table_properties.cc b/table/table_properties.cc index de9eb9477..c7e141943 100644 --- a/table/table_properties.cc +++ b/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 diff --git a/tools/sst_dump.cc b/tools/sst_dump.cc index b8c470fe7..8e00d6913 100644 --- a/tools/sst_dump.cc +++ b/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 props_guard(table_properties); if (!s.ok()) { return s; } + std::unique_ptr props_guard(table_properties); if (table_magic_number == kBlockBasedTableMagicNumber) { options_.table_factory = std::make_shared();