From c7ef5a02d1c6ec0c7300c5e176df7fc7298e945c Mon Sep 17 00:00:00 2001 From: Luxi Liu <71409449+lliu17@users.noreply.github.com> Date: Fri, 19 Aug 2022 16:06:38 +0000 Subject: SERVER-68651 Put isDowngrading to upgraded path under feature flag --- jstests/libs/feature_flag_util.js | 18 +++++++---- .../set_feature_compatibility_version.js | 11 +++++++ ...est_replica_set_startup_in_downgrading_state.js | 9 ++++-- .../test_sharding_startup_in_downgrading_state.js | 9 ++++-- jstests/replsets/rollback_set_fcv.js | 14 +++++--- src/mongo/db/commands/SConscript | 1 + .../db/commands/feature_compatibility_version.cpp | 37 +++++++++++++++------- src/mongo/db/repl/repl_server_parameters.idl | 3 +- 8 files changed, 74 insertions(+), 28 deletions(-) diff --git a/jstests/libs/feature_flag_util.js b/jstests/libs/feature_flag_util.js index 92fb58a5288..4ebb5ef23be 100644 --- a/jstests/libs/feature_flag_util.js +++ b/jstests/libs/feature_flag_util.js @@ -7,9 +7,13 @@ load("jstests/libs/fixture_helpers.js"); */ var FeatureFlagUtil = class { /** - * Returns true if feature flag is enabled, false otherwise. + * If 'ignoreFCV' is true, return whether or not the given feature flag is enabled, + * regardless of the current FCV version (this is used when a feature flag needs to be enabled + * in downgraded FCV versions). + * If 'ignoreFCV' is false or null, we only return true if the flag is enabled and this FCV + * version is greater or equal to the required version for the flag. */ - static isEnabled(db, featureFlag, user) { + static isEnabled(db, featureFlag, user, ignoreFCV) { // In order to get an accurate answer for whether a feature flag is enabled, we need to ask // a mongod. If db represents a connection to mongos, or some other configuration, we need // to obtain the correct connection to a mongod. @@ -46,9 +50,11 @@ var FeatureFlagUtil = class { return false; } - return flagDoc.hasOwnProperty(fullFlagName) && flagDoc[fullFlagName].value && - (!fcvDoc.hasOwnProperty("featureCompatibilityVersion") || - MongoRunner.compareBinVersions(fcvDoc.featureCompatibilityVersion.version, - flagDoc[fullFlagName].version) >= 0); + const flagIsEnabled = flagDoc.hasOwnProperty(fullFlagName) && flagDoc[fullFlagName].value; + const flagVersionIsValid = !fcvDoc.hasOwnProperty("featureCompatibilityVersion") || + MongoRunner.compareBinVersions(fcvDoc.featureCompatibilityVersion.version, + flagDoc[fullFlagName].version) >= 0; + + return flagIsEnabled && (ignoreFCV || flagVersionIsValid); } }; diff --git a/jstests/multiVersion/genericSetFCVUsage/set_feature_compatibility_version.js b/jstests/multiVersion/genericSetFCVUsage/set_feature_compatibility_version.js index ebd7ce28a8b..08c2dd2e6ce 100644 --- a/jstests/multiVersion/genericSetFCVUsage/set_feature_compatibility_version.js +++ b/jstests/multiVersion/genericSetFCVUsage/set_feature_compatibility_version.js @@ -15,6 +15,7 @@ TestData.skipCheckDBHashes = true; load("jstests/libs/index_catalog_helpers.js"); load("jstests/libs/write_concern_util.js"); load("jstests/replsets/rslib.js"); +load("jstests/libs/feature_flag_util.js"); let dbpath = MongoRunner.dataPath + "feature_compatibility_version"; resetDbpath(dbpath); @@ -337,6 +338,16 @@ function runReplicaSetTest(downgradeVersion) { assert.commandFailedWithCode(res, ErrorCodes.WriteConcernFailed); restartServerReplication(secondary); + // If downgrading->upgrading feature is not enabled, + // upgrading the FCV should fail if a previous downgrade has not yet completed. + if (!FeatureFlagUtil.isEnabled(primaryAdminDB, + "DowngradingToUpgrading", + null /* user not specified */, + true /* ignores FCV */)) { + assert.commandFailedWithCode( + primary.adminCommand({setFeatureCompatibilityVersion: latestFCV}), 5147403); + } + if (lastContinuousFCV !== lastLTSFCV) { // We will fail if we have not yet completed a downgrade and attempt to downgrade to a // different target version. diff --git a/jstests/multiVersion/genericSetFCVUsage/test_replica_set_startup_in_downgrading_state.js b/jstests/multiVersion/genericSetFCVUsage/test_replica_set_startup_in_downgrading_state.js index 49baaf22540..16c85aa1cc2 100644 --- a/jstests/multiVersion/genericSetFCVUsage/test_replica_set_startup_in_downgrading_state.js +++ b/jstests/multiVersion/genericSetFCVUsage/test_replica_set_startup_in_downgrading_state.js @@ -1,14 +1,17 @@ /* * Tests startup with a node in downgrading state. * Starts a replica set with 2 nodes. + * + * @tags: [featureFlagDowngradingToUpgrading] */ -load('jstests/multiVersion/libs/verify_versions.js'); -load('jstests/libs/fail_point_util.js'); - (function() { "use strict"; +load('jstests/multiVersion/libs/verify_versions.js'); +load('jstests/libs/fail_point_util.js'); +load("jstests/libs/feature_flag_util.js"); + function runReplicaSet() { let fcvDoc; diff --git a/jstests/multiVersion/genericSetFCVUsage/test_sharding_startup_in_downgrading_state.js b/jstests/multiVersion/genericSetFCVUsage/test_sharding_startup_in_downgrading_state.js index 8d48e87b102..02d0590d956 100644 --- a/jstests/multiVersion/genericSetFCVUsage/test_sharding_startup_in_downgrading_state.js +++ b/jstests/multiVersion/genericSetFCVUsage/test_sharding_startup_in_downgrading_state.js @@ -1,14 +1,17 @@ /* * Tests startup with a node in downgrading state. * Starts a sharded cluster with 2 shards, each with 2 nodes. + * + * @tags: [featureFlagDowngradingToUpgrading] */ -load('jstests/multiVersion/libs/verify_versions.js'); -load('jstests/libs/fail_point_util.js'); - (function() { "use strict"; +load('jstests/multiVersion/libs/verify_versions.js'); +load('jstests/libs/fail_point_util.js'); +load("jstests/libs/feature_flag_util.js"); + function runSharding() { let fcvDoc; let shard0PrimaryAdminDB; diff --git a/jstests/replsets/rollback_set_fcv.js b/jstests/replsets/rollback_set_fcv.js index 4f15a81f3b8..4a82ed33857 100644 --- a/jstests/replsets/rollback_set_fcv.js +++ b/jstests/replsets/rollback_set_fcv.js @@ -16,6 +16,7 @@ load("jstests/replsets/libs/rollback_test.js"); load('jstests/libs/parallel_shell_helpers.js'); load("jstests/libs/fail_point_util.js"); load("jstests/replsets/rslib.js"); +load("jstests/libs/feature_flag_util.js"); function setFCV(fcv) { assert.commandFailedWithCode(db.adminCommand({setFeatureCompatibilityVersion: fcv}), @@ -119,11 +120,16 @@ function rollbackFCVFromDowngradedOrUpgraded(fromFCV, toFCV, failPoint) { checkFCV(secondaryAdminDB, lastLTSFCV, fromFCV); let newPrimary = rollbackTest.getPrimary(); + const newPrimaryAdminDB = newPrimary.getDB('admin'); // As a rule, we forbid downgrading a node while a node is still in the upgrading state and - // vice versa (except for the added path kDowngradingFromLatestToLastLTS -> - // kUpgradingFromLastLTSToLatest -> kLatest). Ensure that the in-memory and on-disk FCV are - // consistent by checking that this rule is upheld after rollback. - if (fromFCV === lastLTSFCV && toFCV === latestFCV) { + // vice versa (except for the added path from downgrading to upgrading). + // Ensure that the in-memory and on-disk FCV are consistent by checking that this rule is + // upheld after rollback. + if (fromFCV === lastLTSFCV && toFCV === latestFCV && + FeatureFlagUtil.isEnabled(newPrimaryAdminDB, + "DowngradingToUpgrading", + null /* user not specified */, + true /* ignores FCV */)) { assert.commandWorked(newPrimary.adminCommand({setFeatureCompatibilityVersion: toFCV})); } else { assert.commandFailedWithCode( diff --git a/src/mongo/db/commands/SConscript b/src/mongo/db/commands/SConscript index a97e065d719..d058ade3878 100644 --- a/src/mongo/db/commands/SConscript +++ b/src/mongo/db/commands/SConscript @@ -249,6 +249,7 @@ env.Library( '$BUILD_DIR/mongo/db/commands', '$BUILD_DIR/mongo/db/dbdirectclient', '$BUILD_DIR/mongo/db/repl/repl_coordinator_interface', + '$BUILD_DIR/mongo/db/repl/repl_server_parameters', '$BUILD_DIR/mongo/db/repl/repl_settings', '$BUILD_DIR/mongo/idl/server_parameter', ], diff --git a/src/mongo/db/commands/feature_compatibility_version.cpp b/src/mongo/db/commands/feature_compatibility_version.cpp index 8ad7cae43f6..3b8e8a4654d 100644 --- a/src/mongo/db/commands/feature_compatibility_version.cpp +++ b/src/mongo/db/commands/feature_compatibility_version.cpp @@ -43,6 +43,7 @@ #include "mongo/db/namespace_string.h" #include "mongo/db/operation_context.h" #include "mongo/db/repl/optime.h" +#include "mongo/db/repl/repl_server_parameters_gen.h" #include "mongo/db/repl/replication_coordinator.h" #include "mongo/db/repl/replication_process.h" #include "mongo/db/repl/storage_interface.h" @@ -74,7 +75,8 @@ namespace { * Utility class for recording permitted transitions between feature compatibility versions and * their on-disk representation as FeatureCompatibilityVersionDocument objects. */ -const class FCVTransitions { +// TODO SERVER-65269: Add back 'const' qualifier to FCVTransitions class declaration +class FCVTransitions { public: FCVTransitions() { auto makeFCVDoc = []( @@ -109,10 +111,6 @@ public: // lastLTS then the second loop iteration just overwrites the first. _transitions[{from, to, isFromConfigServer}] = upgrading; _transitions[{upgrading, to, isFromConfigServer}] = to; - // allow downgrading->upgrading->latest path - _transitions[{GenericFCV::kDowngradingFromLatestToLastLTS, - GenericFCV::kLatest, - isFromConfigServer}] = GenericFCV::kUpgradingFromLastLTSToLatest; } _fcvDocuments[upgrading] = makeFCVDoc(from /* effective */, to /* target */); } @@ -145,6 +143,20 @@ public: } } + /** + * If feature flag gDowngradingToUpgrading is enabled, + * we add the new downgrading->upgrading->latest path. + */ + void featureFlaggedAddNewTransitionState() { + if (repl::feature_flags::gDowngradingToUpgrading.isEnabledAndIgnoreFCV()) { + for (auto isFromConfigServer : std::vector{false, true}) { + _transitions[{GenericFCV::kDowngradingFromLatestToLastLTS, + GenericFCV::kLatest, + isFromConfigServer}] = GenericFCV::kUpgradingFromLastLTSToLatest; + } + } + } + /** * True if a server in multiversion::FeatureCompatibilityVersion "fromVersion" can * transition to "newVersion". Different rules apply if the request is from a config server. @@ -327,14 +339,13 @@ void FeatureCompatibilityVersion::updateFeatureCompatibilityVersionDocument( // We may have just stepped down, in which case we should not proceed. opCtx->checkForInterrupt(); - // Only transition to fully upgraded or downgraded states when we - // have completed all required upgrade/downgrade behavior. - // If kDowngradingFromLatestToLastLTS->kLatest, we want to get the transitional version - // i.e. kUpgradingFromLastLTSToLatest + // Only transition to fully upgraded or downgraded states when we have completed all required + // upgrade/downgrade behavior, unless it is the newly added downgrading to upgrading path. auto transitioningVersion = setTargetVersion && serverGlobalParams.featureCompatibility.isUpgradingOrDowngrading(fromVersion) && - !(fromVersion == GenericFCV::kDowngradingFromLatestToLastLTS && - newVersion == GenericFCV::kLatest) + !(repl::feature_flags::gDowngradingToUpgrading.isEnabledAndIgnoreFCV() && + (fromVersion == GenericFCV::kDowngradingFromLatestToLastLTS && + newVersion == GenericFCV::kLatest)) ? fromVersion : fcvTransitions.getTransitionalVersion(fromVersion, newVersion, isFromConfigServer); FeatureCompatibilityVersionDocument fcvDoc = @@ -487,6 +498,10 @@ void FeatureCompatibilityVersion::fassertInitializedAfterStartup(OperationContex auto fcvDocument = findFeatureCompatibilityVersionDocument(opCtx); + // TODO SERVER-65269: Move downgrading->upgrading transition back to FCVTransitions constructor. + // Adding the new fcv downgrading -> upgrading path + fcvTransitions.featureFlaggedAddNewTransitionState(); + auto const storageEngine = opCtx->getServiceContext()->getStorageEngine(); auto dbNames = storageEngine->listDatabases(); bool nonLocalDatabases = std::any_of(dbNames.begin(), dbNames.end(), [](auto dbName) { diff --git a/src/mongo/db/repl/repl_server_parameters.idl b/src/mongo/db/repl/repl_server_parameters.idl index d823889e66e..16ae22f7a2a 100644 --- a/src/mongo/db/repl/repl_server_parameters.idl +++ b/src/mongo/db/repl/repl_server_parameters.idl @@ -677,4 +677,5 @@ feature_flags: description: When enabled, allow kDowngradingFromLatestToLastLTS -> kUpgrading -> kUpgraded path. cpp_varname: feature_flags::gDowngradingToUpgrading - default: false \ No newline at end of file + default: false + \ No newline at end of file -- cgit v1.2.1