From 8703ef72272bcb912f7f3763040e8db100913422 Mon Sep 17 00:00:00 2001 From: Gregory Wlodarek Date: Wed, 5 Jan 2022 18:23:22 +0000 Subject: SERVER-60911 Remove FCV references for secondary indexes on time-series measurements for kLastContinuous --- ...e_timeseries_collection_from_last_continuous.js | 98 +++++++--------------- .../timeseries_measurement_indexes_downgrade.js | 25 +++--- src/mongo/db/catalog/coll_mod.cpp | 29 +++---- .../set_feature_compatibility_version_command.cpp | 54 +++++++----- src/mongo/db/storage/durable_catalog_impl.cpp | 13 +-- 5 files changed, 100 insertions(+), 119 deletions(-) diff --git a/jstests/multiVersion/upgrade_downgrade_timeseries_collection_from_last_continuous.js b/jstests/multiVersion/upgrade_downgrade_timeseries_collection_from_last_continuous.js index d4e9039a5de..7cc10f5080e 100644 --- a/jstests/multiVersion/upgrade_downgrade_timeseries_collection_from_last_continuous.js +++ b/jstests/multiVersion/upgrade_downgrade_timeseries_collection_from_last_continuous.js @@ -1,10 +1,6 @@ /** - * Tests that upgrading time-series collections created using the last-continuous binary warns about - * potentially mixed-schema data when building secondary indexes on time-series measurements on the - * latest binary. Additionally, tests that downgrading FCV from 5.2 removes the - * 'timeseriesBucketsMayHaveMixedSchemaData' catalog entry flag from time-series collections. - * - * @tags: [disabled_due_to_server_61671] + * Tests that there are no upgrade or downgrade requirements for secondary indexes on time-series + * measurements between kLastContinuous and kLatest. */ (function() { "use strict"; @@ -14,8 +10,8 @@ load("jstests/multiVersion/libs/multi_rs.js"); const oldVersion = "last-continuous"; const nodes = { - n1: {binVersion: oldVersion}, - n2: {binVersion: oldVersion} + n1: {binVersion: oldVersion, setParameter: 'featureFlagTimeseriesMetricIndexes=true'}, + n2: {binVersion: oldVersion, setParameter: 'featureFlagTimeseriesMetricIndexes=true'} }; const rst = new ReplSetTest({nodes: nodes}); @@ -26,6 +22,7 @@ const dbName = "test"; const collName = jsTestName(); let primary = rst.getPrimary(); +let secondary = rst.getSecondary(); let db = primary.getDB(dbName); let coll = db.getCollection(collName); @@ -39,87 +36,52 @@ assert.commandWorked(coll.insert({[timeField]: ISODate(), [metaField]: 1, x: 1}) assert.commandWorked(coll.insert({[timeField]: ISODate(), [metaField]: 2, x: {y: "z"}})); assert.commandWorked(coll.insert({[timeField]: ISODate(), [metaField]: 3, x: "abc"})); -jsTest.log("Upgrading replica set from last-continuous to latest"); -rst.upgradeSet({binVersion: "latest", setParameter: {logComponentVerbosity: tojson({storage: 1})}}); - -primary = rst.getPrimary(); -db = primary.getDB(dbName); -coll = db.getCollection(collName); - -if (!TimeseriesTest.timeseriesMetricIndexesEnabled(primary)) { - jsTest.log("Skipping test as the featureFlagTimeseriesMetricIndexes feature flag is disabled"); - rst.stopSet(); - return; -} - -// Building indexes on time-series measurements is only supported in FCV >= 5.2. -jsTest.log("Setting FCV to 'latestFCV'"); -assert.commandWorked(primary.adminCommand({setFeatureCompatibilityVersion: latestFCV})); - const bucketCollName = dbName + ".system.buckets." + collName; -// The FCV upgrade process adds the catalog entry flag to time-series collections. -const secondary = rst.getSecondary(); -assert(checkLog.checkContainsWithCountJson(primary, 6057601, {setting: true}, /*expectedCount=*/1)); -assert( - checkLog.checkContainsWithCountJson(secondary, 6057601, {setting: true}, /*expectedCount=*/1)); - assert.commandWorked(coll.createIndex({[timeField]: 1}, {name: "time_1"})); -assert(checkLog.checkContainsWithCountJson( - primary, 6057502, {namespace: bucketCollName}, /*expectedCount=*/0)); - assert.commandWorked(coll.createIndex({[metaField]: 1}, {name: "meta_1"})); -assert(checkLog.checkContainsWithCountJson( - primary, 6057502, {namespace: bucketCollName}, /*expectedCount=*/0)); - assert.commandWorked(db.getCollection(collName).createIndex({x: 1}, {name: "x_1"})); -// May have mixed-schema data. -assert(checkLog.checkContainsWithCountJson( - primary, 6057502, {namespace: bucketCollName}, /*expectedCount=*/1)); - // No mixed-schema data detected. +assert(checkLog.checkContainsWithCountJson( + primary, 6057502, {namespace: bucketCollName}, /*expectedCount=*/0)); assert(checkLog.checkContainsWithCountJson( primary, 6057700, {namespace: bucketCollName}, /*expectedCount=*/0)); -// Catalog entry flag gets set to false. +// Catalog entry flag does not get set to false. It is already false. assert( - checkLog.checkContainsWithCountJson(primary, 6057601, {setting: false}, /*expectedCount=*/1)); + checkLog.checkContainsWithCountJson(primary, 6057601, {setting: false}, /*expectedCount=*/0)); assert( - checkLog.checkContainsWithCountJson(secondary, 6057601, {setting: false}, /*expectedCount=*/1)); + checkLog.checkContainsWithCountJson(secondary, 6057601, {setting: false}, /*expectedCount=*/0)); -// After successfully building an index on a time-series measurement, subsequent index builds on -// time-series measurements will skip checking for mixed-schema data. -assert.commandWorked( - db.getCollection(collName).createIndex({[timeField]: 1, x: 1}, {name: "time_1_x_1"})); +jsTest.log("Upgrading replica set from last-continuous to latest"); +rst.upgradeSet({binVersion: "latest", setParameter: {logComponentVerbosity: tojson({storage: 1})}}); -// Check that the log message warning about potential mixed-schema data does not get logged again. -assert(checkLog.checkContainsWithCountJson( - primary, 6057502, {namespace: bucketCollName}, /*expectedCount=*/1)); +primary = rst.getPrimary(); +secondary = rst.getSecondary(); +db = primary.getDB(dbName); +coll = db.getCollection(collName); -// No mixed-schema data detected. -assert(checkLog.checkContainsWithCountJson( - primary, 6057700, {namespace: bucketCollName}, /*expectedCount=*/0)); +if (!TimeseriesTest.timeseriesMetricIndexesEnabled(primary)) { + jsTest.log("Skipping test as the featureFlagTimeseriesMetricIndexes feature flag is disabled"); + rst.stopSet(); + return; +} -// Catalog entry flag should still be set to false, but not again. -assert( - checkLog.checkContainsWithCountJson(primary, 6057601, {setting: false}, /*expectedCount=*/1)); -assert( - checkLog.checkContainsWithCountJson(secondary, 6057601, {setting: false}, /*expectedCount=*/1)); +jsTest.log("Setting FCV to 'latestFCV'"); +assert.commandWorked(primary.adminCommand({setFeatureCompatibilityVersion: latestFCV})); -// Cannot downgrade when there are indexes on time-series measurements present. -assert.commandFailedWithCode( - primary.adminCommand({setFeatureCompatibilityVersion: lastContinuousFCV}), - ErrorCodes.CannotDowngrade); -assert.commandWorked(db.getCollection(collName).dropIndex("x_1")); -assert.commandWorked(db.getCollection(collName).dropIndex("time_1_x_1")); +// The FCV upgrade process does not add the catalog entry flag to time-series collections. +assert(checkLog.checkContainsWithCountJson(primary, 6057601, {setting: true}, /*expectedCount=*/0)); +assert( + checkLog.checkContainsWithCountJson(secondary, 6057601, {setting: true}, /*expectedCount=*/0)); -// The FCV downgrade process removes the catalog entry flag from time-series collections. +// The FCV downgrade process does not remove the catalog entry flag from time-series collections. jsTest.log("Setting FCV to 'lastContinuousFCV'"); assert.commandWorked(primary.adminCommand({setFeatureCompatibilityVersion: lastContinuousFCV})); -assert(checkLog.checkContainsWithCountJson(primary, 6057601, {setting: null}, /*expectedCount=*/1)); +assert(checkLog.checkContainsWithCountJson(primary, 6057601, {setting: null}, /*expectedCount=*/0)); assert( - checkLog.checkContainsWithCountJson(secondary, 6057601, {setting: null}, /*expectedCount=*/1)); + checkLog.checkContainsWithCountJson(secondary, 6057601, {setting: null}, /*expectedCount=*/0)); rst.stopSet(); }()); diff --git a/jstests/noPassthrough/timeseries_measurement_indexes_downgrade.js b/jstests/noPassthrough/timeseries_measurement_indexes_downgrade.js index d1b913832a2..4889af415e6 100644 --- a/jstests/noPassthrough/timeseries_measurement_indexes_downgrade.js +++ b/jstests/noPassthrough/timeseries_measurement_indexes_downgrade.js @@ -37,7 +37,10 @@ function checkIndexForDowngrade(withFCV, isCompatible, createdOnBucketsCollectio const index = bucketsColl.getIndexes()[0]; if (isCompatible) { - assert(!index.hasOwnProperty("originalSpec")); + // All time-series indexes are downgrade compatible to the last continuous FCV as of v5.3. + if (withFCV != lastContinuousFCV) { + assert(!index.hasOwnProperty("originalSpec")); + } } else { if (createdOnBucketsCollection) { // Indexes created directly on the buckets collection do not have the original user @@ -58,8 +61,6 @@ function checkIndexForDowngrade(withFCV, isCompatible, createdOnBucketsCollectio assert.commandWorked(coll.dropIndexes("*")); } -// TODO SERVER-60911: Remove downgrade checks for lastContinuousFCV once kLatest is 5.3. - assert.commandWorked(coll.createIndex({[timeFieldName]: 1})); checkIndexForDowngrade(lastLTSFCV, true, false); @@ -76,31 +77,31 @@ assert.commandWorked(coll.createIndex({[metaFieldName]: 1, a: 1})); checkIndexForDowngrade(lastLTSFCV, false, false); assert.commandWorked(coll.createIndex({[metaFieldName]: 1, a: 1})); -checkIndexForDowngrade(lastContinuousFCV, false, false); +checkIndexForDowngrade(lastContinuousFCV, true, false); assert.commandWorked(coll.createIndex({b: 1})); checkIndexForDowngrade(lastLTSFCV, false, false); assert.commandWorked(coll.createIndex({b: 1})); -checkIndexForDowngrade(lastContinuousFCV, false, false); +checkIndexForDowngrade(lastContinuousFCV, true, false); assert.commandWorked(bucketsColl.createIndex({"control.min.c.d": 1, "control.max.c.d": 1})); checkIndexForDowngrade(lastLTSFCV, false, true); assert.commandWorked(bucketsColl.createIndex({"control.min.c.d": 1, "control.max.c.d": 1})); -checkIndexForDowngrade(lastContinuousFCV, false, true); +checkIndexForDowngrade(lastContinuousFCV, true, true); assert.commandWorked(bucketsColl.createIndex({"control.min.e": 1, "control.min.f": 1})); checkIndexForDowngrade(lastLTSFCV, false, true); assert.commandWorked(bucketsColl.createIndex({"control.min.e": 1, "control.min.f": 1})); -checkIndexForDowngrade(lastContinuousFCV, false, true); +checkIndexForDowngrade(lastContinuousFCV, true, true); assert.commandWorked(coll.createIndex({g: "2dsphere"})); checkIndexForDowngrade(lastLTSFCV, false, false); assert.commandWorked(coll.createIndex({g: "2dsphere"})); -checkIndexForDowngrade(lastContinuousFCV, false, false); +checkIndexForDowngrade(lastContinuousFCV, true, false); assert.commandWorked(coll.createIndex({[metaFieldName]: "2d"})); checkIndexForDowngrade(lastLTSFCV, true, false); @@ -121,7 +122,7 @@ checkIndexForDowngrade(lastLTSFCV, false, false); assert.commandWorked( coll.createIndex({[timeFieldName]: 1}, {partialFilterExpression: {x: {$type: "number"}}})); -checkIndexForDowngrade(lastContinuousFCV, false, false); +checkIndexForDowngrade(lastContinuousFCV, true, false); assert.commandWorked( coll.createIndex({[metaFieldName]: 1}, {partialFilterExpression: {x: {$type: "number"}}})); @@ -129,13 +130,13 @@ checkIndexForDowngrade(lastLTSFCV, false, false); assert.commandWorked( coll.createIndex({[metaFieldName]: 1}, {partialFilterExpression: {x: {$type: "number"}}})); -checkIndexForDowngrade(lastContinuousFCV, false, false); +checkIndexForDowngrade(lastContinuousFCV, true, false); assert.commandWorked(coll.createIndex({x: 1}, {partialFilterExpression: {x: {$type: "number"}}})); checkIndexForDowngrade(lastLTSFCV, false, false); assert.commandWorked(coll.createIndex({x: 1}, {partialFilterExpression: {x: {$type: "number"}}})); -checkIndexForDowngrade(lastContinuousFCV, false, false); +checkIndexForDowngrade(lastContinuousFCV, true, false); assert.commandWorked(coll.createIndex({[metaFieldName]: 1, x: 1}, {partialFilterExpression: {x: {$type: "number"}}})); @@ -143,7 +144,7 @@ checkIndexForDowngrade(lastLTSFCV, false, false); assert.commandWorked(coll.createIndex({x: 1, [metaFieldName]: 1}, {partialFilterExpression: {x: {$type: "number"}}})); -checkIndexForDowngrade(lastContinuousFCV, false, false); +checkIndexForDowngrade(lastContinuousFCV, true, false); MongoRunner.stopMongod(conn); }()); diff --git a/src/mongo/db/catalog/coll_mod.cpp b/src/mongo/db/catalog/coll_mod.cpp index c7a7c3436cd..6a6ce343ddb 100644 --- a/src/mongo/db/catalog/coll_mod.cpp +++ b/src/mongo/db/catalog/coll_mod.cpp @@ -795,24 +795,25 @@ Status _collModInternal(OperationContext* opCtx, opCtx, coll.getWritableCollection(opCtx), desc, CreateIndexEntryFlags::kIsReady); } - // TODO SERVER-60911: When kLatest is 5.3, only check when upgrading from or downgrading to - // kLastLTS (5.0). - // TODO SERVER-60912: When kLastLTS is 6.0, remove this FCV-gated upgrade/downgrade code. + // (Generic FCV reference): TODO SERVER-60912: When kLastLTS is 6.0, remove this FCV-gated + // upgrade/downgrade code. + const auto currentVersion = serverGlobalParams.featureCompatibility.getVersion(); if (coll->getTimeseriesOptions() && !coll->getTimeseriesBucketsMayHaveMixedSchemaData() && - serverGlobalParams.featureCompatibility.isFCVUpgradingToOrAlreadyLatest()) { - // While upgrading the FCV to 5.2+, collMod is called as part of the upgrade process to - // add the 'timeseriesBucketsMayHaveMixedSchemaData=true' catalog entry flag for - // time-series collections that are missing the flag. This indicates that the - // time-series collection existed in earlier server versions and may have mixed-schema - // data. + (currentVersion == multiversion::GenericFCV::kUpgradingFromLastLTSToLatest || + currentVersion == multiversion::GenericFCV::kLatest)) { + // (Generic FCV reference): While upgrading the FCV from kLastLTS to kLatest, collMod is + // called as part of the upgrade process to add the + // 'timeseriesBucketsMayHaveMixedSchemaData=true' catalog entry flag for time-series + // collections that are missing the flag. This indicates that the time-series collection + // existed in earlier server versions and may have mixed-schema data. coll.getWritableCollection(opCtx)->setTimeseriesBucketsMayHaveMixedSchemaData(opCtx, true); } else if (coll->getTimeseriesBucketsMayHaveMixedSchemaData() && - serverGlobalParams.featureCompatibility - .isFCVDowngradingOrAlreadyDowngradedFromLatest()) { - // While downgrading the FCV from 5.2, collMod is called as part of the downgrade - // process to remove the 'timeseriesBucketsMayHaveMixedSchemaData' catalog entry - // flag for time-series collections that have the flag. + (currentVersion == multiversion::GenericFCV::kDowngradingFromLatestToLastLTS || + currentVersion == multiversion::GenericFCV::kLastLTS)) { + // (Generic FCV reference): While downgrading the FCV to kLastLTS, collMod is called as + // part of the downgrade process to remove the 'timeseriesBucketsMayHaveMixedSchemaData' + // catalog entry flag for time-series collections that have the flag. coll.getWritableCollection(opCtx)->setTimeseriesBucketsMayHaveMixedSchemaData( opCtx, boost::none); } diff --git a/src/mongo/db/commands/set_feature_compatibility_version_command.cpp b/src/mongo/db/commands/set_feature_compatibility_version_command.cpp index 828d1857c8a..117137a7c79 100644 --- a/src/mongo/db/commands/set_feature_compatibility_version_command.cpp +++ b/src/mongo/db/commands/set_feature_compatibility_version_command.cpp @@ -426,9 +426,9 @@ private: Lock::GlobalLock lk(opCtx, MODE_S); } - // TODO SERVER-60911: When kLatest is 5.3, only check when upgrading from kLastLTS (5.0). - // TODO SERVER-60912: When kLastLTS is 6.0, remove this FCV-gated upgrade code. - if (serverGlobalParams.featureCompatibility.isFCVUpgradingToOrAlreadyLatest()) { + // (Generic FCV reference): TODO SERVER-60912: When kLastLTS is 6.0, remove this FCV-gated + // upgrade code. + if (requestedVersion == multiversion::GenericFCV::kLatest) { for (const auto& dbName : DatabaseHolder::get(opCtx)->getNames()) { Lock::DBLock dbLock(opCtx, dbName, MODE_IX); catalog::forEachCollectionFromDb( @@ -539,10 +539,9 @@ private: Lock::GlobalLock lk(opCtx, MODE_S); } - // TODO SERVER-60911: When kLatest is 5.3, only check when downgrading to kLastLTS (5.0). - // TODO SERVER-60912: When kLastLTS is 6.0, remove this FCV-gated downgrade code. - if (serverGlobalParams.featureCompatibility - .isFCVDowngradingOrAlreadyDowngradedFromLatest()) { + // (Generic FCV reference): TODO SERVER-60912: When kLastLTS is 6.0, remove this FCV-gated + // downgrade code. + if (requestedVersion == multiversion::GenericFCV::kLastLTS) { for (const auto& dbName : DatabaseHolder::get(opCtx)->getNames()) { Lock::DBLock dbLock(opCtx, dbName, MODE_IX); catalog::forEachCollectionFromDb( @@ -550,16 +549,6 @@ private: dbName, MODE_X, [&](const CollectionPtr& collection) { - // Fail to downgrade if there exists a collection with - // 'changeStreamPreAndPostImages' enabled. - // TODO SERVER-61770: Remove once FCV 6.0 becomes last-lts. - uassert(ErrorCodes::CannotDowngrade, - str::stream() << "Cannot downgrade the cluster as collection " - << collection->ns() - << " has 'changeStreamPreAndPostImages' enabled", - preImagesFeatureFlagDisabledOnDowngradeVersion && - !collection->isChangeStreamPreAndPostImagesEnabled()); - invariant(collection->getTimeseriesOptions()); auto indexCatalog = collection->getIndexCatalog(); @@ -630,12 +619,37 @@ private: return true; }, + [&](const CollectionPtr& collection) { + return collection->getTimeseriesOptions() != boost::none; + }); + } + } + + if (serverGlobalParams.featureCompatibility + .isFCVDowngradingOrAlreadyDowngradedFromLatest()) { + for (const auto& dbName : DatabaseHolder::get(opCtx)->getNames()) { + Lock::DBLock dbLock(opCtx, dbName, MODE_IX); + catalog::forEachCollectionFromDb( + opCtx, + dbName, + MODE_X, + [&](const CollectionPtr& collection) { + // Fail to downgrade if there exists a collection with + // 'changeStreamPreAndPostImages' enabled. + // TODO SERVER-61770: Remove once FCV 6.0 becomes last-lts. + uassert(ErrorCodes::CannotDowngrade, + str::stream() << "Cannot downgrade the cluster as collection " + << collection->ns() + << " has 'changeStreamPreAndPostImages' enabled", + preImagesFeatureFlagDisabledOnDowngradeVersion && + !collection->isChangeStreamPreAndPostImagesEnabled()); + return true; + }, [&](const CollectionPtr& collection) { // TODO SERVER-61770: Remove 'changeStreamPreAndPostImages' check once // FCV 6.0 becomes last-lts. - return collection->getTimeseriesOptions() != boost::none || - (preImagesFeatureFlagDisabledOnDowngradeVersion && - collection->isChangeStreamPreAndPostImagesEnabled()); + return preImagesFeatureFlagDisabledOnDowngradeVersion && + collection->isChangeStreamPreAndPostImagesEnabled(); }); } diff --git a/src/mongo/db/storage/durable_catalog_impl.cpp b/src/mongo/db/storage/durable_catalog_impl.cpp index bfb9aa8b009..57a91d32454 100644 --- a/src/mongo/db/storage/durable_catalog_impl.cpp +++ b/src/mongo/db/storage/durable_catalog_impl.cpp @@ -339,14 +339,17 @@ StatusWith DurableCatalogImpl::_addEntry(OperationContext md.ns = nss.ns(); md.options = options; - // TODO SERVER-60911: When kLatest is 5.3, only check when upgrading from kLastLTS (5.0). - // TODO SERVER-60912: When kLastLTS is 6.0, remove this FCV-gated upgrade code. + // (Generic FCV reference): TODO SERVER-60912: When kLastLTS is 6.0, remove this FCV-gated + // upgrade code. if (options.timeseries && - serverGlobalParams.featureCompatibility.isFCVUpgradingToOrAlreadyLatest()) { - // When the server has begun upgrading FCV to 5.2, all newly created catalog entries for + (serverGlobalParams.featureCompatibility.getVersion() == + multiversion::GenericFCV::kUpgradingFromLastLTSToLatest || + serverGlobalParams.featureCompatibility.getVersion() == + multiversion::GenericFCV::kLatest)) { + // When upgrading FCV from kLastLTS to kLatest, all newly created catalog entries for // time-series collections will have this flag set to false by default as mixed-schema // data is only possible in versions 5.1 and earlier. We do not have to wait for FCV to - // be fully upgraded to 5.2 to start this process. + // be fully upgraded to start this process. md.timeseriesBucketsMayHaveMixedSchemaData = false; } b.append("md", md.toBSON()); -- cgit v1.2.1