Skip to content

Commit

Permalink
Added GetFactoryCount/Names/Types to ObjectRegistry (facebook#9358)
Browse files Browse the repository at this point in the history
Summary:
These methods allow for more thorough testing of the ObjectRegistry and Customizable infrastructure in a simpler manner.  With this change, the Customizable tests can now check what factories are registered and attempt to create each of them in a systematic fashion.

With this change, I think all of the factories registered with the ObjectRegistry/CreateFromString are now tested via the customizable_test classes.

Note that there were a few other minor changes.  There was a "posix://*" register with the ObjectRegistry which was missed during the PatternEntry conversion -- these changes found that.  The nickname and default names for the FileSystem classes was also inverted.

Pull Request resolved: facebook#9358

Reviewed By: pdillinger

Differential Revision: D33433542

Pulled By: mrambacher

fbshipit-source-id: 9a32da74e6620745b4eeffb2712be70eeeadfa7e
  • Loading branch information
mrambacher authored and facebook-github-bot committed May 16, 2022
1 parent c4cd8e1 commit 204a42c
Show file tree
Hide file tree
Showing 13 changed files with 595 additions and 335 deletions.
9 changes: 9 additions & 0 deletions env/env_encryption.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1139,6 +1139,15 @@ CTREncryptionProvider::CTREncryptionProvider(
RegisterOptions("Cipher", &cipher_, &ctr_encryption_provider_type_info);
}

bool CTREncryptionProvider::IsInstanceOf(const std::string& name) const {
// Special case for test purposes.
if (name == "1://test" && cipher_ != nullptr) {
return cipher_->IsInstanceOf(ROT13BlockCipher::kClassName());
} else {
return EncryptionProvider::IsInstanceOf(name);
}
}

// GetPrefixLength returns the length of the prefix that is added to every file
// and used for storing encryption options.
// For optimal performance, the prefix length should be a multiple of
Expand Down
2 changes: 1 addition & 1 deletion env/env_encryption_ctr.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class CTREncryptionProvider : public EncryptionProvider {

static const char* kClassName() { return "CTR"; }
const char* Name() const override { return kClassName(); }

bool IsInstanceOf(const std::string& name) const override;
// GetPrefixLength returns the length of the prefix that is added to every
// file
// and used for storing encryption options.
Expand Down
4 changes: 2 additions & 2 deletions env/env_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,8 @@ class PosixDynamicLibrary : public DynamicLibrary {
class PosixClock : public SystemClock {
public:
static const char* kClassName() { return "PosixClock"; }
const char* Name() const override { return kClassName(); }
const char* NickName() const override { return kDefaultName(); }
const char* Name() const override { return kDefaultName(); }
const char* NickName() const override { return kClassName(); }

uint64_t NowMicros() override {
struct timeval tv;
Expand Down
7 changes: 7 additions & 0 deletions env/fs_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,13 @@ class PosixFileSystem : public FileSystem {
const char* NickName() const override { return kDefaultName(); }

~PosixFileSystem() override {}
bool IsInstanceOf(const std::string& name) const override {
if (name == "posix") {
return true;
} else {
return FileSystem::IsInstanceOf(name);
}
}

void SetFD_CLOEXEC(int fd, const EnvOptions* options) {
if ((options == nullptr || options->set_fd_cloexec) && fd > 0) {
Expand Down
3 changes: 3 additions & 0 deletions include/rocksdb/memtablerep.h
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,9 @@ class MemTableRepFactory : public Customizable {
static Status CreateFromString(const ConfigOptions& config_options,
const std::string& id,
std::unique_ptr<MemTableRepFactory>* factory);
static Status CreateFromString(const ConfigOptions& config_options,
const std::string& id,
std::shared_ptr<MemTableRepFactory>* factory);

virtual MemTableRep* CreateMemTableRep(const MemTableRep::KeyComparator&,
Allocator*, const SliceTransform*,
Expand Down
25 changes: 25 additions & 0 deletions include/rocksdb/utilities/object_registry.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include <mutex>
#include <string>
#include <unordered_map>
#include <unordered_set>
#include <vector>

#include "rocksdb/status.h"
Expand Down Expand Up @@ -217,6 +218,18 @@ class ObjectLibrary {
// @param num_types returns how many unique types are registered.
size_t GetFactoryCount(size_t* num_types) const;

// Returns the number of factories registered for this library
// for the input type.
// @param num_types returns how many unique types are registered.
size_t GetFactoryCount(const std::string& type) const;

// Returns the registered factory names for the input type
// names is updated to include the names for the type
void GetFactoryNames(const std::string& type,
std::vector<std::string>* names) const;

void GetFactoryTypes(std::unordered_set<std::string>* types) const;

void Dump(Logger* logger) const;

// Registers the factory with the library for the name.
Expand Down Expand Up @@ -497,6 +510,18 @@ class ObjectRegistry {
}
}

// Returns the number of factories registered for this library
// for the input type.
// @param num_types returns how many unique types are registered.
size_t GetFactoryCount(const std::string& type) const;

// Returns the names of registered factories for the input type.
// names is updated to include the names for the type
void GetFactoryNames(const std::string& type,
std::vector<std::string>* names) const;

void GetFactoryTypes(std::unordered_set<std::string>* types) const;

// Dump the contents of the registry to the logger
void Dump(Logger* logger) const;

Expand Down
10 changes: 2 additions & 8 deletions options/cf_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -596,10 +596,7 @@ static std::unordered_map<std::string, OptionTypeInfo>
auto* shared =
static_cast<std::shared_ptr<MemTableRepFactory>*>(addr);
Status s =
MemTableRepFactory::CreateFromString(opts, value, &factory);
if (factory && s.ok()) {
shared->reset(factory.release());
}
MemTableRepFactory::CreateFromString(opts, value, shared);
return s;
}}},
{"memtable",
Expand All @@ -612,10 +609,7 @@ static std::unordered_map<std::string, OptionTypeInfo>
auto* shared =
static_cast<std::shared_ptr<MemTableRepFactory>*>(addr);
Status s =
MemTableRepFactory::CreateFromString(opts, value, &factory);
if (factory && s.ok()) {
shared->reset(factory.release());
}
MemTableRepFactory::CreateFromString(opts, value, shared);
return s;
}}},
{"table_factory",
Expand Down
Loading

0 comments on commit 204a42c

Please sign in to comment.