diff --git a/include/rocksdb/configurable.h b/include/rocksdb/configurable.h index f43d78f86..de693eb9e 100644 --- a/include/rocksdb/configurable.h +++ b/include/rocksdb/configurable.h @@ -56,7 +56,6 @@ class Configurable { }; public: - Configurable(); virtual ~Configurable() {} // Returns the raw pointer of the named options that is used by this @@ -262,10 +261,6 @@ class Configurable { virtual Status ValidateOptions(const DBOptions& db_opts, const ColumnFamilyOptions& cf_opts) const; - // Returns true if this object has been initialized via PrepareOptions, false - // otherwise. Once an object has been prepared, only mutable options may be - // changed. - virtual bool IsPrepared() const { return is_prepared_; } // Splits the input opt_value into the ID field and the remaining options. // The input opt_value can be in the form of "name" or "name=value @@ -286,10 +281,6 @@ class Configurable { std::string* id, std::unordered_map* options); protected: - // True once the object is prepared. Once the object is prepared, only - // mutable options can be configured. - std::atomic is_prepared_; - // Returns the raw pointer for the associated named option. // The name is typically the name of an option registered via the // Classes may override this method to provide further specialization (such as diff --git a/options/configurable.cc b/options/configurable.cc index 5469c96d6..a98d2646b 100644 --- a/options/configurable.cc +++ b/options/configurable.cc @@ -17,8 +17,6 @@ namespace ROCKSDB_NAMESPACE { -Configurable::Configurable() : is_prepared_(false) {} - void Configurable::RegisterOptions( const std::string& name, void* opt_ptr, const std::unordered_map* type_map) { @@ -68,9 +66,6 @@ Status Configurable::PrepareOptions(const ConfigOptions& opts) { #else (void)opts; #endif // ROCKSDB_LITE - if (status.ok()) { - is_prepared_ = true; - } return status; } diff --git a/options/configurable_test.cc b/options/configurable_test.cc index 5983e2dc6..fd412528f 100644 --- a/options/configurable_test.cc +++ b/options/configurable_test.cc @@ -331,6 +331,40 @@ TEST_F(ConfigurableTest, PrepareOptionsTest) { ASSERT_EQ(*up, 0); } +TEST_F(ConfigurableTest, CopyObjectTest) { + class CopyConfigurable : public Configurable { + public: + CopyConfigurable() : prepared_(0), validated_(0) {} + Status PrepareOptions(const ConfigOptions& options) override { + prepared_++; + return Configurable::PrepareOptions(options); + } + Status ValidateOptions(const DBOptions& db_opts, + const ColumnFamilyOptions& cf_opts) const override { + validated_++; + return Configurable::ValidateOptions(db_opts, cf_opts); + } + int prepared_; + mutable int validated_; + }; + + CopyConfigurable c1; + ConfigOptions config_options; + Options options; + + ASSERT_OK(c1.PrepareOptions(config_options)); + ASSERT_OK(c1.ValidateOptions(options, options)); + ASSERT_EQ(c1.prepared_, 1); + ASSERT_EQ(c1.validated_, 1); + CopyConfigurable c2 = c1; + ASSERT_OK(c1.PrepareOptions(config_options)); + ASSERT_OK(c1.ValidateOptions(options, options)); + ASSERT_EQ(c2.prepared_, 1); + ASSERT_EQ(c2.validated_, 1); + ASSERT_EQ(c1.prepared_, 2); + ASSERT_EQ(c1.validated_, 2); +} + TEST_F(ConfigurableTest, MutableOptionsTest) { static std::unordered_map imm_option_info = { #ifndef ROCKSDB_LITE diff --git a/options/customizable_test.cc b/options/customizable_test.cc index cb1700f69..193bfe5f6 100644 --- a/options/customizable_test.cc +++ b/options/customizable_test.cc @@ -553,7 +553,6 @@ TEST_F(CustomizableTest, PrepareOptionsTest) { ConfigOptions prepared(config_options_); prepared.invoke_prepare_options = true; - ASSERT_FALSE(base->IsPrepared()); ASSERT_OK(base->ConfigureFromString( prepared, "unique=A_1; shared={id=B;string=s}; pointer.id=S")); SimpleOptions* simple = base->GetOptions(); @@ -561,10 +560,6 @@ TEST_F(CustomizableTest, PrepareOptionsTest) { ASSERT_NE(simple->cu, nullptr); ASSERT_NE(simple->cs, nullptr); ASSERT_NE(simple->cp, nullptr); - ASSERT_TRUE(base->IsPrepared()); - ASSERT_TRUE(simple->cu->IsPrepared()); - ASSERT_TRUE(simple->cs->IsPrepared()); - ASSERT_TRUE(simple->cp->IsPrepared()); delete simple->cp; base.reset(new SimpleConfigurable()); ASSERT_OK(base->ConfigureFromString( @@ -575,16 +570,8 @@ TEST_F(CustomizableTest, PrepareOptionsTest) { ASSERT_NE(simple->cu, nullptr); ASSERT_NE(simple->cs, nullptr); ASSERT_NE(simple->cp, nullptr); - ASSERT_FALSE(base->IsPrepared()); - ASSERT_FALSE(simple->cu->IsPrepared()); - ASSERT_FALSE(simple->cs->IsPrepared()); - ASSERT_FALSE(simple->cp->IsPrepared()); ASSERT_OK(base->PrepareOptions(config_options_)); - ASSERT_TRUE(base->IsPrepared()); - ASSERT_TRUE(simple->cu->IsPrepared()); - ASSERT_TRUE(simple->cs->IsPrepared()); - ASSERT_TRUE(simple->cp->IsPrepared()); delete simple->cp; base.reset(new SimpleConfigurable()); simple = base->GetOptions(); @@ -597,21 +584,16 @@ TEST_F(CustomizableTest, PrepareOptionsTest) { ASSERT_OK( base->ConfigureFromString(prepared, "unique={id=P; can_prepare=true}")); ASSERT_NE(simple->cu, nullptr); - ASSERT_TRUE(simple->cu->IsPrepared()); ASSERT_OK(base->ConfigureFromString(config_options_, "unique={id=P; can_prepare=true}")); ASSERT_NE(simple->cu, nullptr); - ASSERT_FALSE(simple->cu->IsPrepared()); ASSERT_OK(simple->cu->PrepareOptions(prepared)); - ASSERT_TRUE(simple->cu->IsPrepared()); ASSERT_OK(base->ConfigureFromString(config_options_, "unique={id=P; can_prepare=false}")); ASSERT_NE(simple->cu, nullptr); - ASSERT_FALSE(simple->cu->IsPrepared()); ASSERT_NOK(simple->cu->PrepareOptions(prepared)); - ASSERT_FALSE(simple->cu->IsPrepared()); } namespace { @@ -688,6 +670,42 @@ TEST_F(CustomizableTest, WrappedInnerTest) { ASSERT_EQ(wc2->CheckedCast(), ac.get()); } +TEST_F(CustomizableTest, CopyObjectTest) { + class CopyCustomizable : public Customizable { + public: + CopyCustomizable() : prepared_(0), validated_(0) {} + const char* Name() const override { return "CopyCustomizable"; } + + Status PrepareOptions(const ConfigOptions& options) override { + prepared_++; + return Customizable::PrepareOptions(options); + } + Status ValidateOptions(const DBOptions& db_opts, + const ColumnFamilyOptions& cf_opts) const override { + validated_++; + return Customizable::ValidateOptions(db_opts, cf_opts); + } + int prepared_; + mutable int validated_; + }; + + CopyCustomizable c1; + ConfigOptions config_options; + Options options; + + ASSERT_OK(c1.PrepareOptions(config_options)); + ASSERT_OK(c1.ValidateOptions(options, options)); + ASSERT_EQ(c1.prepared_, 1); + ASSERT_EQ(c1.validated_, 1); + CopyCustomizable c2 = c1; + ASSERT_OK(c1.PrepareOptions(config_options)); + ASSERT_OK(c1.ValidateOptions(options, options)); + ASSERT_EQ(c2.prepared_, 1); + ASSERT_EQ(c2.validated_, 1); + ASSERT_EQ(c1.prepared_, 2); + ASSERT_EQ(c1.validated_, 2); +} + TEST_F(CustomizableTest, TestStringDepth) { class ShallowCustomizable : public Customizable { public: @@ -983,7 +1001,6 @@ TEST_F(CustomizableTest, MutableOptionsTest) { std::string opt_str; ConfigOptions options = config_options_; - ASSERT_FALSE(mc.IsPrepared()); ASSERT_OK(mc.ConfigureOption(options, "mutable", "{id=B;}")); options.mutable_options_only = true; ASSERT_OK(mc.GetOptionString(options, &opt_str)); diff --git a/options/options_test.cc b/options/options_test.cc index 08496aa04..2a478a0e0 100644 --- a/options/options_test.cc +++ b/options/options_test.cc @@ -1482,13 +1482,11 @@ TEST_F(OptionsTest, MutableTableOptions) { bbtf.reset(NewBlockBasedTableFactory()); auto bbto = bbtf->GetOptions(); ASSERT_NE(bbto, nullptr); - ASSERT_FALSE(bbtf->IsPrepared()); ASSERT_OK(bbtf->ConfigureOption(config_options, "block_align", "true")); ASSERT_OK(bbtf->ConfigureOption(config_options, "block_size", "1024")); ASSERT_EQ(bbto->block_align, true); ASSERT_EQ(bbto->block_size, 1024); ASSERT_OK(bbtf->PrepareOptions(config_options)); - ASSERT_TRUE(bbtf->IsPrepared()); config_options.mutable_options_only = true; ASSERT_OK(bbtf->ConfigureOption(config_options, "block_size", "1024")); ASSERT_EQ(bbto->block_align, true);