Skip to content

Commit 3525aac

Browse files
committed
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
1 parent 8c265c0 commit 3525aac

File tree

4 files changed

+21
-19
lines changed

4 files changed

+21
-19
lines changed

db/plain_table_db_test.cc

+4-2
Original file line numberDiff line numberDiff line change
@@ -862,8 +862,10 @@ TEST(PlainTableDBTest, AdaptiveTable) {
862862

863863
options.create_if_missing = false;
864864
std::shared_ptr<TableFactory> dummy_factory;
865-
options.table_factory.reset(
866-
NewAdaptiveTableFactory(dummy_factory, dummy_factory, false));
865+
std::shared_ptr<TableFactory> block_based_factory(
866+
NewBlockBasedTableFactory());
867+
options.table_factory.reset(NewAdaptiveTableFactory(
868+
block_based_factory, dummy_factory, dummy_factory));
867869
Reopen(&options);
868870
ASSERT_EQ("v3", Get("1000000000000foo"));
869871
ASSERT_EQ("v2", Get("0000000000000bar"));

include/rocksdb/table.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -207,14 +207,14 @@ class TableFactory {
207207
// Create a special table factory that can open both of block based table format
208208
// and plain table, based on setting inside the SST files. It should be used to
209209
// convert a DB from one table format to another.
210+
// @table_factory_to_write: the table factory used when writing to new files.
210211
// @block_based_table_factory: block based table factory to use. If NULL, use
211212
// a default one.
212213
// @plain_table_factory: plain table factory to use. If NULL, use a default one.
213-
// @table_factory_to_write: the table factory used when writing to new files.
214214
extern TableFactory* NewAdaptiveTableFactory(
215+
std::shared_ptr<TableFactory> table_factory_to_write = nullptr,
215216
std::shared_ptr<TableFactory> block_based_table_factory = nullptr,
216-
std::shared_ptr<TableFactory> plain_table_factory = nullptr,
217-
std::shared_ptr<TableFactory> table_factory_to_write = nullptr);
217+
std::shared_ptr<TableFactory> plain_table_factory = nullptr);
218218

219219
#endif // ROCKSDB_LITE
220220

table/adaptive_table_factory.cc

+11-11
Original file line numberDiff line numberDiff line change
@@ -10,21 +10,21 @@
1010
namespace rocksdb {
1111

1212
AdaptiveTableFactory::AdaptiveTableFactory(
13+
std::shared_ptr<TableFactory> table_factory_to_write,
1314
std::shared_ptr<TableFactory> block_based_table_factory,
14-
std::shared_ptr<TableFactory> plain_table_factory,
15-
std::shared_ptr<TableFactory> table_factory_to_write)
16-
: block_based_table_factory_(block_based_table_factory),
17-
plain_table_factory_(plain_table_factory),
18-
table_factory_to_write_(table_factory_to_write) {
15+
std::shared_ptr<TableFactory> plain_table_factory)
16+
: table_factory_to_write_(table_factory_to_write),
17+
block_based_table_factory_(block_based_table_factory),
18+
plain_table_factory_(plain_table_factory) {
19+
if (!table_factory_to_write_) {
20+
table_factory_to_write_ = block_based_table_factory_;
21+
}
1922
if (!plain_table_factory_) {
2023
plain_table_factory_.reset(NewPlainTableFactory());
2124
}
2225
if (!block_based_table_factory_) {
2326
block_based_table_factory_.reset(NewBlockBasedTableFactory());
2427
}
25-
if (!table_factory_to_write_) {
26-
table_factory_to_write_ = block_based_table_factory_;
27-
}
2828
}
2929

3030
extern const uint64_t kPlainTableMagicNumber;
@@ -62,11 +62,11 @@ TableBuilder* AdaptiveTableFactory::NewTableBuilder(
6262
}
6363

6464
extern TableFactory* NewAdaptiveTableFactory(
65+
std::shared_ptr<TableFactory> table_factory_to_write,
6566
std::shared_ptr<TableFactory> block_based_table_factory,
66-
std::shared_ptr<TableFactory> plain_table_factory,
67-
std::shared_ptr<TableFactory> table_factory_to_write) {
67+
std::shared_ptr<TableFactory> plain_table_factory) {
6868
return new AdaptiveTableFactory(
69-
block_based_table_factory, plain_table_factory, table_factory_to_write);
69+
table_factory_to_write, block_based_table_factory, plain_table_factory);
7070
}
7171

7272
} // namespace rocksdb

table/adaptive_table_factory.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@ class AdaptiveTableFactory : public TableFactory {
2626
~AdaptiveTableFactory() {}
2727

2828
explicit AdaptiveTableFactory(
29+
std::shared_ptr<TableFactory> table_factory_to_write,
2930
std::shared_ptr<TableFactory> block_based_table_factory,
30-
std::shared_ptr<TableFactory> plain_table_factory,
31-
std::shared_ptr<TableFactory> table_factory_to_write);
31+
std::shared_ptr<TableFactory> plain_table_factory);
3232
const char* Name() const override { return "AdaptiveTableFactory"; }
3333
Status NewTableReader(const Options& options, const EnvOptions& soptions,
3434
const InternalKeyComparator& internal_comparator,
@@ -41,9 +41,9 @@ class AdaptiveTableFactory : public TableFactory {
4141
override;
4242

4343
private:
44+
std::shared_ptr<TableFactory> table_factory_to_write_;
4445
std::shared_ptr<TableFactory> block_based_table_factory_;
4546
std::shared_ptr<TableFactory> plain_table_factory_;
46-
std::shared_ptr<TableFactory> table_factory_to_write_;
4747
};
4848

4949
} // namespace rocksdb

0 commit comments

Comments
 (0)