Skip to content

Commit

Permalink
Option type info functions (facebook#9411)
Browse files Browse the repository at this point in the history
Summary:
Add methods to set the various functions (Parse, Serialize, Equals) to the OptionTypeInfo.  These methods simplify the number of constructors required for OptionTypeInfo and make the code a little clearer.

Add functions to the OptionTypeInfo for Prepare and Validate.  These methods allow types other than Configurable and Customizable to have Prepare and Validate logic.  These methods could be used by an option to guarantee that its settings were in a range or that a value was initialized.

Pull Request resolved: facebook#9411

Reviewed By: pdillinger

Differential Revision: D36174849

Pulled By: mrambacher

fbshipit-source-id: 72517d8c6bab4723788a4c1a9e16590bff870125
  • Loading branch information
mrambacher authored and facebook-github-bot committed May 13, 2022
1 parent cdaa957 commit bfc6a8e
Show file tree
Hide file tree
Showing 13 changed files with 550 additions and 326 deletions.
1 change: 1 addition & 0 deletions HISTORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
* EXPERIMENTAL: Add new API AbortIO in file_system to abort the read requests submitted asynchronously.
* CompactionFilter::Decision has a new value: kRemoveWithSingleDelete. If CompactionFilter returns this decision, then CompactionIterator will use `SingleDelete` to mark a key as removed.
* Renamed CompactionFilter::Decision::kRemoveWithSingleDelete to kPurge since the latter sounds more general and hides the implementation details of how compaction iterator handles keys.
* Added ability to specify functions for Prepare and Validate to OptionsTypeInfo. Added methods to OptionTypeInfo to set the functions via an API. These methods are intended for RocksDB plugin developers for configuration management.

### Bug Fixes
* RocksDB calls FileSystem::Poll API during FilePrefetchBuffer destruction which impacts performance as it waits for read requets completion which is not needed anymore. Calling FileSystem::AbortIO to abort those requests instead fixes that performance issue.
Expand Down
1 change: 1 addition & 0 deletions db/compaction/compaction_job.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2952,6 +2952,7 @@ static std::unordered_map<std::string, OptionTypeInfo> cs_result_type_info = {
const void* addr1, const void* addr2, std::string* mismatch) {
const auto status1 = static_cast<const Status*>(addr1);
const auto status2 = static_cast<const Status*>(addr2);

StatusSerializationAdapter adatper1(*status1);
StatusSerializationAdapter adapter2(*status2);
return OptionTypeInfo::TypesAreEqual(opts, status_adapter_type_info,
Expand Down
101 changes: 87 additions & 14 deletions env/composite_env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
//
#include "env/composite_env_wrapper.h"
#include "rocksdb/utilities/options_type.h"
#include "util/string_util.h"

namespace ROCKSDB_NAMESPACE {
namespace {
Expand Down Expand Up @@ -382,19 +383,49 @@ Status CompositeEnv::NewDirectory(const std::string& name,
}

namespace {
static std::unordered_map<std::string, OptionTypeInfo>
composite_env_wrapper_type_info = {
static std::unordered_map<std::string, OptionTypeInfo> env_wrapper_type_info = {
#ifndef ROCKSDB_LITE
{"target",
{0, OptionType::kCustomizable, OptionVerificationType::kByName,
OptionTypeFlags::kDontSerialize | OptionTypeFlags::kRawPointer,
[](const ConfigOptions& opts, const std::string& /*name*/,
const std::string& value, void* addr) {
auto target = static_cast<EnvWrapper::Target*>(addr);
return Env::CreateFromString(opts, value, &(target->env),
&(target->guard));
},
nullptr, nullptr}},
{"target",
OptionTypeInfo(0, OptionType::kUnknown, OptionVerificationType::kByName,
OptionTypeFlags::kDontSerialize)
.SetParseFunc([](const ConfigOptions& opts,
const std::string& /*name*/, const std::string& value,
void* addr) {
auto target = static_cast<EnvWrapper::Target*>(addr);
return Env::CreateFromString(opts, value, &(target->env),
&(target->guard));
})
.SetEqualsFunc([](const ConfigOptions& opts,
const std::string& /*name*/, const void* addr1,
const void* addr2, std::string* mismatch) {
const auto target1 = static_cast<const EnvWrapper::Target*>(addr1);
const auto target2 = static_cast<const EnvWrapper::Target*>(addr2);
if (target1->env != nullptr) {
return target1->env->AreEquivalent(opts, target2->env, mismatch);
} else {
return (target2->env == nullptr);
}
})
.SetPrepareFunc([](const ConfigOptions& opts,
const std::string& /*name*/, void* addr) {
auto target = static_cast<EnvWrapper::Target*>(addr);
if (target->guard.get() != nullptr) {
target->env = target->guard.get();
} else if (target->env == nullptr) {
target->env = Env::Default();
}
return target->env->PrepareOptions(opts);
})
.SetValidateFunc([](const DBOptions& db_opts,
const ColumnFamilyOptions& cf_opts,
const std::string& /*name*/, const void* addr) {
const auto target = static_cast<const EnvWrapper::Target*>(addr);
if (target->env == nullptr) {
return Status::InvalidArgument("Target Env not specified");
} else {
return target->env->ValidateOptions(db_opts, cf_opts);
}
})},
#endif // ROCKSDB_LITE
};
static std::unordered_map<std::string, OptionTypeInfo>
Expand Down Expand Up @@ -425,7 +456,7 @@ CompositeEnvWrapper::CompositeEnvWrapper(Env* env,
const std::shared_ptr<FileSystem>& fs,
const std::shared_ptr<SystemClock>& sc)
: CompositeEnv(fs, sc), target_(env) {
RegisterOptions("", &target_, &composite_env_wrapper_type_info);
RegisterOptions("", &target_, &env_wrapper_type_info);
RegisterOptions("", &file_system_, &composite_fs_wrapper_type_info);
RegisterOptions("", &system_clock_, &composite_clock_wrapper_type_info);
}
Expand All @@ -434,7 +465,7 @@ CompositeEnvWrapper::CompositeEnvWrapper(const std::shared_ptr<Env>& env,
const std::shared_ptr<FileSystem>& fs,
const std::shared_ptr<SystemClock>& sc)
: CompositeEnv(fs, sc), target_(env) {
RegisterOptions("", &target_, &composite_env_wrapper_type_info);
RegisterOptions("", &target_, &env_wrapper_type_info);
RegisterOptions("", &file_system_, &composite_fs_wrapper_type_info);
RegisterOptions("", &system_clock_, &composite_clock_wrapper_type_info);
}
Expand All @@ -461,4 +492,46 @@ std::string CompositeEnvWrapper::SerializeOptions(
return options;
}
#endif // ROCKSDB_LITE

EnvWrapper::EnvWrapper(Env* t) : target_(t) {
RegisterOptions("", &target_, &env_wrapper_type_info);
}

EnvWrapper::EnvWrapper(std::unique_ptr<Env>&& t) : target_(std::move(t)) {
RegisterOptions("", &target_, &env_wrapper_type_info);
}

EnvWrapper::EnvWrapper(const std::shared_ptr<Env>& t) : target_(t) {
RegisterOptions("", &target_, &env_wrapper_type_info);
}

EnvWrapper::~EnvWrapper() {}

Status EnvWrapper::PrepareOptions(const ConfigOptions& options) {
target_.Prepare();
return Env::PrepareOptions(options);
}

#ifndef ROCKSDB_LITE
std::string EnvWrapper::SerializeOptions(const ConfigOptions& config_options,
const std::string& header) const {
auto parent = Env::SerializeOptions(config_options, "");
if (config_options.IsShallow() || target_.env == nullptr ||
target_.env == Env::Default()) {
return parent;
} else {
std::string result = header;
if (!StartsWith(parent, OptionTypeInfo::kIdPropName())) {
result.append(OptionTypeInfo::kIdPropName()).append("=");
}
result.append(parent);
if (!EndsWith(result, config_options.delimiter)) {
result.append(config_options.delimiter);
}
result.append("target=").append(target_.env->ToString(config_options));
return result;
}
}
#endif // ROCKSDB_LITE

} // namespace ROCKSDB_NAMESPACE
60 changes: 0 additions & 60 deletions env/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
#include "rocksdb/utilities/object_registry.h"
#include "rocksdb/utilities/options_type.h"
#include "util/autovector.h"
#include "util/string_util.h"

namespace ROCKSDB_NAMESPACE {
namespace {
Expand Down Expand Up @@ -1084,65 +1083,6 @@ Status ReadFileToString(Env* env, const std::string& fname, std::string* data) {
return ReadFileToString(fs.get(), fname, data);
}

namespace {
static std::unordered_map<std::string, OptionTypeInfo> env_wrapper_type_info = {
#ifndef ROCKSDB_LITE
{"target",
{0, OptionType::kCustomizable, OptionVerificationType::kByName,
OptionTypeFlags::kDontSerialize | OptionTypeFlags::kRawPointer,
[](const ConfigOptions& opts, const std::string& /*name*/,
const std::string& value, void* addr) {
EnvWrapper::Target* target = static_cast<EnvWrapper::Target*>(addr);
return Env::CreateFromString(opts, value, &(target->env),
&(target->guard));
},
nullptr, nullptr}},
#endif // ROCKSDB_LITE
};
} // namespace

EnvWrapper::EnvWrapper(Env* t) : target_(t) {
RegisterOptions("", &target_, &env_wrapper_type_info);
}

EnvWrapper::EnvWrapper(std::unique_ptr<Env>&& t) : target_(std::move(t)) {
RegisterOptions("", &target_, &env_wrapper_type_info);
}

EnvWrapper::EnvWrapper(const std::shared_ptr<Env>& t) : target_(t) {
RegisterOptions("", &target_, &env_wrapper_type_info);
}

EnvWrapper::~EnvWrapper() {
}

Status EnvWrapper::PrepareOptions(const ConfigOptions& options) {
target_.Prepare();
return Env::PrepareOptions(options);
}

#ifndef ROCKSDB_LITE
std::string EnvWrapper::SerializeOptions(const ConfigOptions& config_options,
const std::string& header) const {
auto parent = Env::SerializeOptions(config_options, "");
if (config_options.IsShallow() || target_.env == nullptr ||
target_.env == Env::Default()) {
return parent;
} else {
std::string result = header;
if (!StartsWith(parent, OptionTypeInfo::kIdPropName())) {
result.append(OptionTypeInfo::kIdPropName()).append("=");
}
result.append(parent);
if (!EndsWith(result, config_options.delimiter)) {
result.append(config_options.delimiter);
}
result.append("target=").append(target_.env->ToString(config_options));
return result;
}
}
#endif // ROCKSDB_LITE

namespace { // anonymous namespace

void AssignEnvOptions(EnvOptions* env_options, const DBOptions& options) {
Expand Down
21 changes: 17 additions & 4 deletions env/env_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
#include "env/unique_id_gen.h"
#include "logging/log_buffer.h"
#include "logging/logging.h"
#include "options/options_helper.h"
#include "port/malloc.h"
#include "port/port.h"
#include "port/stack_trace.h"
Expand Down Expand Up @@ -2937,7 +2938,7 @@ TEST_F(EnvTest, FailureToCreateLockFile) {
ASSERT_OK(DestroyDir(env, dir));
}

TEST_F(EnvTest, CreateDefaultEnv) {
TEST_F(CreateEnvTest, CreateDefaultEnv) {
ConfigOptions options;
options.ignore_unsupported_options = false;

Expand Down Expand Up @@ -2989,7 +2990,7 @@ class WrappedEnv : public EnvWrapper {
}
};
} // namespace
TEST_F(EnvTest, CreateMockEnv) {
TEST_F(CreateEnvTest, CreateMockEnv) {
ConfigOptions options;
options.ignore_unsupported_options = false;
WrappedEnv::Register(*(options.registry->AddLibrary("test")), "");
Expand Down Expand Up @@ -3017,7 +3018,7 @@ TEST_F(EnvTest, CreateMockEnv) {
opt_str = copy->ToString(options);
}

TEST_F(EnvTest, CreateWrappedEnv) {
TEST_F(CreateEnvTest, CreateWrappedEnv) {
ConfigOptions options;
options.ignore_unsupported_options = false;
WrappedEnv::Register(*(options.registry->AddLibrary("test")), "");
Expand Down Expand Up @@ -3054,7 +3055,7 @@ TEST_F(EnvTest, CreateWrappedEnv) {
ASSERT_TRUE(guard->AreEquivalent(options, copy.get(), &mismatch));
}

TEST_F(EnvTest, CreateCompositeEnv) {
TEST_F(CreateEnvTest, CreateCompositeEnv) {
ConfigOptions options;
options.ignore_unsupported_options = false;
std::shared_ptr<Env> guard, copy;
Expand Down Expand Up @@ -3109,6 +3110,18 @@ TEST_F(EnvTest, CreateCompositeEnv) {
ASSERT_NE(env, nullptr);
ASSERT_NE(env, Env::Default());
ASSERT_TRUE(guard->AreEquivalent(options, copy.get(), &mismatch));

guard.reset(new CompositeEnvWrapper(nullptr, timed_fs, clock));
ColumnFamilyOptions cf_opts;
DBOptions db_opts;
db_opts.env = guard.get();
auto comp = db_opts.env->CheckedCast<CompositeEnvWrapper>();
ASSERT_NE(comp, nullptr);
ASSERT_EQ(comp->Inner(), nullptr);
ASSERT_NOK(ValidateOptions(db_opts, cf_opts));
ASSERT_OK(db_opts.env->PrepareOptions(options));
ASSERT_NE(comp->Inner(), nullptr);
ASSERT_OK(ValidateOptions(db_opts, cf_opts));
}
#endif // ROCKSDB_LITE

Expand Down
9 changes: 9 additions & 0 deletions include/rocksdb/utilities/customizable_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,15 @@
// This source code is licensed under both the GPLv2 (found in the
// COPYING file in the root directory) and Apache 2.0 License
// (found in the LICENSE.Apache file in the root directory).
//
// The methods in this file are used to instantiate new Customizable
// instances of objects. These methods are most typically used by
// the "CreateFromString" method of a customizable class.
// If not developing a new Type of customizable class, you probably
// do not need the methods in this file.
//
// See https://github.com/facebook/rocksdb/wiki/RocksDB-Configurable-Objects
// for more information on how to develop and use customizable objects

#pragma once
#include <functional>
Expand Down
Loading

0 comments on commit bfc6a8e

Please sign in to comment.