From 17804518dab21bd5f936787b51baf013fe16101e Mon Sep 17 00:00:00 2001 From: Gabriel Marks Date: Wed, 5 Apr 2023 14:31:04 +0000 Subject: [PATCH] SERVER-74107 Use transaction in cluster parameter refresher to get up-to-date FCV --- jstests/auth/list_all_local_sessions.js | 14 +- .../cluster_server_parameter_refresher.js | 5 +- .../router_transactions_metrics.js | 11 +- .../server_parameter_fcv_upgrade_downgrade.js | 15 +- .../telemetry/telemetry_collect_on_mongos.js | 3 +- jstests/noPassthrough/transaction_reaper.js | 3 + .../internal_sessions_reaping_basic.js | 2 +- .../sharding/internal_txns/kill_sessions.js | 7 +- jstests/sharding/internal_txns/sessions.js | 7 +- .../sharding/mongos_precache_routing_info.js | 15 +- .../sharding/query/aggregation_currentop.js | 6 +- .../sessions_collection_auto_healing.js | 9 +- ...mmit_optimizations_for_read_only_shards.js | 8 +- src/mongo/db/server_parameter.cpp | 58 ++---- src/mongo/db/server_parameter.h | 34 +++- src/mongo/db/service_entry_point_common.cpp | 3 +- src/mongo/idl/SConscript | 2 + .../cluster_server_parameter_refresher.cpp | 192 +++++++++++++----- .../idl/cluster_server_parameter_refresher.h | 1 + src/mongo/s/commands/strategy.cpp | 2 +- 20 files changed, 276 insertions(+), 121 deletions(-) diff --git a/jstests/auth/list_all_local_sessions.js b/jstests/auth/list_all_local_sessions.js index 3efe32fc3e1..3b90d01b545 100644 --- a/jstests/auth/list_all_local_sessions.js +++ b/jstests/auth/list_all_local_sessions.js @@ -1,5 +1,5 @@ // Auth tests for the $listLocalSessions {allUsers:true} aggregation stage. -// @tags: [requires_sharding] +// @tags: [requires_fcv_70, requires_sharding] (function() { 'use strict'; @@ -49,8 +49,16 @@ const mongod = MongoRunner.runMongod({auth: ""}); runListAllLocalSessionsTest(mongod); MongoRunner.stopMongod(mongod); -const st = - new ShardingTest({shards: 1, mongos: 1, config: 1, other: {keyFile: 'jstests/libs/key1'}}); +const st = new ShardingTest({ + shards: 1, + mongos: 1, + config: 1, + other: { + keyFile: 'jstests/libs/key1', + mongosOptions: + {setParameter: {'failpoint.skipClusterParameterRefresh': "{'mode':'alwaysOn'}"}} + } +}); runListAllLocalSessionsTest(st.s0); st.stop(); })(); diff --git a/jstests/noPassthrough/cluster_server_parameter_refresher.js b/jstests/noPassthrough/cluster_server_parameter_refresher.js index c4d5e4b8e8d..a0be0a5dac9 100644 --- a/jstests/noPassthrough/cluster_server_parameter_refresher.js +++ b/jstests/noPassthrough/cluster_server_parameter_refresher.js @@ -128,8 +128,11 @@ function runTest(st, startupRefreshIntervalMS) { [st.configRS, ...st._rs.map(rs => rs.test)].forEach(rs => { assert(rs.getPrimary().getDB('config').clusterParameters.drop()); }); - expectedParams = {}; + // Perform a dummy write in order to get the config shard's cluster time cached on the mongos. + st.s.getDB('config').abc.insert({a: "hello"}); + + expectedParams = {}; assertParams(startupRefreshIntervalRelaxedMS); } diff --git a/jstests/noPassthrough/router_transactions_metrics.js b/jstests/noPassthrough/router_transactions_metrics.js index f74b6d596af..21677c30dfa 100644 --- a/jstests/noPassthrough/router_transactions_metrics.js +++ b/jstests/noPassthrough/router_transactions_metrics.js @@ -1,6 +1,7 @@ // Tests multi-statement transactions metrics in the serverStatus output from mongos in various // basic cases. // @tags: [ +// requires_fcv_70, // uses_multi_shard_transaction, // uses_transactions, // ] @@ -201,7 +202,15 @@ const dbName = "test"; const collName = "foo"; const ns = dbName + '.' + collName; -const st = new ShardingTest({shards: 2, mongos: 2, config: 1}); +const st = new ShardingTest({ + shards: 2, + mongos: 2, + config: 1, + other: { + mongosOptions: + {setParameter: {'failpoint.skipClusterParameterRefresh': "{'mode':'alwaysOn'}"}} + } +}); const session = st.s.startSession(); const sessionDB = session.getDatabase(dbName); diff --git a/jstests/noPassthrough/server_parameter_fcv_upgrade_downgrade.js b/jstests/noPassthrough/server_parameter_fcv_upgrade_downgrade.js index 9fc483d85a5..c1fef519d4a 100644 --- a/jstests/noPassthrough/server_parameter_fcv_upgrade_downgrade.js +++ b/jstests/noPassthrough/server_parameter_fcv_upgrade_downgrade.js @@ -111,23 +111,18 @@ function runDowngradeUpgradeTestForCWSP(conn, isMongod, isStandalone, verifyStat verifyStateCallback(sp, true); } - // Downgrade FCV and ensure we can't set, and get either fails (if FCV is known by the - // server) or gets the default value (if it is not). + // Downgrade FCV and ensure we can't set or get. // If our downgrade takes us below the minimum FCV for // featureFlagAuditConfigClusterParameter, we expect all cluster parameter commands to fail // for standalone. assert.commandWorked(admin.runCommand({setFeatureCompatibilityVersion: lastLTSFCV})); - if (isMongod) { - assertGetFailed(admin.runCommand({getClusterParameter: sp})); - } else { - const afterDowngrade = - assert.commandWorked(admin.runCommand({getClusterParameter: sp})); - assert.eq(val(afterDowngrade), initval); - } + + assertGetFailed(admin.runCommand({getClusterParameter: sp})); assertSetFailed(admin.runCommand({setClusterParameter: {[sp]: {intData: updateVal + 1}}})); + if (!(isStandalone && !FeatureFlagUtil.isEnabled(admin, 'AuditConfigClusterParameter'))) { assertParamExistenceInGetParamStar( - assert.commandWorked(admin.runCommand({getClusterParameter: "*"})), sp, !isMongod); + assert.commandWorked(admin.runCommand({getClusterParameter: "*"})), sp, false); } if (verifyStateCallback !== undefined) { verifyStateCallback(sp, false); diff --git a/jstests/noPassthrough/telemetry/telemetry_collect_on_mongos.js b/jstests/noPassthrough/telemetry/telemetry_collect_on_mongos.js index 7731f5f8490..50e8912211b 100644 --- a/jstests/noPassthrough/telemetry/telemetry_collect_on_mongos.js +++ b/jstests/noPassthrough/telemetry/telemetry_collect_on_mongos.js @@ -1,6 +1,6 @@ /** * Test that mongos is collecting telemetry metrics. - * @tags: [featureFlagTelemetry] + * @tags: [requires_fcv_70, featureFlagTelemetry] */ (function() { "use strict"; @@ -18,6 +18,7 @@ const setup = () => { mongosOptions: { setParameter: { internalQueryConfigureTelemetrySamplingRate: 2147483647, + 'failpoint.skipClusterParameterRefresh': "{'mode':'alwaysOn'}" } }, }); diff --git a/jstests/noPassthrough/transaction_reaper.js b/jstests/noPassthrough/transaction_reaper.js index becc6a59fec..1240cfd1ba3 100644 --- a/jstests/noPassthrough/transaction_reaper.js +++ b/jstests/noPassthrough/transaction_reaper.js @@ -1,4 +1,5 @@ // @tags: [ +// requires_fcv_70, // requires_replication, // requires_sharding, // ] @@ -39,6 +40,8 @@ function Sharding(lifetime) { rs: true, rsOptions: {setParameter: {TransactionRecordMinimumLifetimeMinutes: lifetime}}, rs0: {nodes: 1}, + mongosOptions: + {setParameter: {'failpoint.skipClusterParameterRefresh': "{'mode':'alwaysOn'}"}} }, }); diff --git a/jstests/replsets/internal_sessions_reaping_basic.js b/jstests/replsets/internal_sessions_reaping_basic.js index 18a15a6ae1b..adf6ec1021e 100644 --- a/jstests/replsets/internal_sessions_reaping_basic.js +++ b/jstests/replsets/internal_sessions_reaping_basic.js @@ -3,7 +3,7 @@ * config.image_collection entries for a transaction session if the logical session that it * corresponds to has expired and been removed from the config.system.sessions collection. * - * @tags: [requires_fcv_60, uses_transactions] + * @tags: [requires_fcv_70, uses_transactions] */ (function() { diff --git a/jstests/sharding/internal_txns/kill_sessions.js b/jstests/sharding/internal_txns/kill_sessions.js index 605d2d9079b..3d4ba47addd 100644 --- a/jstests/sharding/internal_txns/kill_sessions.js +++ b/jstests/sharding/internal_txns/kill_sessions.js @@ -1,7 +1,7 @@ /* * Tests running killSessions to kill internal sessions on both mongos and mongod. * - * @tags: [requires_fcv_60, uses_transactions, temporary_catalog_shard_incompatible] + * @tags: [requires_fcv_70, uses_transactions, temporary_catalog_shard_incompatible] */ (function() { 'use strict'; @@ -10,7 +10,10 @@ TestData.disableImplicitSessions = true; const st = new ShardingTest({ shards: 1, - mongosOptions: {setParameter: {maxSessions: 1}}, + mongosOptions: { + setParameter: + {maxSessions: 1, 'failpoint.skipClusterParameterRefresh': "{'mode':'alwaysOn'}"} + }, shardOptions: {setParameter: {maxSessions: 1}} }); const shard0Primary = st.rs0.getPrimary(); diff --git a/jstests/sharding/internal_txns/sessions.js b/jstests/sharding/internal_txns/sessions.js index 27f890f14d8..fe4ffbf1e5e 100644 --- a/jstests/sharding/internal_txns/sessions.js +++ b/jstests/sharding/internal_txns/sessions.js @@ -1,7 +1,7 @@ /* * Tests basic support for internal sessions. * - * @tags: [requires_fcv_60, uses_transactions, temporary_catalog_shard_incompatible] + * @tags: [requires_fcv_70, uses_transactions, temporary_catalog_shard_incompatible] */ (function() { 'use strict'; @@ -10,7 +10,10 @@ TestData.disableImplicitSessions = true; const st = new ShardingTest({ shards: 1, - mongosOptions: {setParameter: {maxSessions: 1}}, + mongosOptions: { + setParameter: + {maxSessions: 1, 'failpoint.skipClusterParameterRefresh': "{'mode':'alwaysOn'}"} + }, shardOptions: {setParameter: {maxSessions: 1}} }); const shard0Primary = st.rs0.getPrimary(); diff --git a/jstests/sharding/mongos_precache_routing_info.js b/jstests/sharding/mongos_precache_routing_info.js index 6c3e59e7591..573391a3b59 100644 --- a/jstests/sharding/mongos_precache_routing_info.js +++ b/jstests/sharding/mongos_precache_routing_info.js @@ -1,8 +1,16 @@ +// @tags: [requires_fcv_70] + (function() { 'use strict'; // create -var s = new ShardingTest({shards: 2}); +var s = new ShardingTest({ + shards: 2, + other: { + mongosOptions: + {setParameter: {'failpoint.skipClusterParameterRefresh': "{'mode':'alwaysOn'}"}} + } +}); var db = s.getDB("test"); var ss = db.serverStatus(); @@ -34,7 +42,10 @@ assert.eq(1, ss.shardingStatistics.catalogCache.countFullRefreshesStarted); // does not pre cache when set parameter is disabled s.restartMongos(0, { restart: true, - setParameter: {loadRoutingTableOnStartup: false}, + setParameter: { + loadRoutingTableOnStartup: false, + 'failpoint.skipClusterParameterRefresh': "{'mode':'alwaysOn'}" + }, }); db = s.getDB("test"); diff --git a/jstests/sharding/query/aggregation_currentop.js b/jstests/sharding/query/aggregation_currentop.js index bfa8350a00d..1c566fbc9fb 100644 --- a/jstests/sharding/query/aggregation_currentop.js +++ b/jstests/sharding/query/aggregation_currentop.js @@ -42,7 +42,11 @@ const stParams = { name: jsTestName(), keyFile: key, shards: 3, - rs: {nodes: 1, setParameter: {internalQueryExecYieldIterations: 1}} + rs: {nodes: 1, setParameter: {internalQueryExecYieldIterations: 1}}, + other: { + mongosOptions: + {setParameter: {'failpoint.skipClusterParameterRefresh': "{'mode':'alwaysOn'}"}} + } }; // Create a new sharded cluster for testing. We set the internalQueryExecYieldIterations diff --git a/jstests/sharding/sessions_collection_auto_healing.js b/jstests/sharding/sessions_collection_auto_healing.js index ed5e1e83a38..b662884e44b 100644 --- a/jstests/sharding/sessions_collection_auto_healing.js +++ b/jstests/sharding/sessions_collection_auto_healing.js @@ -1,6 +1,7 @@ /** * Requires no shards. * @tags: [ + * requires_fcv_70, * catalog_shard_incompatible, * requires_fcv_70, * ] @@ -17,7 +18,13 @@ load("jstests/libs/collection_drop_recreate.js"); // For assert[Drop|Create]Col // implicit sessions. TestData.disableImplicitSessions = true; -var st = new ShardingTest({shards: 0}); +var st = new ShardingTest({ + shards: 0, + other: { + mongosOptions: + {setParameter: {'failpoint.skipClusterParameterRefresh': "{'mode':'alwaysOn'}"}} + } +}); var configSvr = st.configRS.getPrimary(); var mongos = st.s; diff --git a/jstests/sharding/txn_commit_optimizations_for_read_only_shards.js b/jstests/sharding/txn_commit_optimizations_for_read_only_shards.js index ef85bd39930..159662b1849 100644 --- a/jstests/sharding/txn_commit_optimizations_for_read_only_shards.js +++ b/jstests/sharding/txn_commit_optimizations_for_read_only_shards.js @@ -6,7 +6,8 @@ * no failures, a participant having failed over, a participant being unable to satisfy the client's * writeConcern, and an invalid client writeConcern. * - * @tags: [uses_transactions, uses_multi_shard_transaction, temporary_catalog_shard_incompatible] + * @tags: [requires_fcv_70, uses_transactions, uses_multi_shard_transaction, + * temporary_catalog_shard_incompatible] */ (function() { @@ -65,7 +66,10 @@ let st = new ShardingTest({ // Create shards with more than one node because we test for writeConcern majority failing. config: 1, other: { - mongosOptions: {verbose: 3}, + mongosOptions: { + verbose: 3, + setParameter: {'failpoint.skipClusterParameterRefresh': "{'mode':'alwaysOn'}"} + }, rs0: {nodes: [{}, {rsConfig: {priority: 0}}]}, rs1: {nodes: [{}, {rsConfig: {priority: 0}}]}, rs2: {nodes: [{}, {rsConfig: {priority: 0}}]}, diff --git a/src/mongo/db/server_parameter.cpp b/src/mongo/db/server_parameter.cpp index 12546757b08..ba2d5db22c1 100644 --- a/src/mongo/db/server_parameter.cpp +++ b/src/mongo/db/server_parameter.cpp @@ -85,6 +85,22 @@ bool ServerParameter::isEnabled() const { } bool ServerParameter::isEnabledOnVersion( + const multiversion::FeatureCompatibilityVersion& targetFCV) const { + if (_disableState != DisableState::Enabled) { + return false; + } + return _isEnabledOnVersion(targetFCV); +} + +bool ServerParameter::canBeEnabledOnVersion( + const multiversion::FeatureCompatibilityVersion& targetFCV) const { + if (_disableState == DisableState::PermanentlyDisabled) { + return false; + } + return _isEnabledOnVersion(targetFCV); +} + +bool ServerParameter::_isEnabledOnVersion( const multiversion::FeatureCompatibilityVersion& targetFCV) const { return minFCVIsLessThanOrEqualToVersion(targetFCV) && !featureFlagIsDisabledOnVersion(targetFCV); @@ -198,51 +214,11 @@ Status IDLServerParameterDeprecatedAlias::setFromString(StringData str, return _sp->setFromString(str, tenantId); } -namespace { -class DisabledTestParameter : public ServerParameter { -public: - explicit DisabledTestParameter(ServerParameter* sp) - : ServerParameter(sp->name(), sp->getServerParameterType()), _sp(sp) {} - - void append(OperationContext* opCtx, - BSONObjBuilder* b, - StringData name, - const boost::optional&) final {} - - Status validate(const BSONElement& newValueElement, - const boost::optional& tenantId) const final { - return {ErrorCodes::BadValue, - str::stream() << "Server parameter: '" << name() << "' is currently disabled"}; - } - - Status setFromString(StringData, const boost::optional&) final { - return {ErrorCodes::BadValue, - str::stream() << "Server parameter: '" << name() << "' is currently disabled"}; - } - - Status set(const BSONElement& newValueElement, const boost::optional&) final { - return setFromString("", boost::none); - } - - Status reset(const boost::optional&) final { - return setFromString("", boost::none); - } - - bool isEnabledOnVersion(const multiversion::FeatureCompatibilityVersion&) const override { - return false; - } - -private: - // Retain the original pointer to avoid ASAN complaining. - ServerParameter* _sp; -}; -} // namespace - void ServerParameterSet::disableTestParameters() { for (auto& spit : _map) { auto*& sp = spit.second; if (sp->isTestOnly()) { - sp = new DisabledTestParameter(sp); + sp->disable(true /* permanent */); } } } diff --git a/src/mongo/db/server_parameter.h b/src/mongo/db/server_parameter.h index 3ecc6a5b6f5..47c840ff99b 100644 --- a/src/mongo/db/server_parameter.h +++ b/src/mongo/db/server_parameter.h @@ -218,11 +218,31 @@ public: _redact = true; } +private: + enum DisableState { Enabled = 0, TemporarilyDisabled = 1, PermanentlyDisabled = 2 }; + +public: + void disable(bool permanent) { + if (_disableState != DisableState::PermanentlyDisabled) { + _disableState = + permanent ? DisableState::PermanentlyDisabled : DisableState::TemporarilyDisabled; + } + } + + void enable() { + if (_disableState == DisableState::TemporarilyDisabled) { + _disableState = DisableState::Enabled; + } + } + bool isEnabled() const; - // Return whether this server parameter is compatible with the given FCV. - virtual bool isEnabledOnVersion( - const multiversion::FeatureCompatibilityVersion& targetFCV) const; + // Return whether this server parameter would be enabled with the given FCV + bool isEnabledOnVersion(const multiversion::FeatureCompatibilityVersion& targetFCV) const; + + // Return whether this server parameter is compatible with the given FCV, regardless of if it is + // temporarily disabled + bool canBeEnabledOnVersion(const multiversion::FeatureCompatibilityVersion& targetFCV) const; void setFeatureFlag(FeatureFlag* featureFlag) { _featureFlag = featureFlag; @@ -233,6 +253,9 @@ public: } protected: + virtual bool _isEnabledOnVersion( + const multiversion::FeatureCompatibilityVersion& targetFCV) const; + bool featureFlagIsDisabledOnVersion( const multiversion::FeatureCompatibilityVersion& targetFCV) const; @@ -251,6 +274,11 @@ private: ServerParameterType _type; bool _testOnly = false; bool _redact = false; + + // Tracks whether a parameter is enabled, temporarily disabled, or permanently disabled. This is + // used when disabling (permanently) test-only parameters, and when enabling/disabling + // (temporarily) cluster parameters on the mongos based on the cluster's FCV. + DisableState _disableState = DisableState::Enabled; }; class ServerParameterSet { diff --git a/src/mongo/db/service_entry_point_common.cpp b/src/mongo/db/service_entry_point_common.cpp index 57095411cb9..b38e15433e1 100644 --- a/src/mongo/db/service_entry_point_common.cpp +++ b/src/mongo/db/service_entry_point_common.cpp @@ -1510,7 +1510,8 @@ void ExecCommandDatabase::_initiateCommand() { const auto allowTransactionsOnConfigDatabase = (serverGlobalParams.clusterRole.has(ClusterRole::ConfigServer) || - serverGlobalParams.clusterRole.has(ClusterRole::ShardServer)); + serverGlobalParams.clusterRole.has(ClusterRole::ShardServer)) || + client->isFromSystemConnection(); const auto invocationNss = _invocation->ns(); diff --git a/src/mongo/idl/SConscript b/src/mongo/idl/SConscript index bdcbe0cde2a..7fd0e80b436 100644 --- a/src/mongo/idl/SConscript +++ b/src/mongo/idl/SConscript @@ -88,6 +88,8 @@ env.Library( LIBDEPS_PRIVATE=[ '$BUILD_DIR/mongo/db/audit', '$BUILD_DIR/mongo/db/server_base', + '$BUILD_DIR/mongo/db/transaction/transaction_api', + '$BUILD_DIR/mongo/executor/inline_executor', '$BUILD_DIR/mongo/s/grid', 'cluster_server_parameter', 'cluster_server_parameter_common', diff --git a/src/mongo/idl/cluster_server_parameter_refresher.cpp b/src/mongo/idl/cluster_server_parameter_refresher.cpp index 5398bdcae98..6b35914cb57 100644 --- a/src/mongo/idl/cluster_server_parameter_refresher.cpp +++ b/src/mongo/idl/cluster_server_parameter_refresher.cpp @@ -33,14 +33,21 @@ #include "mongo/db/audit.h" #include "mongo/db/commands/list_databases_for_all_tenants_gen.h" +#include "mongo/db/feature_compatibility_version_parser.h" #include "mongo/db/multitenancy_gen.h" +#include "mongo/db/transaction/transaction_api.h" +#include "mongo/db/vector_clock.h" #include "mongo/idl/cluster_server_parameter_common.h" #include "mongo/idl/cluster_server_parameter_refresher_gen.h" #include "mongo/logv2/log.h" #include "mongo/s/grid.h" +#include "mongo/s/is_mongos.h" +#include "mongo/util/stacktrace.h" +#include "mongo/util/version/releases.h" #define MONGO_LOGV2_DEFAULT_COMPONENT ::mongo::logv2::LogComponent::kControl +MONGO_FAIL_POINT_DEFINE(skipClusterParameterRefresh); namespace mongo { namespace { @@ -51,44 +58,97 @@ Seconds loadInterval() { return Seconds(clusterServerParameterRefreshIntervalSecs.load()); } -StatusWith>> -getClusterParametersFromConfigServer(OperationContext* opCtx) { - // Attempt to retrieve cluster parameter documents from the config server. - // exhaustiveFindOnConfig makes up to 3 total attempts if it receives a retriable error before - // giving up. - LOGV2_DEBUG(6226404, 3, "Retrieving cluster server parameters from config server"); - auto configServers = Grid::get(opCtx)->shardRegistry()->getConfigShard(); - auto swTenantIds = getTenantsWithConfigDbsOnShard(opCtx, configServers.get()); - if (!swTenantIds.isOK()) { - return swTenantIds.getStatus(); - } - auto tenantIds = std::move(swTenantIds.getValue()); +std::pair>> +getFCVAndClusterParametersFromConfigServer() { + // Use an alternative client region, because we call refreshParameters both from the internal + // refresher process and from getClusterParameter. + auto altClient = getGlobalServiceContext()->makeClient("clusterParameterRefreshTransaction"); + AlternativeClientRegion clientRegion(altClient); + auto opCtx = cc().makeOperationContext(); + auto as = AuthorizationSession::get(cc()); + as->grantInternalAuthorization(opCtx.get()); - TenantIdMap> allDocs; - for (const auto& tenantId : tenantIds) { - auto swFindResponse = configServers->exhaustiveFindOnConfig( - opCtx, - ReadPreferenceSetting{ReadPreference::PrimaryOnly}, - repl::ReadConcernLevel::kMajorityReadConcern, - NamespaceString::makeClusterParametersNSS(tenantId), - BSONObj(), - BSONObj(), - boost::none); + auto configServers = Grid::get(opCtx.get())->shardRegistry()->getConfigShard(); + // Note that we get the list of tenants outside of the transaction. This should be okay, as if + // we miss out on some new tenants created between this call and the transaction, we are just + // getting slightly old data. Importantly, different tenants' cluster parameters don't interact + // with each other, so we don't need a consistent snapshot of cluster parameters across all + // tenants, just a consistent snapshot per tenant. + auto tenantIds = + uassertStatusOK(getTenantsWithConfigDbsOnShard(opCtx.get(), configServers.get())); - // If the error is not retriable or persists beyond the max number of retry attempts, give - // up and throw an error. - if (!swFindResponse.isOK()) { - return swFindResponse.getStatus(); - } - stdx::unordered_map docsMap; - for (const auto& doc : swFindResponse.getValue().docs) { - auto name = doc["_id"].String(); - docsMap.insert({std::move(name), doc}); - } - allDocs.insert({std::move(tenantId), std::move(docsMap)}); - } + auto allDocs = std::make_shared>>(); + auto fcv = std::make_shared(); + auto doFetch = [allDocs, fcv, &tenantIds](const txn_api::TransactionClient& txnClient, + ExecutorPtr txnExec) { + FindCommandRequest findFCV{NamespaceString("admin.system.version")}; + findFCV.setFilter(BSON("_id" + << "featureCompatibilityVersion")); + return txnClient.exhaustiveFind(findFCV) + .thenRunOn(txnExec) + .then([fcv, allDocs, &tenantIds, &txnClient, txnExec]( + const std::vector& foundDocs) { + uassert(7410710, + "Expected to find FCV in admin.system.version but found nothing!", + !foundDocs.empty()); + *fcv = FeatureCompatibilityVersionParser::parseVersion( + foundDocs[0]["version"].String()); - return allDocs; + // Fetch one tenant, then call doFetchTenants for the rest of the tenants within + // then() recursively. + auto doFetchTenants = [](auto it, + const auto& tenantIds, + auto allDocs, + const auto& txnClient, + ExecutorPtr txnExec, + auto& doFetchTenants_ref) mutable { + if (it == tenantIds.end()) { + return SemiFuture::makeReady(); + } + FindCommandRequest findClusterParametersTenant{ + NamespaceString::makeClusterParametersNSS(*it)}; + // We don't specify a filter as we want all documents. + return txnClient.exhaustiveFind(findClusterParametersTenant) + .thenRunOn(txnExec) + .then([&doFetchTenants_ref, &txnClient, &tenantIds, txnExec, it, allDocs]( + const std::vector& foundDocs) { + stdx::unordered_map docsMap; + for (const auto& doc : foundDocs) { + auto name = doc["_id"].String(); + docsMap.insert({std::move(name), doc.getOwned()}); + } + allDocs->insert({*it, std::move(docsMap)}); + return doFetchTenants_ref(std::next(it), + tenantIds, + allDocs, + txnClient, + txnExec, + doFetchTenants_ref); + }) + .semi(); + }; + return doFetchTenants( + tenantIds.begin(), tenantIds, allDocs, txnClient, txnExec, doFetchTenants); + }) + .semi(); + }; + + repl::ReadConcernArgs::get(opCtx.get()) = + repl::ReadConcernArgs(repl::ReadConcernLevel::kSnapshotReadConcern); + + // We need to commit w/ writeConcern = majority for readConcern = snapshot to work. + opCtx->setWriteConcern(WriteConcernOptions{WriteConcernOptions::kMajority, + WriteConcernOptions::SyncMode::UNSET, + WriteConcernOptions::kNoTimeout}); + + auto executor = Grid::get(opCtx.get())->getExecutorPool()->getFixedExecutor(); + auto inlineExecutor = std::make_shared(); + auto sleepInlineExecutor = inlineExecutor->getSleepableExecutor(executor); + txn_api::SyncTransactionWithRetries txn( + opCtx.get(), sleepInlineExecutor, nullptr, inlineExecutor); + txn.run(opCtx.get(), doFetch); + return {*fcv, *allDocs}; } } // namespace @@ -121,30 +181,56 @@ void ClusterServerParameterRefresher::setPeriod(Milliseconds period) { } Status ClusterServerParameterRefresher::refreshParameters(OperationContext* opCtx) { - // Query the config servers for all cluster parameter documents. - auto swClusterParameterDocs = getClusterParametersFromConfigServer(opCtx); - if (!swClusterParameterDocs.isOK()) { - LOGV2_WARNING(6226401, - "Could not refresh cluster server parameters from config servers. Will retry " - "after refresh interval elapses", - "clusterServerParameterRefreshIntervalSecs"_attr = loadInterval(), - "reason"_attr = swClusterParameterDocs.getStatus().reason()); - return swClusterParameterDocs.getStatus(); + invariant(isMongos()); + multiversion::FeatureCompatibilityVersion fcv; + TenantIdMap> clusterParameterDocs; + + try { + std::tie(fcv, clusterParameterDocs) = getFCVAndClusterParametersFromConfigServer(); + } catch (const DBException& ex) { + LOGV2_WARNING( + 7410719, + "Could not refresh cluster server parameters from config servers due to failure in " + "getFCVAndClusterParametersFromConfigServer. Will retry after refresh interval", + "ex"_attr = ex.toStatus()); + return ex.toStatus(); } // Set each in-memory cluster parameter that was returned in the response. bool isSuccessful = true; Status status = Status::OK(); ServerParameterSet* clusterParameterCache = ServerParameterSet::getClusterParameterSet(); - - auto clusterParameterDocs = std::move(swClusterParameterDocs.getValue()); + bool fcvChanged = fcv != _lastFcv; + if (fcvChanged) { + LOGV2_DEBUG(7410705, + 3, + "Cluster's FCV was different from last during refresh", + "oldFCV"_attr = multiversion::toString(_lastFcv), + "newFCV"_attr = multiversion::toString(fcv)); + } std::vector allUpdatedParameters; allUpdatedParameters.reserve(clusterParameterDocs.size()); for (const auto& [tenantId, tenantParamDocs] : clusterParameterDocs) { std::vector updatedParameters; updatedParameters.reserve(tenantParamDocs.size()); - for (const auto& [name, sp] : clusterParameterCache->getMap()) { + for (auto [name, sp] : clusterParameterCache->getMap()) { + if (fcvChanged) { + // Use canBeEnabled because if we previously temporarily disabled the parameter, + // isEnabled will be false + if (sp->canBeEnabledOnVersion(_lastFcv) && !sp->canBeEnabledOnVersion(fcv)) { + // Parameter is newly disabled on cluster + LOGV2_DEBUG( + 7410703, 3, "Disabling parameter during refresh", "name"_attr = name); + sp->disable(false /* permanent */); + continue; + } else if (sp->canBeEnabledOnVersion(fcv) && !sp->canBeEnabledOnVersion(_lastFcv)) { + // Parameter is newly enabled on cluster + LOGV2_DEBUG( + 7410704, 3, "Enabling parameter during refresh", "name"_attr = name); + sp->enable(); + } + } if (!sp->isEnabled()) { continue; } @@ -196,12 +282,18 @@ Status ClusterServerParameterRefresher::refreshParameters(OperationContext* opCt "clusterParameterDocuments"_attr = allUpdatedParameters); } + _lastFcv = fcv; + return status; } void ClusterServerParameterRefresher::start(ServiceContext* serviceCtx, OperationContext* opCtx) { auto refresher = std::make_unique(); - + // On mongos, this should always be true after FCV initialization + // (Generic FCV reference): + invariant(serverGlobalParams.featureCompatibility.getVersion() == + multiversion::GenericFCV::kLatest); + refresher->_lastFcv = serverGlobalParams.featureCompatibility.getVersion(); auto periodicRunner = serviceCtx->getPeriodicRunner(); invariant(periodicRunner); @@ -218,6 +310,10 @@ void ClusterServerParameterRefresher::start(ServiceContext* serviceCtx, Operatio } void ClusterServerParameterRefresher::run() { + if (MONGO_unlikely(skipClusterParameterRefresh.shouldFail())) { + return; + } + auto opCtx = cc().makeOperationContext(); auto status = refreshParameters(opCtx.get()); if (!status.isOK()) { diff --git a/src/mongo/idl/cluster_server_parameter_refresher.h b/src/mongo/idl/cluster_server_parameter_refresher.h index 746d961d0c5..5eaeb5e6557 100644 --- a/src/mongo/idl/cluster_server_parameter_refresher.h +++ b/src/mongo/idl/cluster_server_parameter_refresher.h @@ -67,6 +67,7 @@ private: void run(); std::unique_ptr _job; + multiversion::FeatureCompatibilityVersion _lastFcv; }; Status clusterServerParameterRefreshIntervalSecsNotify(const int& newValue); diff --git a/src/mongo/s/commands/strategy.cpp b/src/mongo/s/commands/strategy.cpp index b1f16d170f3..63a4f1b59d9 100644 --- a/src/mongo/s/commands/strategy.cpp +++ b/src/mongo/s/commands/strategy.cpp @@ -586,7 +586,7 @@ void ParseAndRunCommand::_parseCommand() { command->attachLogicalSessionsToOpCtx(), true)); - auto allowTransactionsOnConfigDatabase = !isMongos(); + auto allowTransactionsOnConfigDatabase = !isMongos() || client->isFromSystemConnection(); validateSessionOptions(*_osi, command->getName(), nss, allowTransactionsOnConfigDatabase); _wc.emplace(uassertStatusOK(WriteConcernOptions::extractWCFromCommand(request.body)));