From aebf1ab7fa5df7c91cbb8b07754ea1d1a67703bb Mon Sep 17 00:00:00 2001 From: Varun Ravichandran Date: Thu, 24 Feb 2022 04:03:03 +0000 Subject: [PATCH] SERVER-63630: Extend IDLServerParameterWithStorage to support cluster server parameters --- buildscripts/idl/idl/generator.py | 4 +- src/mongo/idl/SConscript | 1 + .../cluster_server_parameter_op_observer.cpp | 10 +- ...ster_server_parameter_op_observer_test.cpp | 275 ++++++++---------- ...luster_server_parameter_op_observer_test.h | 38 +++ src/mongo/idl/server_parameter.cpp | 33 ++- src/mongo/idl/server_parameter.h | 50 +++- src/mongo/idl/server_parameter_with_storage.h | 270 +++++++++++++---- .../server_parameter_with_storage_test.cpp | 137 ++++++++- .../server_parameter_with_storage_test.idl | 27 ++ 10 files changed, 605 insertions(+), 240 deletions(-) create mode 100644 src/mongo/idl/cluster_server_parameter_op_observer_test.h diff --git a/buildscripts/idl/idl/generator.py b/buildscripts/idl/idl/generator.py index 4adaabda915..c3034388dba 100644 --- a/buildscripts/idl/idl/generator.py +++ b/buildscripts/idl/idl/generator.py @@ -2341,9 +2341,9 @@ class _CppSourceFileWriter(_CppFileWriterBase): self._writer.write_line('ret->setRedact();') if param.default and not (param.cpp_vartype and param.cpp_varname): - # Only need to call setValue() if we haven't in-place initialized the declared var. + # Only need to call setDefault() if we haven't in-place initialized the declared var. self._writer.write_line( - 'uassertStatusOK(ret->setValue(%s));' % (_get_expression(param.default))) + 'uassertStatusOK(ret->setDefault(%s));' % (_get_expression(param.default))) self._writer.write_line('return ret;') diff --git a/src/mongo/idl/SConscript b/src/mongo/idl/SConscript index 8b5dc9f802e..d08bc0adecf 100644 --- a/src/mongo/idl/SConscript +++ b/src/mongo/idl/SConscript @@ -152,6 +152,7 @@ env.CppUnitTest( '$BUILD_DIR/mongo/util/cmdline_utils/cmdline_utils', '$BUILD_DIR/mongo/util/options_parser/options_parser', 'basic_types', + 'cluster_server_parameter', 'feature_flag', 'server_parameter', ], diff --git a/src/mongo/idl/cluster_server_parameter_op_observer.cpp b/src/mongo/idl/cluster_server_parameter_op_observer.cpp index d682c7fc799..19362dd7bb5 100644 --- a/src/mongo/idl/cluster_server_parameter_op_observer.cpp +++ b/src/mongo/idl/cluster_server_parameter_op_observer.cpp @@ -90,9 +90,7 @@ void updateParameter(BSONObj doc, StringData mode) { return; } - // TODO: SERVER-63630 Create a better interface for updating CSPs - uassertStatusOK(sp->set(BSON("" << doc).firstElement())); - sp->setClusterParameterTime(LogicalTime(cptElem.timestamp())); + uassertStatusOK(sp->set(doc)); } void clearParameter(ServerParameter* sp) { @@ -101,11 +99,7 @@ void clearParameter(ServerParameter* sp) { return; } - // TODO: SERVER-63630 Create a better interface for resetting CSPs - BSONObjBuilder bob; - bob.appendNull(""_sd); - uassertStatusOK(sp->set(bob.obj().firstElement())); - sp->setClusterParameterTime({}); + uassertStatusOK(sp->reset()); } void clearParameter(StringData id) { diff --git a/src/mongo/idl/cluster_server_parameter_op_observer_test.cpp b/src/mongo/idl/cluster_server_parameter_op_observer_test.cpp index e686186a132..9ccd2027622 100644 --- a/src/mongo/idl/cluster_server_parameter_op_observer_test.cpp +++ b/src/mongo/idl/cluster_server_parameter_op_observer_test.cpp @@ -29,7 +29,7 @@ #define MONGO_LOGV2_DEFAULT_COMPONENT ::mongo::logv2::LogComponent::kControl -#include "mongo/idl/cluster_server_parameter_op_observer.h" +#include "mongo/idl/cluster_server_parameter_op_observer_test.h" #include "mongo/platform/basic.h" @@ -41,6 +41,7 @@ #include "mongo/db/repl/storage_interface_mock.h" #include "mongo/db/service_context_d_test_fixture.h" #include "mongo/idl/cluster_server_parameter_gen.h" +#include "mongo/idl/cluster_server_parameter_op_observer.h" #include "mongo/idl/cluster_server_parameter_test_gen.h" #include "mongo/idl/server_parameter.h" #include "mongo/logv2/log.h" @@ -59,73 +60,8 @@ constexpr auto kCSPTest = "cspTest"_sd; const auto kNilCPT = LogicalTime::kUninitialized; constexpr auto kConfigDB = "config"_sd; -constexpr auto kSettingsColl = "clusterParameters"_sd; -const NamespaceString kSettingsNS(kConfigDB, kSettingsColl); - -class CSPTest : public ServerParameter { -public: - CSPTest() : ServerParameter(kCSPTest, ServerParameterType::kClusterWide) {} - - void append(OperationContext*, BSONObjBuilder& b, const std::string& name) final { - BSONObjBuilder bob(b.subobjStart(name)); - bob.append("intValue", _intValue); - bob.append("pbjValue", _strValue); - bob.doneFast(); - } - - Status validate(const BSONElement& newValueElement) const final { - auto swOptCSPTest = parseElement(newValueElement); - if (!swOptCSPTest.isOK()) { - return swOptCSPTest.getStatus(); - } - - return Status::OK(); - } - - Status set(const BSONElement& newValueElement) final { - auto swOptCSPTest = parseElement(newValueElement); - if (!swOptCSPTest.isOK()) { - return swOptCSPTest.getStatus(); - } - - auto optCSPTest = std::move(swOptCSPTest.getValue()); - if (!optCSPTest) { - _isInitialized = false; - _intValue = 0; - _strValue.clear(); - return Status::OK(); - } - - auto cspTest = std::move(optCSPTest.get()); - _isInitialized = true; - _intValue = cspTest.getIntValue(); - _strValue = cspTest.getStrValue().toString(); - return Status::OK(); - } - - Status setFromString(const std::string& newValue) final { - return {ErrorCodes::BadValue, "This API should never be called"}; - } - -private: - static StatusWith> parseElement( - const BSONElement& elem) try { - if (elem.type() == jstNULL) { - return boost::none; - } - if (elem.type() == Object) { - return boost::make_optional(ClusterServerParameterTest::parse({"cspTest"}, elem.Obj())); - } - return Status(ErrorCodes::BadValue, "cspTest value must be an object or null"); - } catch (const DBException& ex) { - return ex.toStatus().withContext("Failed parsing cspTest value"); - } - -public: - bool _isInitialized{false}; - int _intValue{0}; - std::string _strValue; -}; +constexpr auto kClusterParametersColl = "clusterParameters"_sd; +const NamespaceString kClusterParametersNS(kConfigDB, kClusterParametersColl); void upsert(BSONObj doc) { const auto kMajorityWriteConcern = BSON("writeConcern" << BSON("w" @@ -139,7 +75,7 @@ void upsert(BSONObj doc) { client.runCommand(kConfigDB.toString(), [&] { - write_ops::UpdateCommandRequest updateOp(kSettingsNS); + write_ops::UpdateCommandRequest updateOp(kClusterParametersNS); updateOp.setUpdates({[&] { write_ops::UpdateOpEntry entry; entry.setQ(BSON(ClusterServerParameter::k_idFieldName << kCSPTest)); @@ -171,7 +107,7 @@ void remove() { DBDirectClient(opCtx).runCommand( kConfigDB.toString(), [] { - write_ops::DeleteCommandRequest deleteOp(kSettingsNS); + write_ops::DeleteCommandRequest deleteOp(kClusterParametersNS); deleteOp.setDeletes({[] { write_ops::DeleteOpEntry entry; entry.setQ(BSON(ClusterServerParameter::k_idFieldName << kCSPTest)); @@ -191,7 +127,7 @@ void remove() { uassertStatusOK(response.toStatus()); } -BSONObj makeSettingsDoc(const LogicalTime& cpTime, int intValue, StringData strValue) { +BSONObj makeClusterParametersDoc(const LogicalTime& cpTime, int intValue, StringData strValue) { ClusterServerParameter csp; csp.set_id(kCSPTest); csp.setClusterParameterTime(cpTime); @@ -204,8 +140,14 @@ BSONObj makeSettingsDoc(const LogicalTime& cpTime, int intValue, StringData strV return cspt.toBSON(); } -MONGO_INITIALIZER(RegisterCSPTest)(InitializerContext*) { - registerServerParameter(new CSPTest()); +// TO-DO: Make this IDL-generated in SERVER-62253. +ClusterServerParameterTest cspTestStorage; +MONGO_SERVER_PARAMETER_REGISTER(RegisterCSPTest)(InitializerContext*) { + [[maybe_unused]] auto* csp = ([]() -> ServerParameter* { + auto* ret = makeIDLServerParameterWithStorage( + kCSPTest, cspTestStorage); + return ret; + })(); } class ClusterServerParameterOpObserverTest : public ServiceContextMongoDTest { @@ -297,39 +239,47 @@ public: // Asserts that the parameter state does not change for this action. template void assertIgnored(const NamespaceString& nss, F fn) { - auto* sp = ServerParameterSet::getClusterParameterSet()->get(kCSPTest); + auto* sp = ServerParameterSet::getClusterParameterSet() + ->get>(kCSPTest); ASSERT(sp != nullptr); const auto initialCPTime = sp->getClusterParameterTime(); - const auto initialIntValue = sp->_intValue; - const auto initialStrValue = sp->_strValue; + ClusterServerParameterTest initialCspTest = sp->getValue(); fn(nss); + ClusterServerParameterTest finalCspTest = sp->getValue(); + ASSERT_EQ(sp->getClusterParameterTime(), initialCPTime); - ASSERT_EQ(sp->_intValue, initialIntValue); - ASSERT_EQ(sp->_strValue, initialStrValue); + ASSERT_EQ(finalCspTest.getIntValue(), initialCspTest.getIntValue()); + ASSERT_EQ(finalCspTest.getStrValue(), initialCspTest.getStrValue()); } static constexpr auto kInitialIntValue = 123; + static constexpr auto kDefaultIntValue = 42; static constexpr auto kInitialStrValue = "initialState"_sd; + static constexpr auto kDefaultStrValue = ""_sd; BSONObj initializeState() { Timestamp now(time(nullptr)); - const auto doc = makeSettingsDoc(LogicalTime(now), 123, "initialState"); + const auto doc = + makeClusterParametersDoc(LogicalTime(now), kInitialIntValue, kInitialStrValue); upsert(doc); - doInserts(kSettingsNS, {doc}); + doInserts(kClusterParametersNS, {doc}); - auto* sp = ServerParameterSet::getClusterParameterSet()->get(kCSPTest); + auto* sp = ServerParameterSet::getClusterParameterSet() + ->get>(kCSPTest); ASSERT(sp != nullptr); - ASSERT_TRUE(sp->_isInitialized); - ASSERT_EQ(sp->_intValue, kInitialIntValue); - ASSERT_EQ(sp->_strValue, kInitialStrValue); + ClusterServerParameterTest cspTest = sp->getValue(); + ASSERT_EQ(cspTest.getIntValue(), kInitialIntValue); + ASSERT_EQ(cspTest.getStrValue(), kInitialStrValue); return doc; } - // Asserts that a given action is ignore anywhere outside of config.settings. + // Asserts that a given action is ignore anywhere outside of config.clusterParameters. template void assertIgnoredOtherNamespaces(F fn) { for (const auto& nss : kIgnoredNamespaces) { @@ -337,11 +287,11 @@ public: } } - // Asserts that a given action is ignored anywhere, even on the config.settings NS. + // Asserts that a given action is ignored anywhere, even on the config.clusterParameters NS. template void assertIgnoredAlways(F fn) { assertIgnoredOtherNamespaces(fn); - assertIgnored(kSettingsNS, fn); + assertIgnored(kClusterParametersNS, fn); } private: @@ -357,21 +307,25 @@ protected: }; TEST_F(ClusterServerParameterOpObserverTest, OnInsertRecord) { - auto* sp = ServerParameterSet::getClusterParameterSet()->get(kCSPTest); + auto* sp = ServerParameterSet::getClusterParameterSet() + ->get>(kCSPTest); ASSERT(sp != nullptr); // Single record insert. const auto initialLogicalTime = sp->getClusterParameterTime(); const auto singleLogicalTime = initialLogicalTime.addTicks(1); - const auto singleIntValue = sp->_intValue + 1; + const auto singleIntValue = sp->getValue().getIntValue() + 1; const auto singleStrValue = "OnInsertRecord.single"; ASSERT_LT(initialLogicalTime, singleLogicalTime); - doInserts(kSettingsNS, {makeSettingsDoc(singleLogicalTime, singleIntValue, singleStrValue)}); + doInserts(kClusterParametersNS, + {makeClusterParametersDoc(singleLogicalTime, singleIntValue, singleStrValue)}); + ClusterServerParameterTest cspTest = sp->getValue(); ASSERT_EQ(sp->getClusterParameterTime(), singleLogicalTime); - ASSERT_EQ(sp->_intValue, singleIntValue); - ASSERT_EQ(sp->_strValue, singleStrValue); + ASSERT_EQ(cspTest.getIntValue(), singleIntValue); + ASSERT_EQ(cspTest.getStrValue(), singleStrValue); // Multi-record insert. const auto multiLogicalTime = singleLogicalTime.addTicks(1); @@ -379,26 +333,27 @@ TEST_F(ClusterServerParameterOpObserverTest, OnInsertRecord) { const auto multiStrValue = "OnInsertRecord.multi"; ASSERT_LT(singleLogicalTime, multiLogicalTime); - doInserts(kSettingsNS, + doInserts(kClusterParametersNS, { BSON(ClusterServerParameter::k_idFieldName << "ignored"), - makeSettingsDoc(multiLogicalTime, multiIntValue, multiStrValue), + makeClusterParametersDoc(multiLogicalTime, multiIntValue, multiStrValue), BSON(ClusterServerParameter::k_idFieldName << "alsoIgnored"), }); + cspTest = sp->getValue(); ASSERT_EQ(sp->getClusterParameterTime(), multiLogicalTime); - ASSERT_EQ(sp->_intValue, multiIntValue); - ASSERT_EQ(sp->_strValue, multiStrValue); + ASSERT_EQ(cspTest.getIntValue(), multiIntValue); + ASSERT_EQ(cspTest.getStrValue(), multiStrValue); // Insert plausible records to namespaces we don't care about. assertIgnoredOtherNamespaces([this](const auto& nss) { - doInserts(nss, {makeSettingsDoc(LogicalTime(), 42, "yellow")}); + doInserts(nss, {makeClusterParametersDoc(LogicalTime(), 42, "yellow")}); }); // Plausible on other NS, multi-insert. assertIgnoredOtherNamespaces([this](const auto& nss) { - auto d0 = makeSettingsDoc(LogicalTime(), 123, "red"); - auto d1 = makeSettingsDoc(LogicalTime(), 234, "green"); - auto d2 = makeSettingsDoc(LogicalTime(), 345, "blue"); + auto d0 = makeClusterParametersDoc(LogicalTime(), 123, "red"); + auto d1 = makeClusterParametersDoc(LogicalTime(), 234, "green"); + auto d2 = makeClusterParametersDoc(LogicalTime(), 345, "blue"); doInserts(nss, {d0, d1, d2}); }); @@ -420,25 +375,30 @@ TEST_F(ClusterServerParameterOpObserverTest, OnInsertRecord) { TEST_F(ClusterServerParameterOpObserverTest, OnUpdateRecord) { initializeState(); - auto* sp = ServerParameterSet::getClusterParameterSet()->get(kCSPTest); + auto* sp = ServerParameterSet::getClusterParameterSet() + ->get>(kCSPTest); ASSERT(sp != nullptr); // Single record update. const auto initialLogicalTime = sp->getClusterParameterTime(); const auto singleLogicalTime = initialLogicalTime.addTicks(1); - const auto singleIntValue = sp->_intValue + 1; + const auto singleIntValue = sp->getValue().getIntValue() + 1; const auto singleStrValue = "OnUpdateRecord.single"; ASSERT_LT(initialLogicalTime, singleLogicalTime); - doUpdate(kSettingsNS, makeSettingsDoc(singleLogicalTime, singleIntValue, singleStrValue)); + doUpdate(kClusterParametersNS, + makeClusterParametersDoc(singleLogicalTime, singleIntValue, singleStrValue)); + ClusterServerParameterTest cspTest = sp->getValue(); ASSERT_EQ(sp->getClusterParameterTime(), singleLogicalTime); - ASSERT_EQ(sp->_intValue, singleIntValue); - ASSERT_EQ(sp->_strValue, singleStrValue); + ASSERT_EQ(cspTest.getIntValue(), singleIntValue); + ASSERT_EQ(cspTest.getStrValue(), singleStrValue); // Plausible doc in wrong namespace. - assertIgnoredOtherNamespaces( - [this](const auto& nss) { doUpdate(nss, makeSettingsDoc(LogicalTime(), 123, "ignored")); }); + assertIgnoredOtherNamespaces([this](const auto& nss) { + doUpdate(nss, makeClusterParametersDoc(LogicalTime(), 123, "ignored")); + }); // Non cluster parameter doc. assertIgnoredAlways([this](const auto& nss) { @@ -447,7 +407,9 @@ TEST_F(ClusterServerParameterOpObserverTest, OnUpdateRecord) { } TEST_F(ClusterServerParameterOpObserverTest, onDeleteRecord) { - auto* sp = ServerParameterSet::getClusterParameterSet()->get(kCSPTest); + auto* sp = ServerParameterSet::getClusterParameterSet() + ->get>(kCSPTest); ASSERT(sp != nullptr); const auto initialDoc = initializeState(); @@ -456,24 +418,24 @@ TEST_F(ClusterServerParameterOpObserverTest, onDeleteRecord) { assertIgnoredOtherNamespaces( [this, initialDoc](const auto& nss) { doDelete(nss, initialDoc); }); - // Ignore deletes where the _id is known to not be 'audit'. + // Ignore deletes where the _id does not correspond to a known cluster server parameter. assertIgnoredAlways([this](const auto& nss) { doDelete(nss, BSON(ClusterServerParameter::k_idFieldName << "ignored")); }); - // Reset configuration when we claim to have deleted the doc. - doDelete(kSettingsNS, initialDoc); - ASSERT_FALSE(sp->_isInitialized); - ASSERT_EQ(sp->_intValue, 0); - ASSERT_EQ(sp->_strValue, ""); + // Reset configuration to defaults when we claim to have deleted the doc. + doDelete(kClusterParametersNS, initialDoc); + ClusterServerParameterTest cspTest = sp->getValue(); + ASSERT_EQ(cspTest.getIntValue(), kDefaultIntValue); + ASSERT_EQ(cspTest.getStrValue(), kDefaultStrValue); // Restore configured state, and delete without including deleteDoc reference. initializeState(); - doDelete(kSettingsNS, initialDoc, false); - ASSERT_FALSE(sp->_isInitialized); - ASSERT_EQ(sp->_intValue, 0); - ASSERT_EQ(sp->_strValue, ""); + doDelete(kClusterParametersNS, initialDoc, false); + cspTest = sp->getValue(); + ASSERT_EQ(cspTest.getIntValue(), kDefaultIntValue); + ASSERT_EQ(cspTest.getStrValue(), kDefaultStrValue); } TEST_F(ClusterServerParameterOpObserverTest, onDropDatabase) { @@ -490,12 +452,14 @@ TEST_F(ClusterServerParameterOpObserverTest, onDropDatabase) { // Actually drop the config DB. doDropDatabase(kConfigDB); - auto* sp = ServerParameterSet::getClusterParameterSet()->get(kCSPTest); + auto* sp = ServerParameterSet::getClusterParameterSet() + ->get>(kCSPTest); ASSERT(sp != nullptr); - ASSERT_FALSE(sp->_isInitialized); - ASSERT_EQ(sp->_intValue, 0); - ASSERT_EQ(sp->_strValue, ""); + ClusterServerParameterTest cspTest = sp->getValue(); + ASSERT_EQ(cspTest.getIntValue(), kDefaultIntValue); + ASSERT_EQ(cspTest.getStrValue(), kDefaultStrValue); } TEST_F(ClusterServerParameterOpObserverTest, onRenameCollection) { @@ -506,23 +470,25 @@ TEST_F(ClusterServerParameterOpObserverTest, onRenameCollection) { assertIgnoredOtherNamespaces([&](const auto& nss) { doRenameCollection(nss, kTestFoo); }); assertIgnoredOtherNamespaces([&](const auto& nss) { doRenameCollection(kTestFoo, nss); }); - auto* sp = ServerParameterSet::getClusterParameterSet()->get(kCSPTest); + auto* sp = ServerParameterSet::getClusterParameterSet() + ->get>(kCSPTest); ASSERT(sp != nullptr); // These renames "work" despite not mutating durable state // since the rename away doesn't require a rescan. - // Rename away (and reset to initial) - doRenameCollection(kSettingsNS, kTestFoo); - ASSERT_FALSE(sp->_isInitialized); - ASSERT_EQ(sp->_intValue, 0); - ASSERT_EQ(sp->_strValue, ""); + // Rename away (and reset to default) + doRenameCollection(kClusterParametersNS, kTestFoo); + ClusterServerParameterTest cspTest = sp->getValue(); + ASSERT_EQ(cspTest.getIntValue(), kDefaultIntValue); + ASSERT_EQ(cspTest.getStrValue(), kDefaultStrValue); // Rename in (and restore to initialized state) - doRenameCollection(kTestFoo, kSettingsNS); - ASSERT_TRUE(sp->_isInitialized); - ASSERT_EQ(sp->_intValue, kInitialIntValue); - ASSERT_EQ(sp->_strValue, kInitialStrValue); + doRenameCollection(kTestFoo, kClusterParametersNS); + cspTest = sp->getValue(); + ASSERT_EQ(cspTest.getIntValue(), kInitialIntValue); + ASSERT_EQ(cspTest.getStrValue(), kInitialStrValue); } TEST_F(ClusterServerParameterOpObserverTest, onImportCollection) { @@ -532,16 +498,19 @@ TEST_F(ClusterServerParameterOpObserverTest, onImportCollection) { // Import ignorable collections. assertIgnoredOtherNamespaces([&](const auto& nss) { doImportCollection(nss); }); - auto* sp = ServerParameterSet::getClusterParameterSet()->get(kCSPTest); + auto* sp = ServerParameterSet::getClusterParameterSet() + ->get>(kCSPTest); ASSERT(sp != nullptr); // Import the collection (rescan). - auto doc = makeSettingsDoc(LogicalTime(Timestamp(time(nullptr))), 333, "onImportCollection"); + auto doc = + makeClusterParametersDoc(LogicalTime(Timestamp(time(nullptr))), 333, "onImportCollection"); upsert(doc); - doImportCollection(kSettingsNS); - ASSERT_TRUE(sp->_isInitialized); - ASSERT_EQ(sp->_intValue, 333); - ASSERT_EQ(sp->_strValue, "onImportCollection"); + doImportCollection(kClusterParametersNS); + ClusterServerParameterTest cspTest = sp->getValue(); + ASSERT_EQ(cspTest.getIntValue(), 333); + ASSERT_EQ(cspTest.getStrValue(), "onImportCollection"); } TEST_F(ClusterServerParameterOpObserverTest, onReplicationRollback) { @@ -551,29 +520,33 @@ TEST_F(ClusterServerParameterOpObserverTest, onReplicationRollback) { // Import ignorable collections. assertIgnoredOtherNamespaces([&](const auto& nss) { doImportCollection(nss); }); - auto* sp = ServerParameterSet::getClusterParameterSet()->get(kCSPTest); + auto* sp = ServerParameterSet::getClusterParameterSet() + ->get>(kCSPTest); ASSERT(sp != nullptr); // Trigger rollback of ignorable namespaces. doReplicationRollback(kIgnoredNamespaces); - ASSERT_TRUE(sp->_isInitialized); - ASSERT_EQ(sp->_intValue, kInitialIntValue); - ASSERT_EQ(sp->_strValue, kInitialStrValue); + + ClusterServerParameterTest cspTest = sp->getValue(); + ASSERT_EQ(cspTest.getIntValue(), kInitialIntValue); + ASSERT_EQ(cspTest.getStrValue(), kInitialStrValue); // Trigger rollback of relevant namespace. remove(); - doReplicationRollback({kSettingsNS}); - ASSERT_FALSE(sp->_isInitialized); - ASSERT_EQ(sp->_intValue, 0); - ASSERT_EQ(sp->_strValue, ""); + doReplicationRollback({kClusterParametersNS}); + cspTest = sp->getValue(); + ASSERT_EQ(cspTest.getIntValue(), kDefaultIntValue); + ASSERT_EQ(cspTest.getStrValue(), kDefaultStrValue); // Rollback the rollback. - auto doc = makeSettingsDoc(LogicalTime(Timestamp(time(nullptr))), 444, "onReplicationRollback"); + auto doc = makeClusterParametersDoc( + LogicalTime(Timestamp(time(nullptr))), 444, "onReplicationRollback"); upsert(doc); - doReplicationRollback({kSettingsNS}); - ASSERT_TRUE(sp->_isInitialized); - ASSERT_EQ(sp->_intValue, 444); - ASSERT_EQ(sp->_strValue, "onReplicationRollback"); + cspTest = sp->getValue(); + doReplicationRollback({kClusterParametersNS}); + ASSERT_EQ(cspTest.getIntValue(), kDefaultIntValue); + ASSERT_EQ(cspTest.getStrValue(), kDefaultStrValue); } } // namespace diff --git a/src/mongo/idl/cluster_server_parameter_op_observer_test.h b/src/mongo/idl/cluster_server_parameter_op_observer_test.h new file mode 100644 index 00000000000..cdc34ca328e --- /dev/null +++ b/src/mongo/idl/cluster_server_parameter_op_observer_test.h @@ -0,0 +1,38 @@ +/** + * Copyright (C) 2022-present MongoDB, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * . + * + * As a special exception, the copyright holders give permission to link the + * code of portions of this program with the OpenSSL library under certain + * conditions as described in each individual source file and distribute + * linked combinations including the program with the OpenSSL library. You + * must comply with the Server Side Public License in all respects for + * all of the code used other than as permitted herein. If you modify file(s) + * with this exception, you may extend this exception to your version of the + * file(s), but you are not obligated to do so. If you do not wish to do so, + * delete this exception statement from your version. If you delete this + * exception statement from all source files in the program, then also delete + * it in the license file. + */ + +#pragma once + +#include "mongo/platform/basic.h" + +#include "mongo/idl/cluster_server_parameter_test_gen.h" + +namespace mongo { +extern ClusterServerParameterTest cspTestStorage; +} diff --git a/src/mongo/idl/server_parameter.cpp b/src/mongo/idl/server_parameter.cpp index 2d22352c570..8ccad799b36 100644 --- a/src/mongo/idl/server_parameter.cpp +++ b/src/mongo/idl/server_parameter.cpp @@ -48,13 +48,6 @@ MONGO_INITIALIZER_GROUP(EndServerParameterRegistration, ServerParameter::ServerParameter(StringData name, ServerParameterType spt) : _name{name}, _type(spt) {} -void ServerParameter::setClusterParameterTime(const LogicalTime& clusterParameterTime) { - uassert(6225101, - "Invalid call to setClusterParameterTime on locally scoped server parameter", - isClusterWide()); - _clusterParameterTime = clusterParameterTime; -} - Status ServerParameter::set(const BSONElement& newValueElement) { auto swValue = _coerceToString(newValueElement); if (!swValue.isOK()) @@ -138,9 +131,7 @@ void IDLServerParameterDeprecatedAlias::append(OperationContext* opCtx, BSONObjBuilder& b, const std::string& fieldName) { std::call_once(_warnOnce, [&] { - LOGV2_WARNING(23781, - "Use of deprecated server parameter '{deprecatedName}', " - "please use '{canonicalName}' instead", + LOGV2_WARNING(636300, "Use of deprecated server parameter name", "deprecatedName"_attr = name(), "canonicalName"_attr = _sp->name()); @@ -148,11 +139,19 @@ void IDLServerParameterDeprecatedAlias::append(OperationContext* opCtx, _sp->append(opCtx, b, fieldName); } +Status IDLServerParameterDeprecatedAlias::reset() { + std::call_once(_warnOnce, [&] { + LOGV2_WARNING(636301, + "Use of deprecated server parameter name", + "deprecatedName"_attr = name(), + "canonicalName"_attr = _sp->name()); + }); + return _sp->reset(); +} + Status IDLServerParameterDeprecatedAlias::set(const BSONElement& newValueElement) { std::call_once(_warnOnce, [&] { - LOGV2_WARNING(23782, - "Use of deprecated server parameter '{deprecatedName}', " - "please use '{canonicalName}' instead", + LOGV2_WARNING(636302, "Use of deprecated server parameter name", "deprecatedName"_attr = name(), "canonicalName"_attr = _sp->name()); @@ -162,9 +161,7 @@ Status IDLServerParameterDeprecatedAlias::set(const BSONElement& newValueElement Status IDLServerParameterDeprecatedAlias::setFromString(const std::string& str) { std::call_once(_warnOnce, [&] { - LOGV2_WARNING(23783, - "Use of deprecated server parameter '{deprecatedName}', " - "please use '{canonicalName}' instead", + LOGV2_WARNING(636303, "Use of deprecated server parameter name", "deprecatedName"_attr = name(), "canonicalName"_attr = _sp->name()); @@ -192,6 +189,10 @@ public: return setFromString(""); } + Status reset() final { + return setFromString(""); + } + private: // Retain the original pointer to avoid ASAN complaining. ServerParameter* _sp; diff --git a/src/mongo/idl/server_parameter.h b/src/mongo/idl/server_parameter.h index 638e32ec372..3fea692193e 100644 --- a/src/mongo/idl/server_parameter.h +++ b/src/mongo/idl/server_parameter.h @@ -130,12 +130,6 @@ public: return (_type != ServerParameterType::kClusterWide); } - LogicalTime getClusterParameterTime() const { - return _clusterParameterTime; - } - - void setClusterParameterTime(const LogicalTime& clusterParameterTime); - virtual void append(OperationContext* opCtx, BSONObjBuilder& b, const std::string& name) = 0; virtual void appendSupportingRoundtrip(OperationContext* opCtx, @@ -148,12 +142,54 @@ public: return Status::OK(); } + Status validate(const BSONObj& newValueObj) const { + return validate(BSON("" << newValueObj).firstElement()); + } + // This base implementation calls `setFromString(coerceToString(newValueElement))`. // Derived classes may customize the behavior by specifying `override_set` in IDL. virtual Status set(const BSONElement& newValueElement); + /** + * This method will reset the server parameter's value back to its default. This is currently + * only used by cluster server parameters, but can work with node-only + * IDLServerParameterWithStorage. + * - IDLServerParameterWithStorage automatically initializes a copy of the storage variable's + * initial value when it is constructed, which is treated as the default value. When the storage + * variable is not declared by the IDL generator, it will use the setDefault() method to + * adjust both the current value and the default value. + * - Specialized server parameters can opt into providing resettability by implementing this + * method. If it is called without being implemented, it will return an error via the inherited + * method below. + */ + virtual Status reset() { + return Status{ErrorCodes::OperationFailed, + str::stream() + << "Parameter reset not implemented for server parameter: " << name()}; + } + + /** + * Overload of set() that accepts BSONObjs instead of BSONElements. This is currently only used + * for cluster server parameters but can be used for node-only server parameters. + */ + Status set(const BSONObj& newValueObj) { + return set(BSON("" << newValueObj).firstElement()); + } + virtual Status setFromString(const std::string& str) = 0; + /** + * Simply returns the uninitialized/default-constructed LogicalTime by default. + * IDLServerParameterWithStorage overrides this to atomically return the clusterParameterTime + * stored in the base ClusterServerParameter class that all non-specialized cluster server + * parameter storage types must be chained from. Specialized server parameters are expected to + * implement a mechanism for atomically setting the clusterParameterTime in the set() method and + * retrieving it via this method. + */ + virtual const LogicalTime getClusterParameterTime() const { + return LogicalTime(); + } + bool isTestOnly() const { return _testOnly; } @@ -176,7 +212,6 @@ protected: private: std::string _name; - LogicalTime _clusterParameterTime; ServerParameterType _type; bool _testOnly = false; bool _redact = false; @@ -255,6 +290,7 @@ public: IDLServerParameterDeprecatedAlias(StringData name, ServerParameter* sp); void append(OperationContext* opCtx, BSONObjBuilder& b, const std::string& name) final; + Status reset() final; Status set(const BSONElement& newValueElement) final; Status setFromString(const std::string& str) final; diff --git a/src/mongo/idl/server_parameter_with_storage.h b/src/mongo/idl/server_parameter_with_storage.h index 8f6ddac26d2..e116dee1078 100644 --- a/src/mongo/idl/server_parameter_with_storage.h +++ b/src/mongo/idl/server_parameter_with_storage.h @@ -43,6 +43,7 @@ #include "mongo/base/string_data.h" #include "mongo/bson/bsonelement.h" #include "mongo/bson/bsonobjbuilder.h" +#include "mongo/idl/idl_parser.h" #include "mongo/idl/server_parameter.h" #include "mongo/platform/atomic_proxy.h" #include "mongo/platform/atomic_word.h" @@ -126,56 +127,142 @@ struct storage_wrapper; template struct storage_wrapper> { - static constexpr bool thread_safe = true; using type = U; - static void store(AtomicWord& storage, const U& value) { - storage.store(value); + storage_wrapper(AtomicWord& storage) : _storage(storage), _defaultValue(storage.load()) {} + + void store(const U& value) { + _storage.store(value); } - static U load(const AtomicWord& storage) { - return storage.load(); + + U load() const { + return _storage.load(); } + + void reset() { + _storage.store(_defaultValue); + } + + // Not thread-safe, will only be called once at most per ServerParameter in its initialization + // block. + void setDefault(const U& value) { + _defaultValue = value; + } + +private: + AtomicWord& _storage; + + // Copy of original value to be read from during resets. + U _defaultValue; }; // Covers AtomicDouble template struct storage_wrapper> { - static constexpr bool thread_safe = true; using type = U; - static void store(AtomicProxy& storage, const U& value) { - storage.store(value); + storage_wrapper(AtomicProxy& storage) + : _storage(storage), _defaultValue(storage.load()) {} + + void store(const U& value) { + _storage.store(value); } - static U load(const AtomicProxy& storage) { - return storage.load(); + + U load() const { + return _storage.load(); } + + void reset() { + _storage.store(_defaultValue); + } + + // Not thread-safe, will only be called once at most per ServerParameter in its initialization + // block. + void setDefault(const U& value) { + _defaultValue = value; + } + +private: + AtomicProxy& _storage; + + // Copy of original value to be read from during resets. + U _defaultValue; }; template struct storage_wrapper> { - static constexpr bool thread_safe = true; using type = U; - static void store(synchronized_value& storage, const U& value) { - *storage = value; + storage_wrapper(synchronized_value& storage) : _storage(storage), _defaultValue(*storage) {} + + void store(const U& value) { + *_storage = value; } - static U load(const synchronized_value& storage) { - return *storage; + + U load() const { + return *_storage; } + + void reset() { + *_storage = _defaultValue; + } + + // Not thread-safe, will only be called once at most per ServerParameter in its initialization + // block. + void setDefault(const U& value) { + _defaultValue = value; + } + +private: + synchronized_value& _storage; + + // Copy of original value to be read from during resets. + U _defaultValue; }; -// All other types +// All other types will use a mutex to synchronize in a threadsafe manner. template struct storage_wrapper { - static constexpr bool thread_safe = false; using type = U; - static void store(U& storage, const U& value) { - storage = value; + storage_wrapper(U& storage) : _storage(storage), _defaultValue(storage) {} + + void store(const U& value) { + stdx::lock_guard lg(_storageMutex); + _storage = value; } - static U load(const U& storage) { - return storage; + + U load() const { + stdx::lock_guard lg(_storageMutex); + return _storage; } + + void reset() { + stdx::lock_guard lg(_storageMutex); + _storage = _defaultValue; + } + + // Not thread-safe, will only be called once at most per ServerParameter in its initialization + // block. + void setDefault(const U& value) { + _defaultValue = value; + } + +private: + mutable Mutex _storageMutex = MONGO_MAKE_LATCH("IDLServerParameterWithStorage:_storageMutex"); + U& _storage; + + // Copy of original value to be read from during resets. + U _defaultValue; }; } // namespace idl_server_parameter_detail +/** + * Used to check if the parameter type has the getClusterServerParameter method, which proves + * that ClusterServerParameter is inline chained to it. + */ +template +using HasClusterServerParameter = decltype(std::declval().getClusterServerParameter()); +template +constexpr bool hasClusterServerParameter = stdx::is_detected_v; + /** * Specialization of ServerParameter used by IDL generator. */ @@ -186,14 +273,16 @@ private: using SW = idl_server_parameter_detail::storage_wrapper; public: - static constexpr bool thread_safe = SW::thread_safe; using element_type = typename SW::type; IDLServerParameterWithStorage(StringData name, T& storage) : ServerParameter(name, paramType), _storage(storage) { - constexpr bool notRuntime = - (paramType == SPT::kStartupOnly) || (paramType == SPT::kReadOnly); - static_assert(thread_safe || notRuntime, "Runtime server parameters must be thread safe"); + constexpr bool notClusterParameter = (paramType != SPT::kClusterWide); + // Compile-time assertion to ensure that IDL-defined in-memory storage for CSPs are + // chained to the ClusterServerParameter base type. + static_assert( + notClusterParameter || hasClusterServerParameter, + "Cluster server parameter storage must be chained from ClusterServerParameter"); } Status validateValue(const element_type& newValue) const { @@ -214,7 +303,7 @@ public: return status; } - SW::store(_storage, newValue); + _storage.store(newValue); if (_onUpdate) { return _onUpdate(newValue); @@ -227,58 +316,83 @@ public: * Convenience wrapper for fetching value from storage. */ element_type getValue() const { - return SW::load(_storage); + return _storage.load(); + } + + /** + * Allows the default value stored in the underlying storage_wrapper to be changed exactly once + * after initialization. This should only be called by the IDL generator when creating + * MONGO_SERVER_PARAMETER_REGISTER blocks for parameters that do not specify a `cpp_vartype` + * (the storage variable is not defined by the IDL generator). + */ + Status setDefault(const element_type& newDefaultValue) { + Status status = Status::OK(); + std::call_once(_setDefaultOnce, [&] { + // Update the default value. + _storage.setDefault(newDefaultValue); + + // Update the actual storage, performing validation and any post-update functions as + // necessary. + status = reset(); + }); + return status; } /** * Encode the setting into BSON object. * - * Typically invoked by {getParameter:...} to produce a dictionary + * Typically invoked by {getParameter:...} or {getClusterParameter:...} to produce a dictionary * of SCP settings. */ - void append(OperationContext* opCtx, BSONObjBuilder& b, const std::string& name) final { + void append(OperationContext* opCtx, + BSONObjBuilder& b, + const std::string& name) override final { if (isRedact()) { b.append(name, "###"); + } else if constexpr (paramType == SPT::kClusterWide) { + getValue().serialize(&b); } else { b.append(name, getValue()); } } - Status validate(const BSONElement& newValueElement) const final { + StatusWith parseElement(const BSONElement& newValueElement) const { element_type newValue; - - if (auto status = newValueElement.tryCoerce(&newValue); !status.isOK()) { - return {status.code(), - str::stream() << "Failed validating " << name() << ": " << status.reason()}; + if constexpr (paramType == SPT::kClusterWide) { + try { + BSONObj cspObj = newValueElement.Obj(); + newValue = element_type::parse({"ClusterServerParameter"}, cspObj); + } catch (const DBException& ex) { + return ex.toStatus().withContext( + str::stream() << "Failed parsing ClusterServerParameter '" << name() << "'"); + } + } else { + if (auto status = newValueElement.tryCoerce(&newValue); !status.isOK()) { + return {status.code(), + str::stream() << "Failed validating " << name() << ": " << status.reason()}; + } } - return validateValue(newValue); + return newValue; + } + + Status validate(const BSONElement& newValueElement) const override final { + StatusWith swNewValue = parseElement(newValueElement); + if (!swNewValue.isOK()) { + return swNewValue.getStatus(); + } + + return validateValue(swNewValue.getValue()); } /** * Update the underlying value using a BSONElement * * Allows setting non-basic values (e.g. vector) - * via the {setParameter: ...} call. + * via the {setParameter: ...} call or {setClusterParameter: ...} call. */ - Status set(const BSONElement& newValueElement) final { - element_type newValue; - - if (auto status = newValueElement.tryCoerce(&newValue); !status.isOK()) { - return {status.code(), - str::stream() << "Failed setting " << name() << ": " << status.reason()}; - } - - return setValue(newValue); - } - - /** - * Update the underlying value from a string. - * - * Typically invoked from commandline --setParameter usage. - */ - Status setFromString(const std::string& str) final { - auto swNewValue = idl_server_parameter_detail::coerceFromString(str); + Status set(const BSONElement& newValueElement) override final { + StatusWith swNewValue = parseElement(newValueElement); if (!swNewValue.isOK()) { return swNewValue.getStatus(); } @@ -286,6 +400,51 @@ public: return setValue(swNewValue.getValue()); } + /** + * Resets the current storage value in storage_wrapper with the default value. + */ + Status reset() override final { + _storage.reset(); + if (_onUpdate) { + return _onUpdate(_storage.load()); + } + + return Status::OK(); + } + + /** + * Update the underlying value from a string. + * + * Typically invoked from commandline --setParameter usage. Prohibited for cluster server + * parameters. + */ + Status setFromString(const std::string& str) override final { + if constexpr (paramType == SPT::kClusterWide) { + return {ErrorCodes::BadValue, + "Unable to set a cluster-wide server parameter from the command line or config " + "file. See command 'setClusterParameter'"}; + } else { + auto swNewValue = idl_server_parameter_detail::coerceFromString(str); + if (!swNewValue.isOK()) { + return swNewValue.getStatus(); + } + + return setValue(swNewValue.getValue()); + } + } + + /** + * Retrieves the cluster parameter time from the chained ClusterServerParameter struct in + * storage. All other server parameters simply return the uninitialized LogicalTime. + */ + const LogicalTime getClusterParameterTime() const override final { + if constexpr (hasClusterServerParameter) { + return getValue().getClusterParameterTime(); + } else { + return LogicalTime(); + } + } + /** * Called *after* updating the underlying storage to its new value. */ @@ -323,10 +482,11 @@ public: } private: - T& _storage; + SW _storage; std::vector> _validators; std::function _onUpdate; + std::once_flag _setDefaultOnce; }; // MSVC has trouble resolving T=decltype(param) through the above class template. diff --git a/src/mongo/idl/server_parameter_with_storage_test.cpp b/src/mongo/idl/server_parameter_with_storage_test.cpp index d0d11097a90..c149c6519da 100644 --- a/src/mongo/idl/server_parameter_with_storage_test.cpp +++ b/src/mongo/idl/server_parameter_with_storage_test.cpp @@ -38,6 +38,8 @@ namespace mongo { AtomicWord test::gStdIntPreallocated; AtomicWord test::gStdIntPreallocatedUpdateCount; +test::ChangeStreamOptionsClusterParam clusterParamStorage; + namespace { using SPT = ServerParameterType; @@ -58,7 +60,7 @@ template void doStorageTest(StringData name, const std::vector& valid, const std::vector& invalid) { - T val; + T val = T(); IDLServerParameterWithStorage param(name, val); using element_type = typename decltype(param)::element_type; @@ -192,6 +194,10 @@ TEST(IDLServerParameterWithStorage, stdIntDeclared) { ASSERT_NOT_OK(stdIntDeclared->setFromString("1000")); ASSERT_NOT_OK(stdIntDeclared->setFromString("-1")); ASSERT_NOT_OK(stdIntDeclared->setFromString("alpha")); + + // Reset to default. + ASSERT_OK(stdIntDeclared->reset()); + ASSERT_EQ(test::gStdIntDeclared.load(), 42); } TEST(IDLServerParameterWithStorage, stdIntPreallocated) { @@ -209,6 +215,11 @@ TEST(IDLServerParameterWithStorage, stdIntPreallocated) { ASSERT_NOT_OK(stdIntPreallocated->setFromString("-1")); ASSERT_NOT_OK(stdIntPreallocated->setFromString("alpha")); ASSERT_EQ(test::gStdIntPreallocatedUpdateCount.load(), 2); + + // Reset to default. + ASSERT_OK(stdIntPreallocated->reset()); + ASSERT_EQ(test::gStdIntPreallocated.load(), 11); + ASSERT_EQ(test::gStdIntPreallocatedUpdateCount.load(), 3); } TEST(IDLServerParameterWithStorage, startupString) { @@ -217,6 +228,10 @@ TEST(IDLServerParameterWithStorage, startupString) { ASSERT_EQ(sp->allowedToChangeAtRuntime(), false); ASSERT_OK(sp->setFromString("New Value")); ASSERT_EQ(test::gStartupString, "New Value"); + + // Reset to default. + ASSERT_OK(sp->reset()); + ASSERT_EQ(test::gStartupString, ""); } TEST(IDLServerParameterWithStorage, runtimeBoostDouble) { @@ -225,6 +240,10 @@ TEST(IDLServerParameterWithStorage, runtimeBoostDouble) { ASSERT_EQ(sp->allowedToChangeAtRuntime(), true); ASSERT_OK(sp->setFromString("1.0")); ASSERT_EQ(test::gRuntimeBoostDouble.get(), 1.0); + + // Reset to default. + ASSERT_OK(sp->reset()); + ASSERT_EQ(test::gRuntimeBoostDouble.get(), 0.0); } TEST(IDLServerParameterWithStorage, startupStringRedacted) { @@ -237,6 +256,10 @@ TEST(IDLServerParameterWithStorage, startupStringRedacted) { auto obj = b.obj(); ASSERT_EQ(obj.nFields(), 1); ASSERT_EQ(obj[sp->name()].String(), "###"); + + // Reset to default. + ASSERT_OK(sp->reset()); + ASSERT_EQ(test::gStartupStringRedacted, ""); } TEST(IDLServerParameterWithStorage, startupIntWithExpressions) { @@ -295,5 +318,117 @@ TEST(IDLServerParameterWithStorage, RAIIServerParameterController) { ASSERT_EQ(test::gStartupString, coolStartupString); } +/** + * IDLServerParameterWithStorage unit test. + */ +TEST(IDLServerParameterWithStorage, CSPStorageTest) { + // Construct a new IDLClusterServerParameter with the already-defined storage. + // TO-DO: Instantiate this in the IDL file as part of testing SERVER-62253. + auto* clusterParam = makeIDLServerParameterWithStorage( + "changeStreamOptions", clusterParamStorage); + + // Check that current value is the default value. + test::ChangeStreamOptionsClusterParam retrievedParam = clusterParam->getValue(); + ASSERT_EQ(retrievedParam.getPreAndPostImages().getExpireAfterSeconds(), 30); + ASSERT_EQ(retrievedParam.getTestStringField(), ""); + ASSERT_EQ(clusterParam->getClusterParameterTime(), LogicalTime()); + + // Assert that onUpdate functions are fired after set and reset. + size_t count = 0; + clusterParam->setOnUpdate([&count](const test::ChangeStreamOptionsClusterParam& newVal) { + ++count; + return Status::OK(); + }); + + // Set to new value and check that the updated value is seen on get. + test::ChangeStreamOptionsClusterParam updatedParam; + test::PreAndPostImagesStruct updatedPrePostImgs; + ClusterServerParameter baseCSP; + + updatedPrePostImgs.setExpireAfterSeconds(40); + LogicalTime updateTime = LogicalTime(Timestamp(Date_t::now())); + baseCSP.setClusterParameterTime(updateTime); + baseCSP.set_id("changeStreamOptions"_sd); + + updatedParam.setClusterServerParameter(baseCSP); + updatedParam.setPreAndPostImages(updatedPrePostImgs); + updatedParam.setTestStringField("testString"); + ASSERT_OK(clusterParam->ServerParameter::set(updatedParam.toBSON())); + + retrievedParam = clusterParam->getValue(); + ASSERT_EQ(retrievedParam.getPreAndPostImages().getExpireAfterSeconds(), 40); + ASSERT_EQ(retrievedParam.getTestStringField(), "testString"); + ASSERT_EQ(retrievedParam.getClusterParameterTime(), updateTime); + ASSERT_EQ(clusterParam->getClusterParameterTime(), updateTime); + ASSERT_EQ(count, 1); + + // Append to BSONObj and verify that expected fields are present. + BSONObjBuilder b; + clusterParam->append(nullptr, b, clusterParam->name()); + auto obj = b.obj(); + ASSERT_EQ(obj.nFields(), 4); + ASSERT_EQ(obj["_id"_sd].String(), "changeStreamOptions"); + ASSERT_EQ(obj["preAndPostImages"_sd].Obj()["expireAfterSeconds"].Long(), 40); + ASSERT_EQ(obj["testStringField"_sd].String(), "testString"); + ASSERT_EQ(obj["clusterParameterTime"_sd].timestamp(), updateTime.asTimestamp()); + + // setFromString should fail for cluster server parameters. + ASSERT_NOT_OK(clusterParam->setFromString("")); + + // Reset the parameter and check that it now has its default value. + ASSERT_OK(clusterParam->reset()); + retrievedParam = clusterParam->getValue(); + ASSERT_EQ(retrievedParam.getPreAndPostImages().getExpireAfterSeconds(), 30); + ASSERT_EQ(retrievedParam.getTestStringField(), ""); + ASSERT_EQ(retrievedParam.getClusterParameterTime(), LogicalTime()); + ASSERT_EQ(clusterParam->getClusterParameterTime(), LogicalTime()); + ASSERT_EQ(count, 2); + + // Update the default value. The parameter should automatically reset to the new default value. + test::ChangeStreamOptionsClusterParam newDefaultParam; + test::PreAndPostImagesStruct newDefaultPrePostImgs; + + newDefaultPrePostImgs.setExpireAfterSeconds(35); + newDefaultParam.setPreAndPostImages(newDefaultPrePostImgs); + newDefaultParam.setTestStringField("default"); + ASSERT_OK(clusterParam->setDefault(newDefaultParam)); + retrievedParam = clusterParam->getValue(); + ASSERT_EQ(retrievedParam.getPreAndPostImages().getExpireAfterSeconds(), 35); + ASSERT_EQ(retrievedParam.getTestStringField(), "default"); + ASSERT_EQ(retrievedParam.getClusterParameterTime(), LogicalTime()); + ASSERT_EQ(clusterParam->getClusterParameterTime(), LogicalTime()); + ASSERT_EQ(count, 3); + + // Updating the default value a second time should have no effect. + newDefaultPrePostImgs.setExpireAfterSeconds(45); + newDefaultParam.setPreAndPostImages(newDefaultPrePostImgs); + newDefaultParam.setTestStringField("newDefault"); + ASSERT_OK(clusterParam->setDefault(newDefaultParam)); + retrievedParam = clusterParam->getValue(); + ASSERT_EQ(retrievedParam.getPreAndPostImages().getExpireAfterSeconds(), 35); + ASSERT_EQ(retrievedParam.getTestStringField(), "default"); + ASSERT_EQ(count, 3); + + // Assert that validation works as expected both when called separately and implicitly during + // set. + clusterParam->addValidator([&](const test::ChangeStreamOptionsClusterParam& newVal) { + if (newVal.getPreAndPostImages().getExpireAfterSeconds() < 0) { + return Status(ErrorCodes::BadValue, "Should be positive value only"); + } + return Status::OK(); + }); + updatedPrePostImgs.setExpireAfterSeconds(-1); + updatedParam.setPreAndPostImages(updatedPrePostImgs); + updatedParam.setTestStringField("newTestString"); + updateTime = LogicalTime(Timestamp(Date_t::now())); + ASSERT_NOT_OK(clusterParam->ServerParameter::validate(updatedParam.toBSON())); + ASSERT_NOT_OK(clusterParam->ServerParameter::set(updatedParam.toBSON())); + retrievedParam = clusterParam->getValue(); + ASSERT_EQ(retrievedParam.getPreAndPostImages().getExpireAfterSeconds(), 35); + ASSERT_EQ(retrievedParam.getTestStringField(), "default"); + ASSERT_EQ(clusterParam->getClusterParameterTime(), LogicalTime()); + ASSERT_EQ(count, 3); +} + } // namespace } // namespace mongo diff --git a/src/mongo/idl/server_parameter_with_storage_test.idl b/src/mongo/idl/server_parameter_with_storage_test.idl index f6724464fef..fb057b4ac1f 100644 --- a/src/mongo/idl/server_parameter_with_storage_test.idl +++ b/src/mongo/idl/server_parameter_with_storage_test.idl @@ -30,6 +30,33 @@ global: cpp_namespace: "mongo::test" cpp_includes: - "mongo/idl/server_parameter_with_storage_test.h" + +imports: + - "mongo/idl/basic_types.idl" + - "mongo/idl/cluster_server_parameter.idl" + +structs: + preAndPostImagesStruct: + description: "Test struct storing pre/post image config params" + fields: + expireAfterSeconds: + description: "Amount of time before image should expire" + type: safeInt64 + default: 30 + + changeStreamOptionsClusterParam: + description: "Test struct storing change stream options" + inline_chained_structs: true + chained_structs: + ClusterServerParameter: ClusterServerParameter + fields: + preAndPostImages: + description: "Stores config about pre/post images" + type: preAndPostImagesStruct + testStringField: + description: "Field to test string parsing" + type: string + default: "" server_parameters: stdIntPreallocated: