From 3525aac9e5714be6635361dee5e320854ed814fc Mon Sep 17 00:00:00 2001 From: Igor Canadi Date: Wed, 18 Jun 2014 07:04:37 +0200 Subject: [PATCH] Change order of parameters in adaptive table factory Summary: This is minor, but if we put the writing talbe factory as the third parameter, when we add a new table format, we'll have a situation: 1) block based factory 2) plain table factory 3) output factory 4) new format factory I think it makes more sense to have output as the first parameter. Also, fixed a NewAdaptiveTableFactory() call in unit test Test Plan: unit test Reviewers: sdong Reviewed By: sdong Subscribers: leveldb Differential Revision: https://reviews.facebook.net/D19119 --- db/plain_table_db_test.cc | 6 ++++-- include/rocksdb/table.h | 6 +++--- table/adaptive_table_factory.cc | 22 +++++++++++----------- table/adaptive_table_factory.h | 6 +++--- 4 files changed, 21 insertions(+), 19 deletions(-) diff --git a/db/plain_table_db_test.cc b/db/plain_table_db_test.cc index 2e1e8be6e..0df57a41b 100644 --- a/db/plain_table_db_test.cc +++ b/db/plain_table_db_test.cc @@ -862,8 +862,10 @@ TEST(PlainTableDBTest, AdaptiveTable) { options.create_if_missing = false; std::shared_ptr dummy_factory; - options.table_factory.reset( - NewAdaptiveTableFactory(dummy_factory, dummy_factory, false)); + std::shared_ptr block_based_factory( + NewBlockBasedTableFactory()); + options.table_factory.reset(NewAdaptiveTableFactory( + block_based_factory, dummy_factory, dummy_factory)); Reopen(&options); ASSERT_EQ("v3", Get("1000000000000foo")); ASSERT_EQ("v2", Get("0000000000000bar")); diff --git a/include/rocksdb/table.h b/include/rocksdb/table.h index fb169fdf2..01bfae431 100644 --- a/include/rocksdb/table.h +++ b/include/rocksdb/table.h @@ -207,14 +207,14 @@ class TableFactory { // Create a special table factory that can open both of block based table format // and plain table, based on setting inside the SST files. It should be used to // convert a DB from one table format to another. +// @table_factory_to_write: the table factory used when writing to new files. // @block_based_table_factory: block based table factory to use. If NULL, use // a default one. // @plain_table_factory: plain table factory to use. If NULL, use a default one. -// @table_factory_to_write: the table factory used when writing to new files. extern TableFactory* NewAdaptiveTableFactory( + std::shared_ptr table_factory_to_write = nullptr, std::shared_ptr block_based_table_factory = nullptr, - std::shared_ptr plain_table_factory = nullptr, - std::shared_ptr table_factory_to_write = nullptr); + std::shared_ptr plain_table_factory = nullptr); #endif // ROCKSDB_LITE diff --git a/table/adaptive_table_factory.cc b/table/adaptive_table_factory.cc index 9d05be7c7..2a7d17251 100644 --- a/table/adaptive_table_factory.cc +++ b/table/adaptive_table_factory.cc @@ -10,21 +10,21 @@ namespace rocksdb { AdaptiveTableFactory::AdaptiveTableFactory( + std::shared_ptr table_factory_to_write, std::shared_ptr block_based_table_factory, - std::shared_ptr plain_table_factory, - std::shared_ptr table_factory_to_write) - : block_based_table_factory_(block_based_table_factory), - plain_table_factory_(plain_table_factory), - table_factory_to_write_(table_factory_to_write) { + std::shared_ptr plain_table_factory) + : table_factory_to_write_(table_factory_to_write), + block_based_table_factory_(block_based_table_factory), + plain_table_factory_(plain_table_factory) { + if (!table_factory_to_write_) { + table_factory_to_write_ = block_based_table_factory_; + } if (!plain_table_factory_) { plain_table_factory_.reset(NewPlainTableFactory()); } if (!block_based_table_factory_) { block_based_table_factory_.reset(NewBlockBasedTableFactory()); } - if (!table_factory_to_write_) { - table_factory_to_write_ = block_based_table_factory_; - } } extern const uint64_t kPlainTableMagicNumber; @@ -62,11 +62,11 @@ TableBuilder* AdaptiveTableFactory::NewTableBuilder( } extern TableFactory* NewAdaptiveTableFactory( + std::shared_ptr table_factory_to_write, std::shared_ptr block_based_table_factory, - std::shared_ptr plain_table_factory, - std::shared_ptr table_factory_to_write) { + std::shared_ptr plain_table_factory) { return new AdaptiveTableFactory( - block_based_table_factory, plain_table_factory, table_factory_to_write); + table_factory_to_write, block_based_table_factory, plain_table_factory); } } // namespace rocksdb diff --git a/table/adaptive_table_factory.h b/table/adaptive_table_factory.h index 4bf2fb2c3..e5632f786 100644 --- a/table/adaptive_table_factory.h +++ b/table/adaptive_table_factory.h @@ -26,9 +26,9 @@ class AdaptiveTableFactory : public TableFactory { ~AdaptiveTableFactory() {} explicit AdaptiveTableFactory( + std::shared_ptr table_factory_to_write, std::shared_ptr block_based_table_factory, - std::shared_ptr plain_table_factory, - std::shared_ptr table_factory_to_write); + std::shared_ptr plain_table_factory); const char* Name() const override { return "AdaptiveTableFactory"; } Status NewTableReader(const Options& options, const EnvOptions& soptions, const InternalKeyComparator& internal_comparator, @@ -41,9 +41,9 @@ class AdaptiveTableFactory : public TableFactory { override; private: + std::shared_ptr table_factory_to_write_; std::shared_ptr block_based_table_factory_; std::shared_ptr plain_table_factory_; - std::shared_ptr table_factory_to_write_; }; } // namespace rocksdb