andreacavalli/rocksdb
0
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Revert "Make Comparator into a Customizable Object (#8336)"

Browse Source 
This reverts commit 6ad0810393.
This commit is contained in:
Andrew Kryczka 2021-06-23 15:02:20 -07:00
parent 6fc2818ef8
commit 6ee0acdbb4
8 changed files with 44 additions and 152 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
include/rocksdb/comparator.h
View File

@ -10,7 +10,6 @@
#include <string>
#include "rocksdb/customizable.h"
#include "rocksdb/rocksdb_namespace.h"
namespace ROCKSDB_NAMESPACE {
@ -21,7 +20,7 @@ class Slice;
// used as keys in an sstable or a database. A Comparator implementation
// must be thread-safe since rocksdb may invoke its methods concurrently
// from multiple threads.
class Comparator : public Customizable {
class Comparator {
public:
Comparator() : timestamp_size_(0) {}
@ -38,11 +37,7 @@ class Comparator : public Customizable {
virtual ~Comparator() {}
static Status CreateFromString(const ConfigOptions& opts,
const std::string& id,
const Comparator** comp);
static const char* Type() { return "Comparator"; }
// Three-way comparison. Returns value:
// < 0 iff "a" < "b",
// == 0 iff "a" == "b",

3
include/rocksdb/configurable.h
View File

@ -8,7 +8,6 @@
#pragma once
#include <atomic>
#include <string>
#include <unordered_map>
#include <unordered_set>
@ -270,7 +269,7 @@ class Configurable {
protected:
// True once the object is prepared. Once the object is prepared, only
// mutable options can be configured.
std::atomic<bool> prepared_;
bool prepared_;
// Returns the raw pointer for the associated named option.
// The name is typically the name of an option registered via the

1
include/rocksdb/utilities/options_type.h
View File

@ -35,6 +35,7 @@ enum class OptionType {
kCompactionPri,
kSliceTransform,
kCompressionType,
kComparator,
kCompactionFilter,
kCompactionFilterFactory,
kCompactionStopStyle,

40
options/cf_options.cc
View File

@ -535,30 +535,22 @@ static std::unordered_map<std::string, OptionTypeInfo>
OptionVerificationType::kNormal, OptionTypeFlags::kNone,
{0, OptionType::kCompressionType})},
{"comparator",
OptionTypeInfo::AsCustomRawPtr<const Comparator>(
offset_of(&ImmutableCFOptions::user_comparator),
OptionVerificationType::kByName, OptionTypeFlags::kCompareLoose,
// Serializes a Comparator
[](const ConfigOptions& /*opts*/, const std::string&,
const void* addr, std::string* value) {
// it's a const pointer of const Comparator*
const auto* ptr = static_cast<const Comparator* const*>(addr);
// Since the user-specified comparator will be wrapped by
// InternalKeyComparator, we should persist the user-specified
// one instead of InternalKeyComparator.
if (*ptr == nullptr) {
*value = kNullptrString;
} else {
const Comparator* root_comp = (*ptr)->GetRootComparator();
if (root_comp == nullptr) {
root_comp = (*ptr);
}
*value = root_comp->Name();
}
return Status::OK();
},
/* Use the default match function*/ nullptr)},
{offset_of(&ImmutableCFOptions::user_comparator),
OptionType::kComparator, OptionVerificationType::kByName,
OptionTypeFlags::kCompareLoose,
// Parses the string and sets the corresponding comparator
[](const ConfigOptions& opts, const std::string& /*name*/,
const std::string& value, void* addr) {
auto old_comparator = static_cast<const Comparator**>(addr);
const Comparator* new_comparator = *old_comparator;
Status status =
opts.registry->NewStaticObject(value, &new_comparator);
if (status.ok()) {
*old_comparator = new_comparator;
return status;
}
return Status::OK();
}}},
{"memtable_insert_with_hint_prefix_extractor",
{offset_of(
&ImmutableCFOptions::memtable_insert_with_hint_prefix_extractor),

36
options/customizable_test.cc
View File

@ -771,15 +771,6 @@ static int RegisterTestObjects(ObjectLibrary& library,
guard->reset(new mock::MockTableFactory());
return guard->get();
});
library.Register<const Comparator>(
test::SimpleSuffixReverseComparator::kClassName(),
[](const std::string& /*uri*/,
std::unique_ptr<const Comparator>* /*guard*/,
std::string* /* errmsg */) {
static test::SimpleSuffixReverseComparator ssrc;
return &ssrc;
});
return static_cast<int>(library.GetFactoryCount(&num_types));
}
@ -789,7 +780,6 @@ static int RegisterLocalObjects(ObjectLibrary& library,
// Load any locally defined objects here
return static_cast<int>(library.GetFactoryCount(&num_types));
}
#endif // !ROCKSDB_LITE
class LoadCustomizableTest : public testing::Test {
public:
@ -829,31 +819,7 @@ TEST_F(LoadCustomizableTest, LoadTableFactoryTest) {
ASSERT_STREQ(factory->Name(), "MockTable");
}
}
TEST_F(LoadCustomizableTest, LoadComparatorTest) {
const Comparator* bytewise = BytewiseComparator();
const Comparator* reverse = ReverseBytewiseComparator();
const Comparator* result = nullptr;
ASSERT_NOK(Comparator::CreateFromString(
config_options_, test::SimpleSuffixReverseComparator::kClassName(),
&result));
ASSERT_OK(
Comparator::CreateFromString(config_options_, bytewise->Name(), &result));
ASSERT_EQ(result, bytewise);
ASSERT_OK(
Comparator::CreateFromString(config_options_, reverse->Name(), &result));
ASSERT_EQ(result, reverse);
if (RegisterTests("Test")) {
ASSERT_OK(Comparator::CreateFromString(
config_options_, test::SimpleSuffixReverseComparator::kClassName(),
&result));
ASSERT_NE(result, nullptr);
ASSERT_STREQ(result->Name(),
test::SimpleSuffixReverseComparator::kClassName());
}
}
#endif // !ROCKSDB_LITE
} // namespace ROCKSDB_NAMESPACE
int main(int argc, char** argv) {

17
options/options_helper.cc
View File

@ -562,6 +562,23 @@ bool SerializeSingleOptionHelper(const void* opt_address,
: kNullptrString;
break;
}
case OptionType::kComparator: {
// it's a const pointer of const Comparator*
const auto* ptr = static_cast<const Comparator* const*>(opt_address);
// Since the user-specified comparator will be wrapped by
// InternalKeyComparator, we should persist the user-specified one
// instead of InternalKeyComparator.
if (*ptr == nullptr) {
*value = kNullptrString;
} else {
const Comparator* root_comp = (*ptr)->GetRootComparator();
if (root_comp == nullptr) {
root_comp = (*ptr);
}
*value = root_comp->Name();
}
break;
}
case OptionType::kCompactionFilter: {
// it's a const pointer of const CompactionFilter*
const auto* ptr =

6
test_util/testutil.h
View File

@ -98,8 +98,10 @@ class PlainInternalKeyComparator : public InternalKeyComparator {
class SimpleSuffixReverseComparator : public Comparator {
public:
SimpleSuffixReverseComparator() {}
static const char* kClassName() { return "SimpleSuffixReverseComparator"; }
virtual const char* Name() const override { return kClassName(); }
virtual const char* Name() const override {
return "SimpleSuffixReverseComparator";
}
virtual int Compare(const Slice& a, const Slice& b) const override {
Slice prefix_a = Slice(a.data(), 8);

86
util/comparator.cc
View File

@ -8,17 +8,11 @@
// found in the LICENSE file. See the AUTHORS file for names of contributors.
#include "rocksdb/comparator.h"
#include <stdint.h>
#include <algorithm>
#include <memory>
#include <mutex>
#include "options/configurable_helper.h"
#include "port/port.h"
#include "rocksdb/slice.h"
#include "rocksdb/utilities/object_registry.h"
namespace ROCKSDB_NAMESPACE {
@ -26,8 +20,8 @@ namespace {
class BytewiseComparatorImpl : public Comparator {
public:
BytewiseComparatorImpl() { }
static const char* kClassName() { return "leveldb.BytewiseComparator"; }
const char* Name() const override { return kClassName(); }
const char* Name() const override { return "leveldb.BytewiseComparator"; }
int Compare(const Slice& a, const Slice& b) const override {
return a.compare(b);
@ -145,10 +139,9 @@ class ReverseBytewiseComparatorImpl : public BytewiseComparatorImpl {
public:
ReverseBytewiseComparatorImpl() { }
static const char* kClassName() {
const char* Name() const override {
return "rocksdb.ReverseBytewiseComparator";
}
const char* Name() const override { return kClassName(); }
int Compare(const Slice& a, const Slice& b) const override {
return -a.compare(b);
@ -227,77 +220,4 @@ const Comparator* ReverseBytewiseComparator() {
return &rbytewise;
}
#ifndef ROCKSDB_LITE
static int RegisterBuiltinComparators(ObjectLibrary& library,
const std::string& /*arg*/) {
library.Register<const Comparator>(
BytewiseComparatorImpl::kClassName(),
[](const std::string& /*uri*/,
std::unique_ptr<const Comparator>* /*guard */,
std::string* /* errmsg */) { return BytewiseComparator(); });
library.Register<const Comparator>(
ReverseBytewiseComparatorImpl::kClassName(),
[](const std::string& /*uri*/,
std::unique_ptr<const Comparator>* /*guard */,
std::string* /* errmsg */) { return ReverseBytewiseComparator(); });
return 2;
}
#endif // ROCKSDB_LITE
Status Comparator::CreateFromString(const ConfigOptions& config_options,
const std::string& value,
const Comparator** result) {
#ifndef ROCKSDB_LITE
static std::once_flag once;
std::call_once(once, [&]() {
RegisterBuiltinComparators(*(ObjectLibrary::Default().get()), "");
});
#endif // ROCKSDB_LITE
std::string id;
std::unordered_map<std::string, std::string> opt_map;
Status status =
ConfigurableHelper::GetOptionsMap(value, *result, &id, &opt_map);
if (!status.ok()) { // GetOptionsMap failed
return status;
}
std::string curr_opts;
#ifndef ROCKSDB_LITE
if (*result != nullptr && (*result)->GetId() == id) {
// Try to get the existing options, ignoring any errors
ConfigOptions embedded = config_options;
embedded.delimiter = ";";
(*result)->GetOptionString(embedded, &curr_opts).PermitUncheckedError();
}
#endif
if (id == BytewiseComparatorImpl::kClassName()) {
*result = BytewiseComparator();
} else if (id == ReverseBytewiseComparatorImpl::kClassName()) {
*result = ReverseBytewiseComparator();
} else if (value.empty()) {
// No Id and no options. Clear the object
*result = nullptr;
return Status::OK();
} else if (id.empty()) { // We have no Id but have options. Not good
return Status::NotSupported("Cannot reset object ", id);
} else {
#ifndef ROCKSDB_LITE
status = config_options.registry->NewStaticObject(id, result);
#else
status = Status::NotSupported("Cannot load object in LITE mode ", id);
#endif // ROCKSDB_LITE
if (!status.ok()) {
if (config_options.ignore_unsupported_options &&
status.IsNotSupported()) {
return Status::OK();
} else {
return status;
}
} else if (!curr_opts.empty() || !opt_map.empty()) {
Comparator* comparator = const_cast<Comparator*>(*result);
status = ConfigurableHelper::ConfigureNewObject(
config_options, comparator, id, curr_opts, opt_map);
}
}
return status;
}
} // namespace ROCKSDB_NAMESPACE