SERVER-79274 Fix race where FCV is uninitialized in between FCV checks

Crescentek · Nov 8, 2023 · a5da627 · a5da627
1 parent 60a2f25
commit a5da627
Show file tree

Hide file tree

Showing 157 changed files with 583 additions and 426 deletions.
diff --git a/buildscripts/linter/mongolint.py b/buildscripts/linter/mongolint.py
@@ -20,6 +20,7 @@
     r'::kDowngradingFromLatestToLastLTS',
     r'::kDowngradingFromLatestToLastContinuous',
     r'\.isUpgradingOrDowngrading',
+    r'->isUpgradingOrDowngrading',
     r'::kDowngradingFromLatestToLastContinuous',
     r'::kUpgradingFromLastLTSToLastContinuous',
 ]

diff --git a/src/mongo/db/catalog/coll_mod.cpp b/src/mongo/db/catalog/coll_mod.cpp
@@ -297,7 +297,7 @@ StatusWith<std::pair<ParsedCollModRequest, BSONObj>> parseCollModRequest(
             }
             if (coll->isCapped() &&
                 !feature_flags::gFeatureFlagTTLIndexesOnCappedCollections.isEnabled(
-                    serverGlobalParams.featureCompatibility)) {
+                    serverGlobalParams.featureCompatibility.acquireFCVSnapshot())) {
                 return {ErrorCodes::InvalidOptions,
                         "TTL indexes are not supported for capped collections."};
             }
@@ -502,8 +502,8 @@ StatusWith<std::pair<ParsedCollModRequest, BSONObj>> parseCollModRequest(
         // (Generic FCV reference): This FCV check should exist across LTS binary versions.
         multiversion::FeatureCompatibilityVersion fcv;
         if (serverGlobalParams.validateFeaturesAsPrimary.load() &&
-            serverGlobalParams.featureCompatibility.isLessThan(multiversion::GenericFCV::kLatest,
-                                                               &fcv)) {
+            serverGlobalParams.featureCompatibility.acquireFCVSnapshot().isLessThan(
+                multiversion::GenericFCV::kLatest, &fcv)) {
             maxFeatureCompatibilityVersion = fcv;
         }
         auto validatorObj = *validator;
@@ -980,7 +980,7 @@ Status _collModInternal(OperationContext* opCtx,
             if (changed) {
                 coll.getWritableCollection(opCtx)->setTimeseriesOptions(opCtx, newOptions);
                 if (feature_flags::gTSBucketingParametersUnchanged.isEnabled(
-                        serverGlobalParams.featureCompatibility)) {
+                        serverGlobalParams.featureCompatibility.acquireFCVSnapshot())) {
                     coll.getWritableCollection(opCtx)->setTimeseriesBucketingParametersChanged(
                         opCtx, true);
                 };
@@ -992,8 +992,9 @@ Status _collModInternal(OperationContext* opCtx,
         // (Generic FCV reference): This FCV check happens whenever we upgrade to the latest
         // version.
         // TODO SERVER-80490: remove this check when 8.0 becomes the next LTS release.
-        if (auto version = serverGlobalParams.featureCompatibility.getVersion();
-            cmrNew.numModifications == 0 &&
+        const auto version =
+            serverGlobalParams.featureCompatibility.acquireFCVSnapshot().getVersion();
+        if (cmrNew.numModifications == 0 &&
             (version == multiversion::GenericFCV::kUpgradingFromLastContinuousToLatest ||
              version == multiversion::GenericFCV::kUpgradingFromLastLTSToLatest)) {
             auto writableCollection = coll.getWritableCollection(opCtx);
@@ -1007,8 +1008,7 @@ Status _collModInternal(OperationContext* opCtx,
         // (Generic FCV reference): This FCV check should exist across LTS binary versions.
         // TODO SERVER-80003 remove special version handling when LTS becomes 8.0.
         if (cmrNew.numModifications == 0 && coll->timeseriesBucketingParametersHaveChanged() &&
-            serverGlobalParams.featureCompatibility.getVersion() ==
-                multiversion::GenericFCV::kDowngradingFromLatestToLastLTS) {
+            version == multiversion::GenericFCV::kDowngradingFromLatestToLastLTS) {
             coll.getWritableCollection(opCtx)->setTimeseriesBucketingParametersChanged(opCtx,
                                                                                        boost::none);
         }

diff --git a/src/mongo/db/catalog/collection_options_test.cpp b/src/mongo/db/catalog/collection_options_test.cpp
@@ -113,7 +113,7 @@ TEST(CollectionOptions, ErrorBadMax) {
 }
 
 TEST(CollectionOptions, CappedSizeNotRoundUpForAlignment) {
-    serverGlobalParams.mutableFeatureCompatibility.setVersion(
+    serverGlobalParams.mutableFCV.setVersion(
         multiversion::FeatureCompatibilityVersion::kVersion_6_2);
     const long long kUnalignedCappedSize = 1000;
     const long long kAlignedCappedSize = 1000;

diff --git a/src/mongo/db/catalog/database_impl.cpp b/src/mongo/db/catalog/database_impl.cpp
@@ -948,8 +948,8 @@ Status DatabaseImpl::userCreateNS(OperationContext* opCtx,
         // (Generic FCV reference): This FCV check should exist across LTS binary versions.
         multiversion::FeatureCompatibilityVersion fcv;
         if (serverGlobalParams.validateFeaturesAsPrimary.load() &&
-            serverGlobalParams.featureCompatibility.isLessThan(multiversion::GenericFCV::kLatest,
-                                                               &fcv)) {
+            serverGlobalParams.featureCompatibility.acquireFCVSnapshot().isLessThan(
+                multiversion::GenericFCV::kLatest, &fcv)) {
             expCtx->maxFeatureCompatibilityVersion = fcv;
         }
 

diff --git a/src/mongo/db/catalog/drop_indexes.cpp b/src/mongo/db/catalog/drop_indexes.cpp
@@ -355,7 +355,7 @@ void dropReadyIndexes(OperationContext* opCtx,
                     const auto& shardKey = collDescription.getShardKeyPattern();
                     const bool skipDropIndex = skipDroppingHashedShardKeyIndex ||
                         !(gFeatureFlagShardKeyIndexOptionalHashedSharding.isEnabled(
-                              serverGlobalParams.featureCompatibility) &&
+                              serverGlobalParams.featureCompatibility.acquireFCVSnapshot()) &&
                           shardKey.isHashedPattern());
                     if (isCompatibleWithShardKey(opCtx,
                                                  CollectionPtr(collection),

diff --git a/src/mongo/db/catalog/index_build_block.cpp b/src/mongo/db/catalog/index_build_block.cpp
@@ -275,7 +275,7 @@ void IndexBuildBlock::success(OperationContext* opCtx, Collection* collection) {
             // collection scan, which does not use an index.
             if (spec.hasField(IndexDescriptor::kExpireAfterSecondsFieldName) &&
                 (feature_flags::gFeatureFlagTTLIndexesOnCappedCollections.isEnabled(
-                     serverGlobalParams.featureCompatibility) ||
+                     serverGlobalParams.featureCompatibility.acquireFCVSnapshot()) ||
                  !coll->isCapped())) {
                 auto validateStatus = index_key_validate::validateExpireAfterSeconds(
                     spec[IndexDescriptor::kExpireAfterSecondsFieldName],

diff --git a/src/mongo/db/catalog/index_catalog_impl.cpp b/src/mongo/db/catalog/index_catalog_impl.cpp
@@ -275,7 +275,7 @@ void IndexCatalogImpl::init(OperationContext* opCtx,
             // Note that TTL deletion is supported on capped clustered collections via bounded
             // collection scan, which does not use an index.
             if (feature_flags::gFeatureFlagTTLIndexesOnCappedCollections.isEnabled(
-                    serverGlobalParams.featureCompatibility) ||
+                    serverGlobalParams.featureCompatibility.acquireFCVSnapshot()) ||
                 !collection->isCapped()) {
                 if (opCtx->lockState()->inAWriteUnitOfWork()) {
                     opCtx->recoveryUnit()->onCommit(
@@ -519,7 +519,7 @@ StatusWith<BSONObj> IndexCatalogImpl::prepareSpecForCreate(
     if (collection && collection->isCapped() &&
         validatedSpec.hasField(IndexDescriptor::kExpireAfterSecondsFieldName) &&
         !feature_flags::gFeatureFlagTTLIndexesOnCappedCollections.isEnabled(
-            serverGlobalParams.featureCompatibility) &&
+            serverGlobalParams.featureCompatibility.acquireFCVSnapshot()) &&
         MONGO_likely(!ignoreTTLIndexCappedCollectionCheck.shouldFail())) {
         return {ErrorCodes::CannotCreateIndex, "Cannot create TTL index on a capped collection"};
     }
@@ -975,15 +975,15 @@ Status IndexCatalogImpl::_isSpecOk(OperationContext* opCtx,
             return wildcardSpecStatus;
         }
     } else if (pluginName == IndexNames::COLUMN) {
+        const auto fcvSnapshot = serverGlobalParams.featureCompatibility.acquireFCVSnapshot();
         uassert(ErrorCodes::NotImplemented,
                 str::stream() << pluginName
                               << " indexes are under development and cannot be used without "
                                  "enabling the feature flag",
                 // With our testing failpoint we may try to run this code before we've initialized
                 // the FCV.
-                !serverGlobalParams.featureCompatibility.isVersionInitialized() ||
-                    feature_flags::gFeatureFlagColumnstoreIndexes.isEnabled(
-                        serverGlobalParams.featureCompatibility));
+                !fcvSnapshot.isVersionInitialized() ||
+                    feature_flags::gFeatureFlagColumnstoreIndexes.isEnabled(fcvSnapshot));
         if (auto columnSpecStatus = validateColumnStoreSpec(collection, spec, indexVersion);
             !columnSpecStatus.isOK()) {
             return columnSpecStatus;
@@ -1740,7 +1740,7 @@ Status IndexCatalogImpl::_updateRecord(OperationContext* const opCtx,
     // index has incorrect keys. Replace this failpoint with a test command instead.
     if (auto failpoint = skipUpdatingIndexDocument.scoped(); MONGO_unlikely(failpoint.isActive()) &&
         repl::feature_flags::gSecondaryIndexChecksInDbCheck.isEnabled(
-            serverGlobalParams.featureCompatibility)) {
+            serverGlobalParams.featureCompatibility.acquireFCVSnapshot())) {
         auto indexName = failpoint.getData()["indexName"].valueStringDataSafe();
         if (indexName == index->descriptor()->indexName()) {
             LOGV2_DEBUG(

diff --git a/src/mongo/db/catalog/multi_index_block.cpp b/src/mongo/db/catalog/multi_index_block.cpp
@@ -169,7 +169,7 @@ auto makeOnSuppressedErrorFn(const std::function<void()>& saveCursorBeforeWrite,
 
 bool shouldRelaxConstraints(OperationContext* opCtx, const CollectionPtr& collection) {
     if (!feature_flags::gIndexBuildGracefulErrorHandling.isEnabled(
-            serverGlobalParams.featureCompatibility)) {
+            serverGlobalParams.featureCompatibility.acquireFCVSnapshot())) {
         // Always suppress.
         return true;
     }

diff --git a/src/mongo/db/catalog/validate_state.cpp b/src/mongo/db/catalog/validate_state.cpp
@@ -136,7 +136,7 @@ ValidateState::ValidateState(OperationContext* opCtx,
     if (additionalOptions.enforceTimeseriesBucketsAreAlwaysCompressed) {
         if (TestingProctor::instance().isEnabled() &&
             feature_flags::gTimeseriesAlwaysUseCompressedBuckets.isEnabled(
-                serverGlobalParams.featureCompatibility)) {
+                serverGlobalParams.featureCompatibility.acquireFCVSnapshot())) {
             _enforceTimeseriesBucketsAreAlwaysCompressed = true;
         } else {
             LOGV2_WARNING(7735102, "Not enforcing that time-series buckets are always compressed");

diff --git a/src/mongo/db/change_stream_pre_images_collection_manager.cpp b/src/mongo/db/change_stream_pre_images_collection_manager.cpp
@@ -138,7 +138,7 @@ std::unique_ptr<PlanExecutor, PlanExecutor::Deleter> getDeleteExpiredPreImagesEx
 
 bool useUnreplicatedTruncates() {
     bool res = feature_flags::gFeatureFlagUseUnreplicatedTruncatesForDeletions.isEnabled(
-        serverGlobalParams.featureCompatibility);
+        serverGlobalParams.featureCompatibility.acquireFCVSnapshot());
     return res;
 }
 }  // namespace

diff --git a/src/mongo/db/change_stream_serverless_helpers.cpp b/src/mongo/db/change_stream_serverless_helpers.cpp
@@ -68,7 +68,7 @@ namespace change_stream_serverless_helpers {
 namespace {
 bool isServerlessChangeStreamFeatureFlagEnabled() {
     return feature_flags::gFeatureFlagServerlessChangeStreams.isEnabled(
-        serverGlobalParams.featureCompatibility);
+        serverGlobalParams.featureCompatibility.acquireFCVSnapshot());
 }
 }  // namespace
 

diff --git a/src/mongo/db/cloner.cpp b/src/mongo/db/cloner.cpp
@@ -371,7 +371,7 @@ StatusWith<std::vector<BSONObj>> DefaultClonerImpl::_filterCollectionsForClone(
 
         const auto nss = NamespaceStringUtil::deserialize(fromDBName, collectionName.c_str());
         if (nss.isSystem()) {
-            if (!nss.isLegalClientSystemNS(serverGlobalParams.featureCompatibility)) {
+            if (!nss.isLegalClientSystemNS()) {
                 LOGV2_DEBUG(20419, 2, "\t\t not cloning because system collection");
                 continue;
             }

diff --git a/src/mongo/db/commands/analyze_cmd.cpp b/src/mongo/db/commands/analyze_cmd.cpp
@@ -165,7 +165,7 @@ class CmdAnalyze final : public TypedCommand<CmdAnalyze> {
             uassert(6660400,
                     "Analyze command requires common query framework feature flag to be enabled",
                     feature_flags::gFeatureFlagCommonQueryFramework.isEnabled(
-                        serverGlobalParams.featureCompatibility));
+                        serverGlobalParams.featureCompatibility.acquireFCVSnapshot()));
 
             const auto& cmd = request();
             const NamespaceString& nss = ns();

diff --git a/src/mongo/db/commands/bulk_write.cpp b/src/mongo/db/commands/bulk_write.cpp
@@ -1262,10 +1262,10 @@ class BulkWriteCmd : public BulkWriteCmdVersion1Gen<BulkWriteCmd> {
                    const Command* command,
                    const OpMsgRequest& opMsgRequest)
             : InvocationBaseGen(opCtx, command, opMsgRequest), _commandObj(opMsgRequest.body) {
-            uassert(
-                ErrorCodes::CommandNotSupported,
-                "BulkWrite may not be run without featureFlagBulkWriteCommand enabled",
-                gFeatureFlagBulkWriteCommand.isEnabled(serverGlobalParams.featureCompatibility));
+            uassert(ErrorCodes::CommandNotSupported,
+                    "BulkWrite may not be run without featureFlagBulkWriteCommand enabled",
+                    gFeatureFlagBulkWriteCommand.isEnabled(
+                        serverGlobalParams.featureCompatibility.acquireFCVSnapshot()));
 
             bulk_write_common::validateRequest(request(), /*isRouter=*/false);
 

diff --git a/src/mongo/db/commands/create_indexes_cmd.cpp b/src/mongo/db/commands/create_indexes_cmd.cpp
@@ -562,7 +562,7 @@ CreateIndexesReply runCreateIndexesWithCoordinator(OperationContext* opCtx,
             !opCtx->inMultiDocumentTransaction());
 
     if (feature_flags::gIndexBuildGracefulErrorHandling.isEnabled(
-            serverGlobalParams.featureCompatibility)) {
+            serverGlobalParams.featureCompatibility.acquireFCVSnapshot())) {
         uassertStatusOK(IndexBuildsCoordinator::checkDiskSpaceSufficientToStartIndexBuild(opCtx));
     }
 

diff --git a/src/mongo/db/commands/dbcheck_command.cpp b/src/mongo/db/commands/dbcheck_command.cpp
@@ -230,7 +230,7 @@ std::unique_ptr<DbCheckRun> singleCollectionRun(OperationContext* opCtx,
                                                 const DbCheckSingleInvocation& invocation) {
     const auto gSecondaryIndexChecksInDbCheck =
         repl::feature_flags::gSecondaryIndexChecksInDbCheck.isEnabled(
-            serverGlobalParams.featureCompatibility);
+            serverGlobalParams.featureCompatibility.acquireFCVSnapshot());
     if (!gSecondaryIndexChecksInDbCheck) {
         uassert(ErrorCodes::InvalidOptions,
                 "When featureFlagSecondaryIndexChecksInDbCheck is not enabled, the validateMode "