From 7f47ba0e26d08379c4fe6a132c3ad8c15afea20b Mon Sep 17 00:00:00 2001 From: Igor Canadi Date: Wed, 29 Apr 2015 10:52:31 -0700 Subject: [PATCH] Fix possible SIGSEGV in CompactRange (github issue #596) Summary: For very detailed explanation of what's happening read this: https://github.com/facebook/rocksdb/issues/596 Test Plan: make check + new unit test Reviewers: yhchiang, anthony, rven Reviewed By: rven Subscribers: adamretter, dhruba, leveldb Differential Revision: https://reviews.facebook.net/D37779 --- db/db_impl.cc | 3 ++- db/db_test.cc | 19 +++++++++++++++++++ db/file_indexer.cc | 3 +++ db/file_indexer_test.cc | 2 ++ db/version_set.cc | 9 +++++++++ util/autovector.h | 4 ++-- 6 files changed, 37 insertions(+), 3 deletions(-) diff --git a/db/db_impl.cc b/db/db_impl.cc index 5fad92208..6752e0d0a 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -1279,7 +1279,8 @@ Status DBImpl::CompactRange(ColumnFamilyHandle* column_family, { InstrumentedMutexLock l(&mutex_); Version* base = cfd->current(); - for (int level = 1; level < cfd->NumberLevels(); level++) { + for (int level = 1; level < base->storage_info()->num_non_empty_levels(); + level++) { if (base->storage_info()->OverlapInLevel(level, begin, end)) { max_level_with_files = level; } diff --git a/db/db_test.cc b/db/db_test.cc index 5dc607e24..2eb49213d 100644 --- a/db/db_test.cc +++ b/db/db_test.cc @@ -12695,6 +12695,25 @@ TEST_F(DBTest, PromoteL0Failure) { ASSERT_TRUE(status.IsInvalidArgument()); } +// Github issue #596 +TEST_F(DBTest, HugeNumberOfLevels) { + Options options = CurrentOptions(); + options.write_buffer_size = 2 * 1024 * 1024; // 2MB + options.max_bytes_for_level_base = 2 * 1024 * 1024; // 2MB + options.num_levels = 12; + options.max_background_compactions = 10; + options.max_bytes_for_level_multiplier = 2; + options.level_compaction_dynamic_level_bytes = true; + DestroyAndReopen(options); + + Random rnd(301); + for (int i = 0; i < 300000; ++i) { + ASSERT_OK(Put(Key(i), RandomString(&rnd, 1024))); + } + + ASSERT_OK(db_->CompactRange(nullptr, nullptr)); +} + } // namespace rocksdb int main(int argc, char** argv) { diff --git a/db/file_indexer.cc b/db/file_indexer.cc index c59036bd6..222cca9c0 100644 --- a/db/file_indexer.cc +++ b/db/file_indexer.cc @@ -20,6 +20,9 @@ FileIndexer::FileIndexer(const Comparator* ucmp) size_t FileIndexer::NumLevelIndex() const { return next_level_index_.size(); } size_t FileIndexer::LevelIndexSize(size_t level) const { + if (level >= next_level_index_.size()) { + return 0; + } return next_level_index_[level].num_index; } diff --git a/db/file_indexer_test.cc b/db/file_indexer_test.cc index 85e083546..98fea47fe 100644 --- a/db/file_indexer_test.cc +++ b/db/file_indexer_test.cc @@ -11,6 +11,7 @@ #include "db/file_indexer.h" #include "db/dbformat.h" #include "db/version_edit.h" +#include "port/stack_trace.h" #include "rocksdb/comparator.h" #include "util/testharness.h" #include "util/testutil.h" @@ -343,6 +344,7 @@ TEST_F(FileIndexerTest, mixed) { } // namespace rocksdb int main(int argc, char** argv) { + rocksdb::port::InstallStackTraceHandler(); ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); } diff --git a/db/version_set.cc b/db/version_set.cc index 8fb8e300e..95e4b7718 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -1219,6 +1219,10 @@ bool Version::Unref() { bool VersionStorageInfo::OverlapInLevel(int level, const Slice* smallest_user_key, const Slice* largest_user_key) { + if (level >= num_non_empty_levels_) { + // empty level, no overlap + return false; + } return SomeFileOverlapsRange(*internal_comparator_, (level > 0), level_files_brief_[level], smallest_user_key, largest_user_key); @@ -1263,6 +1267,11 @@ int VersionStorageInfo::PickLevelForMemTableOutput( void VersionStorageInfo::GetOverlappingInputs( int level, const InternalKey* begin, const InternalKey* end, std::vector* inputs, int hint_index, int* file_index) { + if (level >= num_non_empty_levels_) { + // this level is empty, no overlapping inputs + return; + } + inputs->clear(); Slice user_begin, user_end; if (begin != nullptr) { diff --git a/util/autovector.h b/util/autovector.h index 9362536d3..c9befe965 100644 --- a/util/autovector.h +++ b/util/autovector.h @@ -190,16 +190,16 @@ class autovector { bool empty() const { return size() == 0; } - // will not check boundry const_reference operator[](size_type n) const { + assert(n < size()); return n < kSize ? values_[n] : vect_[n - kSize]; } reference operator[](size_type n) { + assert(n < size()); return n < kSize ? values_[n] : vect_[n - kSize]; } - // will check boundry const_reference at(size_type n) const { assert(n < size()); return (*this)[n];