Change ioptions to store user_comparator, fix bug

Summary:
change ioptions.comparator to user_comparator instread of internal_comparator.
Also change Comparator* to InternalKeyComparator* to make its type explicitly.

Test Plan: make all check -j64

Reviewers: andrewkr, sdong, yiwu

Reviewed By: yiwu

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D65121
main
Aaron Gao 8 years ago
parent 869ae5d786
commit 59a7c0337b
  1. 5
      db/column_family.cc
  2. 1
      db/column_family.h
  3. 4
      db/column_family_test.cc
  4. 3
      db/db_impl.cc
  5. 1
      db/db_impl.h
  6. 2
      db/db_options_test.cc
  7. 1
      db/table_properties_collector_test.cc
  8. 15
      table/block_based_table_builder.cc
  9. 8
      table/table_reader_bench.cc
  10. 2
      util/cf_options.cc
  11. 2
      util/cf_options.h

@ -139,10 +139,8 @@ Status CheckConcurrentWritesSupported(const ColumnFamilyOptions& cf_options) {
} }
ColumnFamilyOptions SanitizeOptions(const ImmutableDBOptions& db_options, ColumnFamilyOptions SanitizeOptions(const ImmutableDBOptions& db_options,
const InternalKeyComparator* icmp,
const ColumnFamilyOptions& src) { const ColumnFamilyOptions& src) {
ColumnFamilyOptions result = src; ColumnFamilyOptions result = src;
result.comparator = icmp;
size_t clamp_max = std::conditional< size_t clamp_max = std::conditional<
sizeof(size_t) == 4, std::integral_constant<size_t, 0xffffffff>, sizeof(size_t) == 4, std::integral_constant<size_t, 0xffffffff>,
std::integral_constant<uint64_t, 64ull << 30>>::type::value; std::integral_constant<uint64_t, 64ull << 30>>::type::value;
@ -349,8 +347,7 @@ ColumnFamilyData::ColumnFamilyData(
refs_(0), refs_(0),
dropped_(false), dropped_(false),
internal_comparator_(cf_options.comparator), internal_comparator_(cf_options.comparator),
initial_cf_options_( initial_cf_options_(SanitizeOptions(db_options, cf_options)),
SanitizeOptions(db_options, &internal_comparator_, cf_options)),
ioptions_(db_options, initial_cf_options_), ioptions_(db_options, initial_cf_options_),
mutable_cf_options_(initial_cf_options_), mutable_cf_options_(initial_cf_options_),
write_buffer_manager_(write_buffer_manager), write_buffer_manager_(write_buffer_manager),

@ -137,7 +137,6 @@ extern Status CheckConcurrentWritesSupported(
const ColumnFamilyOptions& cf_options); const ColumnFamilyOptions& cf_options);
extern ColumnFamilyOptions SanitizeOptions(const ImmutableDBOptions& db_options, extern ColumnFamilyOptions SanitizeOptions(const ImmutableDBOptions& db_options,
const InternalKeyComparator* icmp,
const ColumnFamilyOptions& src); const ColumnFamilyOptions& src);
// Wrap user defined table proproties collector factories `from cf_options` // Wrap user defined table proproties collector factories `from cf_options`
// into internal ones in int_tbl_prop_collector_factories. Add a system internal // into internal ones in int_tbl_prop_collector_factories. Add a system internal

@ -2182,8 +2182,8 @@ TEST_F(ColumnFamilyTest, SanitizeOptions) {
original.write_buffer_size = original.write_buffer_size =
l * 4 * 1024 * 1024 + i * 1024 * 1024 + j * 1024 + k; l * 4 * 1024 * 1024 + i * 1024 * 1024 + j * 1024 + k;
ColumnFamilyOptions result = SanitizeOptions( ColumnFamilyOptions result =
ImmutableDBOptions(db_options), nullptr, original); SanitizeOptions(ImmutableDBOptions(db_options), original);
ASSERT_TRUE(result.level0_stop_writes_trigger >= ASSERT_TRUE(result.level0_stop_writes_trigger >=
result.level0_slowdown_writes_trigger); result.level0_slowdown_writes_trigger);
ASSERT_TRUE(result.level0_slowdown_writes_trigger >= ASSERT_TRUE(result.level0_slowdown_writes_trigger >=

@ -122,12 +122,11 @@ struct DBImpl::WriteContext {
}; };
Options SanitizeOptions(const std::string& dbname, Options SanitizeOptions(const std::string& dbname,
const InternalKeyComparator* icmp,
const Options& src) { const Options& src) {
auto db_options = SanitizeOptions(dbname, DBOptions(src)); auto db_options = SanitizeOptions(dbname, DBOptions(src));
ImmutableDBOptions immutable_db_options(db_options); ImmutableDBOptions immutable_db_options(db_options);
auto cf_options = auto cf_options =
SanitizeOptions(immutable_db_options, icmp, ColumnFamilyOptions(src)); SanitizeOptions(immutable_db_options, ColumnFamilyOptions(src));
return Options(db_options, cf_options); return Options(db_options, cf_options);
} }

@ -1089,7 +1089,6 @@ class DBImpl : public DB {
}; };
extern Options SanitizeOptions(const std::string& db, extern Options SanitizeOptions(const std::string& db,
const InternalKeyComparator* icmp,
const Options& src); const Options& src);
extern DBOptions SanitizeOptions(const std::string& db, const DBOptions& src); extern DBOptions SanitizeOptions(const std::string& db, const DBOptions& src);

@ -64,7 +64,7 @@ class DBOptionsTest : public DBTestBase {
Options options; Options options;
ImmutableDBOptions db_options(options); ImmutableDBOptions db_options(options);
test::RandomInitCFOptions(&options, rnd); test::RandomInitCFOptions(&options, rnd);
auto sanitized_options = SanitizeOptions(db_options, nullptr, options); auto sanitized_options = SanitizeOptions(db_options, options);
auto opt_map = GetMutableCFOptionsMap(sanitized_options); auto opt_map = GetMutableCFOptionsMap(sanitized_options);
delete options.compaction_filter; delete options.compaction_filter;
return opt_map; return opt_map;

@ -391,7 +391,6 @@ void TestInternalKeyPropertiesCollector(
// SanitizeOptions(). // SanitizeOptions().
options.info_log = std::make_shared<test::NullLogger>(); options.info_log = std::make_shared<test::NullLogger>();
options = SanitizeOptions("db", // just a place holder options = SanitizeOptions("db", // just a place holder
&pikc,
options); options);
ImmutableCFOptions ioptions(options); ImmutableCFOptions ioptions(options);
GetIntTblPropCollectorFactory(ioptions, &int_tbl_prop_collector_factories); GetIntTblPropCollectorFactory(ioptions, &int_tbl_prop_collector_factories);

@ -74,7 +74,7 @@ class IndexBuilder {
Slice index_block_contents; Slice index_block_contents;
std::unordered_map<std::string, Slice> meta_blocks; std::unordered_map<std::string, Slice> meta_blocks;
}; };
explicit IndexBuilder(const Comparator* comparator) explicit IndexBuilder(const InternalKeyComparator* comparator)
: comparator_(comparator) {} : comparator_(comparator) {}
virtual ~IndexBuilder() {} virtual ~IndexBuilder() {}
@ -107,7 +107,7 @@ class IndexBuilder {
virtual size_t EstimatedSize() const = 0; virtual size_t EstimatedSize() const = 0;
protected: protected:
const Comparator* comparator_; const InternalKeyComparator* comparator_;
}; };
// This index builder builds space-efficient index block. // This index builder builds space-efficient index block.
@ -121,7 +121,7 @@ class IndexBuilder {
// substitute key that serves the same function. // substitute key that serves the same function.
class ShortenedIndexBuilder : public IndexBuilder { class ShortenedIndexBuilder : public IndexBuilder {
public: public:
explicit ShortenedIndexBuilder(const Comparator* comparator, explicit ShortenedIndexBuilder(const InternalKeyComparator* comparator,
int index_block_restart_interval) int index_block_restart_interval)
: IndexBuilder(comparator), : IndexBuilder(comparator),
index_block_builder_(index_block_restart_interval) {} index_block_builder_(index_block_restart_interval) {}
@ -180,7 +180,7 @@ class ShortenedIndexBuilder : public IndexBuilder {
// data copy or small heap allocations for prefixes. // data copy or small heap allocations for prefixes.
class HashIndexBuilder : public IndexBuilder { class HashIndexBuilder : public IndexBuilder {
public: public:
explicit HashIndexBuilder(const Comparator* comparator, explicit HashIndexBuilder(const InternalKeyComparator* comparator,
const SliceTransform* hash_key_extractor, const SliceTransform* hash_key_extractor,
int index_block_restart_interval) int index_block_restart_interval)
: IndexBuilder(comparator), : IndexBuilder(comparator),
@ -269,7 +269,8 @@ class HashIndexBuilder : public IndexBuilder {
namespace { namespace {
// Create a index builder based on its type. // Create a index builder based on its type.
IndexBuilder* CreateIndexBuilder(IndexType type, const Comparator* comparator, IndexBuilder* CreateIndexBuilder(IndexType type,
const InternalKeyComparator* comparator,
const SliceTransform* prefix_extractor, const SliceTransform* prefix_extractor,
int index_block_restart_interval) { int index_block_restart_interval) {
switch (type) { switch (type) {
@ -893,8 +894,8 @@ Status BlockBasedTableBuilder::Finish() {
r->table_options.filter_policy->Name() : ""; r->table_options.filter_policy->Name() : "";
r->props.index_size = r->props.index_size =
r->index_builder->EstimatedSize() + kBlockTrailerSize; r->index_builder->EstimatedSize() + kBlockTrailerSize;
r->props.comparator_name = r->ioptions.comparator != nullptr r->props.comparator_name = r->ioptions.user_comparator != nullptr
? r->ioptions.comparator->Name() ? r->ioptions.user_comparator->Name()
: "nullptr"; : "nullptr";
r->props.merge_operator_name = r->ioptions.merge_operator != nullptr r->props.merge_operator_name = r->ioptions.merge_operator != nullptr
? r->ioptions.merge_operator->Name() ? r->ioptions.merge_operator->Name()

@ -168,10 +168,10 @@ void TableReaderBenchmark(Options& opts, EnvOptions& env_options,
if (!through_db) { if (!through_db) {
std::string value; std::string value;
MergeContext merge_context; MergeContext merge_context;
GetContext get_context(ioptions.comparator, ioptions.merge_operator, GetContext get_context(
ioptions.info_log, ioptions.statistics, ioptions.user_comparator, ioptions.merge_operator,
GetContext::kNotFound, Slice(key), &value, ioptions.info_log, ioptions.statistics, GetContext::kNotFound,
nullptr, &merge_context, env); Slice(key), &value, nullptr, &merge_context, env);
s = table_reader->Get(read_options, key, &get_context); s = table_reader->Get(read_options, key, &get_context);
} else { } else {
s = db->Get(read_options, key, &result); s = db->Get(read_options, key, &result);

@ -30,7 +30,7 @@ ImmutableCFOptions::ImmutableCFOptions(const ImmutableDBOptions& db_options,
compaction_options_universal(cf_options.compaction_options_universal), compaction_options_universal(cf_options.compaction_options_universal),
compaction_options_fifo(cf_options.compaction_options_fifo), compaction_options_fifo(cf_options.compaction_options_fifo),
prefix_extractor(cf_options.prefix_extractor.get()), prefix_extractor(cf_options.prefix_extractor.get()),
comparator(cf_options.comparator), user_comparator(cf_options.comparator),
merge_operator(cf_options.merge_operator.get()), merge_operator(cf_options.merge_operator.get()),
compaction_filter(cf_options.compaction_filter), compaction_filter(cf_options.compaction_filter),
compaction_filter_factory(cf_options.compaction_filter_factory.get()), compaction_filter_factory(cf_options.compaction_filter_factory.get()),

@ -34,7 +34,7 @@ struct ImmutableCFOptions {
const SliceTransform* prefix_extractor; const SliceTransform* prefix_extractor;
const Comparator* comparator; const Comparator* user_comparator;
MergeOperator* merge_operator; MergeOperator* merge_operator;

Loading…
Cancel
Save