From 96d989f70dbb612c3395d9e155e2f64c3098a564 Mon Sep 17 00:00:00 2001 From: Jinfu Leng Date: Mon, 23 Feb 2015 16:08:27 -0800 Subject: [PATCH] catch config errors with L0 file count triggers Test Plan: Run "make clean && make all check" Reviewers: rven, igor, yhchiang, kradhakrishnan, MarkCallaghan, sdong Reviewed By: sdong Subscribers: dhruba Differential Revision: https://reviews.facebook.net/D33627 --- db/column_family.cc | 37 +++++++++++++++++++++++++++++++++++-- db/column_family.h | 3 ++- db/column_family_test.cc | 28 ++++++++++++++++++++++++++++ db/db_impl.cc | 3 ++- db/db_test.cc | 10 ++++++++++ 5 files changed, 77 insertions(+), 4 deletions(-) diff --git a/db/column_family.cc b/db/column_family.cc index 7128234b0..0335db9c0 100644 --- a/db/column_family.cc +++ b/db/column_family.cc @@ -101,7 +101,8 @@ const Comparator* ColumnFamilyHandleImpl::user_comparator() const { } ColumnFamilyOptions SanitizeOptions(const InternalKeyComparator* icmp, - const ColumnFamilyOptions& src) { + const ColumnFamilyOptions& src, + Logger* info_log) { ColumnFamilyOptions result = src; result.comparator = icmp; #ifdef OS_MACOSX @@ -163,6 +164,37 @@ ColumnFamilyOptions SanitizeOptions(const InternalKeyComparator* icmp, result.level0_stop_writes_trigger = std::numeric_limits::max(); } + if (result.level0_stop_writes_trigger < + result.level0_slowdown_writes_trigger || + result.level0_slowdown_writes_trigger < + result.level0_file_num_compaction_trigger) { + Warn(info_log, + "This condition must be satisfied: " + "level0_stop_writes_trigger(%d) >= " + "level0_slowdown_writes_trigger(%d) >= " + "level0_file_num_compaction_trigger(%d)", + result.level0_stop_writes_trigger, + result.level0_slowdown_writes_trigger, + result.level0_file_num_compaction_trigger); + if (result.level0_slowdown_writes_trigger < + result.level0_file_num_compaction_trigger) { + result.level0_slowdown_writes_trigger = + result.level0_file_num_compaction_trigger; + } + if (result.level0_stop_writes_trigger < + result.level0_slowdown_writes_trigger) { + result.level0_stop_writes_trigger = result.level0_slowdown_writes_trigger; + } + Warn(info_log, + "Adjust the value to " + "level0_stop_writes_trigger(%d)" + "level0_slowdown_writes_trigger(%d)" + "level0_file_num_compaction_trigger(%d)", + result.level0_stop_writes_trigger, + result.level0_slowdown_writes_trigger, + result.level0_file_num_compaction_trigger); + } + return result; } @@ -237,7 +269,8 @@ ColumnFamilyData::ColumnFamilyData( refs_(0), dropped_(false), internal_comparator_(cf_options.comparator), - options_(*db_options, SanitizeOptions(&internal_comparator_, cf_options)), + options_(*db_options, SanitizeOptions(&internal_comparator_, cf_options, + db_options->info_log.get())), ioptions_(options_), mutable_cf_options_(options_, ioptions_), write_buffer_(write_buffer), diff --git a/db/column_family.h b/db/column_family.h index e32a71975..1bddef6f4 100644 --- a/db/column_family.h +++ b/db/column_family.h @@ -123,7 +123,8 @@ struct SuperVersion { }; extern ColumnFamilyOptions SanitizeOptions(const InternalKeyComparator* icmp, - const ColumnFamilyOptions& src); + const ColumnFamilyOptions& src, + Logger* info_log); class ColumnFamilySet; diff --git a/db/column_family_test.cc b/db/column_family_test.cc index 209f7b528..ff25ac1cf 100644 --- a/db/column_family_test.cc +++ b/db/column_family_test.cc @@ -326,6 +326,13 @@ class ColumnFamilyTest { Random rnd_; }; +class DumbLogger : public Logger { + public: + using Logger::Logv; + virtual void Logv(const char* format, va_list ap) {} + virtual size_t GetLogFileSize() const { return 0; } +}; + TEST(ColumnFamilyTest, DontReuseColumnFamilyID) { for (int iter = 0; iter < 3; ++iter) { Open(); @@ -1012,6 +1019,27 @@ TEST(ColumnFamilyTest, CreateMissingColumnFamilies) { Close(); } +TEST(ColumnFamilyTest, SanitizeOptions) { + DumbLogger logger; + for (int i = 1; i <= 3; i++) { + for (int j = 1; j <= 3; j++) { + for (int k = 1; k <= 3; k++) { + ColumnFamilyOptions original; + original.level0_stop_writes_trigger = i; + original.level0_slowdown_writes_trigger = j; + original.level0_file_num_compaction_trigger = k; + ColumnFamilyOptions result = SanitizeOptions(NULL, original, &logger); + ASSERT_TRUE(result.level0_stop_writes_trigger >= + result.level0_slowdown_writes_trigger); + ASSERT_TRUE(result.level0_slowdown_writes_trigger >= + result.level0_file_num_compaction_trigger); + ASSERT_TRUE(result.level0_file_num_compaction_trigger == + original.level0_file_num_compaction_trigger); + } + } + } +} + } // namespace rocksdb int main(int argc, char** argv) { diff --git a/db/db_impl.cc b/db/db_impl.cc index 54e13f6a3..593ac2324 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -108,7 +108,8 @@ Options SanitizeOptions(const std::string& dbname, const InternalKeyComparator* icmp, const Options& src) { auto db_options = SanitizeOptions(dbname, DBOptions(src)); - auto cf_options = SanitizeOptions(icmp, ColumnFamilyOptions(src)); + auto cf_options = SanitizeOptions(icmp, ColumnFamilyOptions(src), + db_options.info_log.get()); return Options(db_options, cf_options); } diff --git a/db/db_test.cc b/db/db_test.cc index 070a133d8..e4674d558 100644 --- a/db/db_test.cc +++ b/db/db_test.cc @@ -1742,6 +1742,16 @@ TEST(DBTest, GetLevel0Ordering) { } while (ChangeOptions()); } +TEST(DBTest, WrongLevel0Config) { + Options options = CurrentOptions(); + Close(); + ASSERT_OK(DestroyDB(dbname_, options)); + options.level0_stop_writes_trigger = 1; + options.level0_slowdown_writes_trigger = 2; + options.level0_file_num_compaction_trigger = 3; + ASSERT_OK(DB::Open(options, dbname_, &db_)); +} + TEST(DBTest, GetOrderedByLevels) { do { CreateAndReopenWithCF({"pikachu"}, CurrentOptions());