diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 73acab1db76..5de3b5f1a24 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1208,6 +1208,7 @@ WORKSPACE.bazel @10gen/devprod-build @svc-auto-approve-bot /jstests/multiVersion/genericSetFCVUsage/**/cluster_parameters_disabled_correctly_after_downgrade.js @10gen/server-catalog-and-routing-routing-and-topology @svc-auto-approve-bot /jstests/multiVersion/genericSetFCVUsage/**/priority_port_downgrade_checks.js @10gen/server-catalog-and-routing-routing-and-topology @svc-auto-approve-bot /jstests/multiVersion/genericSetFCVUsage/**/*upgrade_downgrade_viewless_timeseries_*.js @10gen/server-catalog-and-routing-ddl @svc-auto-approve-bot +/jstests/multiVersion/genericSetFCVUsage/**/s2_index_v4_downgrade.js @10gen/query-integration-features @svc-auto-approve-bot # The following patterns are parsed from ./jstests/multiVersion/genericSetFCVUsage/collection_write_path/OWNERS.yml /jstests/multiVersion/genericSetFCVUsage/collection_write_path/**/* @10gen/server-collection-write-path @svc-auto-approve-bot diff --git a/etc/evergreen_yml_components/tasks/resmoke/server_divisions/query/tasks.yml b/etc/evergreen_yml_components/tasks/resmoke/server_divisions/query/tasks.yml index 00eb4103a3e..07734a636ea 100644 --- a/etc/evergreen_yml_components/tasks/resmoke/server_divisions/query/tasks.yml +++ b/etc/evergreen_yml_components/tasks/resmoke/server_divisions/query/tasks.yml @@ -1952,7 +1952,7 @@ tasks: resmoke_args: >- --mongodSetParameters='{logComponentVerbosity: {command: 2}}' --runNoFeatureFlagTests - jstestfuzz_vars: --metaSeed 1726779665485 --jstestfuzzGitRev b25ed7c + jstestfuzz_vars: --metaSeed 1726779665485 --jstestfuzzGitRev d4c83dd # jstestfuzz standalone update time-series generational fuzzer ## - <<: *jstestfuzz_template diff --git a/jstests/change_streams/ddl_create_drop_index_events.js b/jstests/change_streams/ddl_create_drop_index_events.js index a6c12559509..bdfd722ad4c 100644 --- a/jstests/change_streams/ddl_create_drop_index_events.js +++ b/jstests/change_streams/ddl_create_drop_index_events.js @@ -8,6 +8,8 @@ * ] */ import {ChangeStreamTest} from "jstests/libs/query/change_stream_util.js"; +import {FeatureFlagUtil} from "jstests/libs/feature_flag_util.js"; +import {isStableFCVSuite} from "jstests/libs/feature_compatibility_version.js"; const testDB = db.getSiblingDB(jsTestName()); @@ -112,7 +114,17 @@ function runTest(startChangeStream, pipeline, insertDataBeforeCreateIndex) { testCreateIndexAndDropIndex({f: "2d"}, options); // Test createIndex() for a 2dsphere index with various options, followed by dropIndex(). - options = {name: "2dsphere", "2dsphereIndexVersion": 3}; + // The 2dsphere index version depends on whether featureFlag2dsphereIndexVersion4 is enabled: + // - If enabled: defaults to version 4, which is included in change stream events. + // - If disabled: defaults to version 3, which is included in change stream events. + options = {name: "2dsphere"}; + var twoDSphereIndexVersion = FeatureFlagUtil.isPresentAndEnabled(db, "2dsphereIndexVersion4") ? 4 : 3; + if (!isStableFCVSuite()) { + // If we are upgrading/downgrading the FCV, avoid having to drop any v4 indexes by pinning the version to 3 + // TODO SERVER-118561 Remove this when 9.0 is last LTS. + twoDSphereIndexVersion = 3; + } + options["2dsphereIndexVersion"] = twoDSphereIndexVersion; testCreateIndexAndDropIndex({f: "2dsphere"}, options); // Test createIndexes() to create two sparse indexes (with one index being a compound index), diff --git a/jstests/core/catalog/list_catalog_stage_consistency.js b/jstests/core/catalog/list_catalog_stage_consistency.js index ab4196d1e1d..80abfab4869 100644 --- a/jstests/core/catalog/list_catalog_stage_consistency.js +++ b/jstests/core/catalog/list_catalog_stage_consistency.js @@ -16,6 +16,8 @@ import { getTimeseriesBucketsColl, } from "jstests/core/timeseries/libs/viewless_timeseries_util.js"; import {assertCatalogListOperationsConsistencyForCollection} from "jstests/libs/catalog_list_operations_consistency_validator.js"; +import {FeatureFlagUtil} from "jstests/libs/feature_flag_util.js"; +import {isStableFCVSuite} from "jstests/libs/feature_compatibility_version.js"; // Validate catalog list operations consistency after each command, // so that if an inconsistency is introduced, we fail immediately. @@ -130,13 +132,19 @@ createIndexAndCheckConsistency(db.collection_simple, {f2dNonIntBits: "2d"}, {bit createIndexAndCheckConsistency(db.collection_simple, {f2dSphere: "2dsphere"}); createIndexAndCheckConsistency(db.timeseries_simple, {"timestamp": 1}); +var twoDSphereIndexVersion = FeatureFlagUtil.isPresentAndEnabled(db, "2dsphereIndexVersion4") ? 4 : 3; +if (!isStableFCVSuite()) { + // If we are upgrading/downgrading the FCV, avoid having to drop any v4 indexes by pinning the version to 3 + // TODO SERVER-118561 Remove this when 9.0 is last LTS. + twoDSphereIndexVersion = 3; +} // TODO(SERVER-97084): Remove when options for index plugins are denied in basic indexes. createIndexAndCheckConsistency( db.collection_simple, {fUnrelatedIndexPluginOptions: 1}, { textIndexVersion: 3, - "2dsphereIndexVersion": 3, + "2dsphereIndexVersion": FeatureFlagUtil.isPresentAndEnabled(db, "2dsphereIndexVersion4") ? 4 : 3, bits: 26, min: -180, max: 180, diff --git a/jstests/core/index/geo/geo_s2indexversion1.js b/jstests/core/index/geo/geo_s2indexversion1.js index 6c4fed03297..011e6d4914c 100644 --- a/jstests/core/index/geo/geo_s2indexversion1.js +++ b/jstests/core/index/geo/geo_s2indexversion1.js @@ -7,9 +7,27 @@ // Tests 2dsphere index option "2dsphereIndexVersion". Verifies that GeoJSON objects that are new // in version 2 are not allowed in version 1. +import {FeatureFlagUtil} from "jstests/libs/feature_flag_util.js"; +import {isStableFCVSuite} from "jstests/libs/feature_compatibility_version.js"; + let coll = db.getCollection("geo_s2indexversion1"); coll.drop(); +/** + * Helper function to get the appropriate 2dsphere index version based on feature flag. + * Returns version 4 if the feature flag is enabled and we're in a stable FCV suite. + * Returns version 3 otherwise (to avoid having to drop v4 indexes during FCV transitions). + */ +function get2dsphereIndexVersion() { + const version = FeatureFlagUtil.isPresentAndEnabled(db, "2dsphereIndexVersion4") ? 4 : 3; + if (!isStableFCVSuite()) { + // If we are upgrading/downgrading the FCV, avoid having to drop any v4 indexes by pinning the version to 3. + // TODO SERVER-118561 Remove this when 9.0 is last LTS. + return 3; + } + return version; +} + // // Index build should fail for invalid values of "2dsphereIndexVersion". // @@ -23,7 +41,7 @@ res = coll.createIndex({geo: "2dsphere"}, {"2dsphereIndexVersion": 0}); assert.commandFailed(res); coll.drop(); -res = coll.createIndex({geo: "2dsphere"}, {"2dsphereIndexVersion": 4}); +res = coll.createIndex({geo: "2dsphere"}, {"2dsphereIndexVersion": 5}); assert.commandFailed(res); coll.drop(); @@ -71,16 +89,51 @@ res = coll.createIndex({geo: "2dsphere"}, {"2dsphereIndexVersion": NumberLong(2) assert.commandWorked(res); coll.drop(); -// -// {2dsphereIndexVersion: 3} should be the default for new indexes. -// +res = coll.createIndex({geo: "2dsphere"}, {"2dsphereIndexVersion": 3}); +assert.commandWorked(res); +coll.drop(); -res = coll.createIndex({geo: "2dsphere"}); +res = coll.createIndex({geo: "2dsphere"}, {"2dsphereIndexVersion": NumberInt(3)}); +assert.commandWorked(res); +coll.drop(); + +res = coll.createIndex({geo: "2dsphere"}, {"2dsphereIndexVersion": NumberLong(3)}); +assert.commandWorked(res); +coll.drop(); + +const kDefault2dSphereIndexVersion = get2dsphereIndexVersion(); + +if (kDefault2dSphereIndexVersion == 4) { + res = coll.createIndex({geo: "2dsphere"}, {"2dsphereIndexVersion": 4}); + assert.commandWorked(res); + coll.drop(); + + res = coll.createIndex({geo: "2dsphere"}, {"2dsphereIndexVersion": NumberInt(4)}); + assert.commandWorked(res); + coll.drop(); + + res = coll.createIndex({geo: "2dsphere"}, {"2dsphereIndexVersion": NumberLong(4)}); + assert.commandWorked(res); + coll.drop(); +} + +// +// The default 2dsphere index version should match what the feature flag determines. +// Avoid allowing default version in background fcv upgrade/downgrade tests. +// +const options = { + name: "geo_2dsphere", +}; +// TODO SERVER-118561 Remove this when 9.0 is last LTS. +if (!isStableFCVSuite()) { + options["2dsphereIndexVersion"] = get2dsphereIndexVersion(); // Note this will always be v3. +} +res = coll.createIndex({geo: "2dsphere"}, options); assert.commandWorked(res); let specObj = coll.getIndexes().filter(function (z) { return z.name == "geo_2dsphere"; })[0]; -assert.eq(3, specObj["2dsphereIndexVersion"]); +assert.eq(kDefault2dSphereIndexVersion, specObj["2dsphereIndexVersion"]); coll.drop(); // diff --git a/jstests/core/query/geo/geo_extra_fields.js b/jstests/core/query/geo/geo_extra_fields.js index 32e3386ffb2..5fc95e96442 100644 --- a/jstests/core/query/geo/geo_extra_fields.js +++ b/jstests/core/query/geo/geo_extra_fields.js @@ -1,7 +1,7 @@ // This test verifies that queries on GeoJSON work regardless of extra fields and their // position in respect to the GeoJSON fields. // @tags: [ -// requires_fcv_82, +// requires_fcv_83, // # geoNear requires setup such as a optimizations to be enabled for TS. See timeseries_geonear.js // exclude_from_timeseries_crud_passthrough, // ] diff --git a/jstests/core/query/index_key_expression.js b/jstests/core/query/index_key_expression.js index 08d66fa9ceb..783139d88fd 100644 --- a/jstests/core/query/index_key_expression.js +++ b/jstests/core/query/index_key_expression.js @@ -10,6 +10,7 @@ * ] */ import {FeatureFlagUtil} from "jstests/libs/feature_flag_util.js"; +import {isStableFCVSuite} from "jstests/libs/feature_compatibility_version.js"; // TODO (SERVER-117130): Remove the mongos pinning once the related issue is resolved. // When a database is dropped, a stale router will report "database not found" error for @@ -20,6 +21,21 @@ if (TestData.pauseMigrationsDuringMultiUpdates) { const collection = db.index_key_expression; +/** + * Helper function to get the appropriate 2dsphere index version based on feature flag. + * Returns version 4 if the feature flag is enabled and we're in a stable FCV suite. + * Returns version 3 otherwise (to avoid having to drop v4 indexes during FCV transitions). + */ +function get2dsphereIndexVersion() { + const version = FeatureFlagUtil.isPresentAndEnabled(db, "2dsphereIndexVersion4") ? 4 : 3; + if (!isStableFCVSuite()) { + // If we are upgrading/downgrading the FCV, avoid having to drop any v4 indexes by pinning the version to 3. + // TODO SERVER-118561 Remove this when 9.0 is last LTS. + return 3; + } + return version; +} + /** * Returns the hash of the provided BSON element that is compatible with 'hashed' indexes. */ @@ -27,6 +43,9 @@ function getHash(bsonElement) { return assert.commandWorked(db.runCommand({_hashBSONElement: bsonElement, seed: 0})).out; } +// Get the appropriate 2dsphere index version for this test run. +const twoDSphereIndexVersion = get2dsphereIndexVersion(); + // A dictionary consisting of various test scenarios that must be run against the // '$_internalIndexKey'. // @@ -594,7 +613,7 @@ const testScenarios = [ }, spec: { key: {"data.geo": "2dsphere_bucket"}, - "2dsphereIndexVersion": 3, + "2dsphereIndexVersion": twoDSphereIndexVersion, name: "2dsphereBucketIndex", }, expectedIndexKeys: [ @@ -618,7 +637,7 @@ const testScenarios = [ }, spec: { key: {"data.geo1": "2dsphere_bucket", "data.geo2": "2dsphere_bucket"}, - "2dsphereIndexVersion": 3, + "2dsphereIndexVersion": twoDSphereIndexVersion, name: "2dsphereBucketIndex", }, expectedIndexKeys: [ @@ -656,7 +675,7 @@ const testScenarios = [ }, spec: { key: {"data.geo2": "2dsphere_bucket", "data.geo1": "2dsphere_bucket"}, - "2dsphereIndexVersion": 3, + "2dsphereIndexVersion": twoDSphereIndexVersion, name: "2dsphereBucketIndex", }, expectedIndexKeys: [ @@ -694,7 +713,7 @@ const testScenarios = [ }, spec: { key: {"data.geo1": "2dsphere_bucket", "data.geo2": 1}, - "2dsphereIndexVersion": 3, + "2dsphereIndexVersion": twoDSphereIndexVersion, name: "2dsphereBucketIndex", }, expectedIndexKeys: [ @@ -726,7 +745,7 @@ const testScenarios = [ }, spec: { key: {"data.none1": "2dsphere_bucket"}, - "2dsphereIndexVersion": 3, + "2dsphereIndexVersion": twoDSphereIndexVersion, name: "2dsphereBucketIndex", }, expectedIndexKeys: [], @@ -738,7 +757,7 @@ const testScenarios = [ }, spec: { key: {"data.geo": "2dsphere_bucket"}, - "2dsphereIndexVersion": 3, + "2dsphereIndexVersion": twoDSphereIndexVersion, name: "2dsphereBucketIndex", }, expectedErrorCode: 183934, // Can't extract geo keys: unknown GeoJSON type. @@ -754,7 +773,7 @@ const testScenarios = [ }, spec: { key: {"data.geo": "2dsphere_bucket"}, - "2dsphereIndexVersion": 3, + "2dsphereIndexVersion": twoDSphereIndexVersion, name: "2dsphereBucketIndex", }, expectedErrorCode: 6540600, // Time-series bucket documents must have 'control' object present. diff --git a/jstests/core/timeseries/ddl/timeseries_index.js b/jstests/core/timeseries/ddl/timeseries_index.js index 1e012f5589a..2f7bdbeb6ba 100644 --- a/jstests/core/timeseries/ddl/timeseries_index.js +++ b/jstests/core/timeseries/ddl/timeseries_index.js @@ -94,6 +94,31 @@ TimeseriesTest.run((insert) => { return new Date(date - (date % 60000)); }; + /** + * Helper function to check if an index spec contains a 2dsphere index. + */ + const has2dsphereIndex = function (spec) { + for (const key in spec) { + if (spec[key] === "2dsphere") { + return true; + } + } + return false; + }; + + /** + * Helper function to add 2dsphereIndexVersion: 3 to options if the spec contains a 2dsphere index. + * TODO SERVER-118561 Remove this and use the default server-selected version when 9.0 is last LTS. + */ + const add2dsphereVersionIfNeeded = function (spec, options = {}) { + if (has2dsphereIndex(spec) && TestData.isRunningFCVUpgradeDowngradeSuite) { + // Pin the index version to v3 when upgrading/downgrading FCV during the test run, + // such that we don't need to drop v4 indexes to downgrade the FCV. + options["2dsphereIndexVersion"] = 3; + } + return options; + }; + /** * Tests time-series * - createIndex @@ -140,7 +165,10 @@ TimeseriesTest.run((insert) => { // Insert data on the time-series collection and index it. assert.commandWorked(insert(coll, doc), "failed to insert doc: " + tojson(doc)); - assert.commandWorked(coll.createIndex(spec), "failed to create index: " + tojson(spec)); + assert.commandWorked( + coll.createIndex(spec, add2dsphereVersionIfNeeded(spec)), + "failed to create index: " + tojson(spec), + ); assertIndexExists(coll, spec, bucketSpec); assertIndexNotHidden(coll, spec, bucketSpec); @@ -205,14 +233,20 @@ TimeseriesTest.run((insert) => { ); // Check that we are able to drop the index by name (single name and array of names). - assert.commandWorked(coll.createIndex(spec, {name: "myindex1"}), "failed to create index: " + tojson(spec)); + assert.commandWorked( + coll.createIndex(spec, add2dsphereVersionIfNeeded(spec, {name: "myindex1"})), + "failed to create index: " + tojson(spec), + ); assertIndexExists(coll, spec, bucketSpec); assertIndexNotHidden(coll, spec, bucketSpec); assert.commandWorked(coll.dropIndex("myindex1"), "failed to drop index: myindex1"); assertIndexNotExists(coll, spec, bucketSpec); - assert.commandWorked(coll.createIndex(spec, {name: "myindex2"}), "failed to create index: " + tojson(spec)); + assert.commandWorked( + coll.createIndex(spec, add2dsphereVersionIfNeeded(spec, {name: "myindex2"})), + "failed to create index: " + tojson(spec), + ); assertIndexExists(coll, spec, bucketSpec); assertIndexNotHidden(coll, spec, bucketSpec); @@ -220,7 +254,10 @@ TimeseriesTest.run((insert) => { assertIndexNotExists(coll, spec, bucketSpec); // Check that we are able to hide and unhide the index by name. - assert.commandWorked(coll.createIndex(spec, {name: "hide1"}), "failed to create index: " + tojson(spec)); + assert.commandWorked( + coll.createIndex(spec, add2dsphereVersionIfNeeded(spec, {name: "hide1"})), + "failed to create index: " + tojson(spec), + ); assertIndexExists(coll, spec, bucketSpec); assertIndexNotHidden(coll, spec, bucketSpec); @@ -247,7 +284,10 @@ TimeseriesTest.run((insert) => { assertIndexNotExists(coll, spec, bucketSpec); // Check that we are able to hide and unhide the index by key. - assert.commandWorked(coll.createIndex(spec, {name: "hide2"}), "failed to create index: " + tojson(spec)); + assert.commandWorked( + coll.createIndex(spec, add2dsphereVersionIfNeeded(spec, {name: "hide2"})), + "failed to create index: " + tojson(spec), + ); assertIndexExists(coll, spec, bucketSpec); assertIndexNotHidden(coll, spec, bucketSpec); @@ -273,7 +313,7 @@ TimeseriesTest.run((insert) => { // Check that we are able to create the index as hidden. assert.commandWorked( - coll.createIndex(spec, {name: "hide3", hidden: true}), + coll.createIndex(spec, add2dsphereVersionIfNeeded(spec, {name: "hide3", hidden: true})), "failed to create index: " + tojson(spec), ); assertIndexExists(coll, spec, bucketSpec); @@ -297,7 +337,7 @@ TimeseriesTest.run((insert) => { // Check that user hints on queries will be allowed and will reference the raw indexes on // the buckets directly. assert.commandWorked( - coll.createIndex(spec, {name: "index_for_hint_test"}), + coll.createIndex(spec, add2dsphereVersionIfNeeded(spec, {name: "index_for_hint_test"})), "failed to create index index_for_hint_test: " + tojson(spec), ); assertIndexExists(coll, spec, bucketSpec); diff --git a/jstests/core/timeseries/ddl/timeseries_metric_index_2dsphere.js b/jstests/core/timeseries/ddl/timeseries_metric_index_2dsphere.js index 0b8f8e14f63..2bc79f4ba4e 100644 --- a/jstests/core/timeseries/ddl/timeseries_metric_index_2dsphere.js +++ b/jstests/core/timeseries/ddl/timeseries_metric_index_2dsphere.js @@ -19,6 +19,8 @@ import {getTimeseriesCollForRawOps, kRawOperationSpec} from "jstests/core/libs/raw_operation_utils.js"; import {TimeseriesTest} from "jstests/core/timeseries/libs/timeseries.js"; import {getAggPlanStage} from "jstests/libs/query/analyze_plan.js"; +import {FeatureFlagUtil} from "jstests/libs/feature_flag_util.js"; +import {isStableFCVSuite} from "jstests/libs/feature_compatibility_version.js"; TimeseriesTest.run((insert) => { const testdb = db.getSiblingDB(jsTestName() + "_db"); @@ -51,17 +53,38 @@ TimeseriesTest.run((insert) => { // Create a 2dsphere index on the time-series collection. const twoDSphereTimeseriesIndexSpec = {"location": "2dsphere"}; const twoDSphereTimeseriesIndexName = "location_2dsphere"; + const options = { + name: twoDSphereTimeseriesIndexName, + }; + // TODO SERVER-118561 Remove this and use the default server-selected version when 9.0 is last LTS. + if (TestData.isRunningFCVUpgradeDowngradeSuite) { + // Pin the index version to v3 when upgrading/downgrading FCV during the test run, + // such that we don't need to drop v4 indexes to downgrade the FCV. + // Otherwise, use the default version. + options["2dsphereIndexVersion"] = 3; + } assert.commandWorked( - timeseriescoll.createIndex(twoDSphereTimeseriesIndexSpec, { - name: twoDSphereTimeseriesIndexName, - "2dsphereIndexVersion": 3, - }), + timeseriescoll.createIndex(twoDSphereTimeseriesIndexSpec, options), "Failed to create a 2dsphere index with: " + tojson(twoDSphereTimeseriesIndexSpec), ); // Verify that the 2dsphereIndexVersion field is visible on the collection. const created = timeseriescoll.getIndexes().filter((idx) => idx.name === twoDSphereTimeseriesIndexName)[0]; - assert.eq(created["2dsphereIndexVersion"], 3, "Created index does not have version field."); + + // Build set of allowed versions. Skip v3 if feature flag is enabled and we're in a stable FCV suite. + const allowedVersions = new Set([4]); + const shouldSkipV3 = FeatureFlagUtil.isPresentAndEnabled(testdb, "2dsphereIndexVersion4") && isStableFCVSuite(); + if (!shouldSkipV3) { + // TODO SERVER-118561 We can remove this when 9.0 becomes last LTS. + allowedVersions.add(3); + } + assert( + allowedVersions.has(created["2dsphereIndexVersion"]), + "Created index does not have valid version field. Expected one of: " + + Array.from(allowedVersions).join(", ") + + ", got: " + + created["2dsphereIndexVersion"], + ); // Insert a 2dsphere index usable document. const twoDSphereDocs = [ { diff --git a/jstests/multiVersion/genericSetFCVUsage/OWNERS.yml b/jstests/multiVersion/genericSetFCVUsage/OWNERS.yml index 03ad6000ee8..a18e9cd3e32 100644 --- a/jstests/multiVersion/genericSetFCVUsage/OWNERS.yml +++ b/jstests/multiVersion/genericSetFCVUsage/OWNERS.yml @@ -42,3 +42,6 @@ filters: - "*upgrade_downgrade_viewless_timeseries_*.js": approvers: - 10gen/server-catalog-and-routing-ddl + - "s2_index_v4_downgrade.js": + approvers: + - 10gen/query-integration-features diff --git a/jstests/multiVersion/genericSetFCVUsage/s2_index_v4_downgrade.js b/jstests/multiVersion/genericSetFCVUsage/s2_index_v4_downgrade.js new file mode 100644 index 00000000000..731bf14895e --- /dev/null +++ b/jstests/multiVersion/genericSetFCVUsage/s2_index_v4_downgrade.js @@ -0,0 +1,215 @@ +/** + * Tests that downgrading FCV below 8.3 fails when v4 2dsphere indexes exist. + * This test covers both sharded clusters and replica sets. + * + * This test: + * 1. Starts a sharded cluster and replica set with latest version + * 2. Creates some v4 2dsphere indexes + * 3. Attempts to downgrade FCV to 8.0 + * 4. Validates that the FCV downgrade fails with CannotDowngrade error + * 5. Drops the v4 indexes + * 6. Attempts to downgrade FCV again and validates that it succeeds + * 7. Tests that v3 2dsphere indexes can be created on 8.3 binary with FCV 8.0 + * + * TODO SERVER-118561 Remove this test file when 9.0 is last LTS. + */ + +import "jstests/multiVersion/libs/verify_versions.js"; + +import {ReplSetTest} from "jstests/libs/replsettest.js"; +import {ShardingTest} from "jstests/libs/shardingtest.js"; + +const targetDowngradeVersion = "8.0"; + +/** + * Runs the v4 2dsphere index downgrade test against the provided connection. + * @param {Mongo} conn - The connection to run the test against (mongos or replica set primary) + * @param {string} testName - The name of the test for logging purposes + */ +function runS2IndexV4DowngradeTest(conn, testName) { + jsTest.log.info( + `Starting test: ${testName} - downgrading FCV below 8.3 should fail when v4 2dsphere indexes exist`, + ); + + const testDB = conn.getDB(jsTestName() + "_" + testName.replace(/\s+/g, "_")); + const coll = testDB.getCollection("geo_coll"); + + // Verify we're on latest FCV. + const adminDB = conn.getDB("admin"); + checkFCV(adminDB, latestFCV); + + jsTest.log.info(`[${testName}] Creating v4 2dsphere indexes`); + + // Create a collection and insert some documents with geo data. + assert.commandWorked( + coll.insert([ + {_id: 1, location: {type: "Point", coordinates: [40, -70]}}, + {_id: 2, location: {type: "Point", coordinates: [41, -71]}}, + {_id: 3, location: {type: "Point", coordinates: [42, -72]}}, + ]), + ); + + // Create multiple v4 2dsphere indexes. + assert.commandWorked( + coll.createIndex({location: "2dsphere"}, {"2dsphereIndexVersion": 4}), + "Failed to create first v4 2dsphere index", + ); + + // Create another v4 index on a different collection. + const coll2 = testDB.getCollection("geo_coll2"); + assert.commandWorked(coll2.insert([{_id: 1, geo: {type: "Point", coordinates: [50, -80]}}])); + assert.commandWorked( + coll2.createIndex({geo: "2dsphere"}, {"2dsphereIndexVersion": 4}), + "Failed to create second v4 2dsphere index", + ); + + // Verify the indexes were created with version 4. + const indexes1 = coll.getIndexes(); + const locationIndex = indexes1.find((idx) => idx.name === "location_2dsphere"); + assert.neq(null, locationIndex, "location_2dsphere index not found"); + assert.eq(4, locationIndex["2dsphereIndexVersion"], "Index should have version 4"); + + const indexes2 = coll2.getIndexes(); + const geoIndex = indexes2.find((idx) => idx.name === "geo_2dsphere"); + assert.neq(null, geoIndex, "geo_2dsphere index not found"); + assert.eq(4, geoIndex["2dsphereIndexVersion"], "Index should have version 4"); + + jsTest.log.info(`[${testName}] Attempting to downgrade FCV to ${targetDowngradeVersion}`); + const downgradeFCV = binVersionToFCV(targetDowngradeVersion); + + // Attempt to downgrade FCV - this should fail due to v4 indexes. + jsTest.log.info(`[${testName}] Setting FCV to ${downgradeFCV} - this should fail due to v4 indexes`); + const fcvResult = adminDB.runCommand({ + setFeatureCompatibilityVersion: downgradeFCV, + confirm: true, + }); + + // The downgrade should fail with CannotDowngrade error. + assert.commandFailedWithCode( + fcvResult, + ErrorCodes.CannotDowngrade, + "FCV downgrade should have failed due to v4 indexes", + ); + + jsTest.log.info(`[${testName}] FCV downgrade failed as expected: ${tojson(fcvResult)}`); + assert( + fcvResult.errmsg.includes("2dsphere") || fcvResult.errmsg.includes("version 4"), + "Error message should mention 2dsphere indexes or version 4", + ); + + // Verify FCV is still at latest (downgrade should not have proceeded). + checkFCV(adminDB, latestFCV); + + // Verify indexes still exist. + assert.neq( + null, + coll.getIndexes().find((idx) => idx.name === "location_2dsphere"), + ); + assert.neq( + null, + coll2.getIndexes().find((idx) => idx.name === "geo_2dsphere"), + ); + + jsTest.log.info(`[${testName}] Dropping v4 2dsphere indexes`); + + // Drop the v4 indexes. + assert.commandWorked(coll.dropIndex("location_2dsphere"), "Failed to drop location_2dsphere index"); + assert.commandWorked(coll2.dropIndex("geo_2dsphere"), "Failed to drop geo_2dsphere index"); + + // Verify indexes are dropped. + assert.eq( + null, + coll.getIndexes().find((idx) => idx.name === "location_2dsphere"), + ); + assert.eq( + null, + coll2.getIndexes().find((idx) => idx.name === "geo_2dsphere"), + ); + + jsTest.log.info( + `[${testName}] Attempting to downgrade FCV to ${targetDowngradeVersion} again after dropping indexes`, + ); + + // Now attempt to downgrade FCV again - this should succeed. + const fcvResult2 = adminDB.runCommand({ + setFeatureCompatibilityVersion: downgradeFCV, + confirm: true, + }); + + // The downgrade should succeed now that v4 indexes are removed. + assert.commandWorked(fcvResult2, "FCV downgrade should succeed after dropping v4 indexes"); + + // Verify FCV was downgraded. + checkFCV(adminDB, downgradeFCV); + jsTest.log.info( + `[${testName}] Test completed successfully: FCV downgrade correctly failed with v4 indexes and succeeded after dropping them`, + ); + + // Test that v3 2dsphere indexes can be created on 8.3 binary with FCV 8.0. + jsTest.log.info(`[${testName}] Testing v3 2dsphere index creation with FCV ${downgradeFCV}`); + + const coll3 = testDB.getCollection("geo_coll_v3"); + + // Insert test documents with geo data. + assert.commandWorked( + coll3.insert([ + {_id: 1, location: {type: "Point", coordinates: [40, -70]}}, + {_id: 2, location: {type: "Point", coordinates: [41, -71]}}, + {_id: 3, location: {type: "Point", coordinates: [42, -72]}}, + ]), + ); + + // Create a v3 2dsphere index (new default value) - this should succeed on 8.3 binary with FCV 8.0. + assert.commandWorked( + coll3.createIndex({location: "2dsphere"}), + `Failed to create v3 2dsphere index with FCV ${downgradeFCV}`, + ); + + // Verify the index was created with version 3. + const indexes3 = coll3.getIndexes(); + const locationIndexV3 = indexes3.find((idx) => idx.name === "location_2dsphere"); + assert.neq(null, locationIndexV3, "location_2dsphere index not found"); + assert.eq(3, locationIndexV3["2dsphereIndexVersion"], "Index should have version 3"); + + jsTest.log.info(`[${testName}] Successfully created v3 2dsphere index on 8.3 binary with FCV 8.0`); + + // Verify the index works by running a simple query. + const result = coll3 + .find({ + location: { + $near: { + $geometry: {type: "Point", coordinates: [40, -70]}, + $maxDistance: 1000000, + }, + }, + }) + .toArray(); + assert.gte(result.length, 1, "Query using v3 2dsphere index should return results"); + + jsTest.log.info(`[${testName}] v3 2dsphere index creation works on 8.3 binary with FCV 8.0`); + + // Clean up the test database. + assert.commandWorked(testDB.dropDatabase()); +} + +// Test with sharded cluster. +(function testShardedCluster() { + const st = new ShardingTest({shards: 1, mongos: 1}); + try { + runS2IndexV4DowngradeTest(st.s, "sharded cluster"); + } finally { + st.stop(); + } +})(); + +// Test with replica set. +(function testReplicaSet() { + const rst = new ReplSetTest({nodes: 3}); + rst.startSet(); + rst.initiate(); + try { + runS2IndexV4DowngradeTest(rst.getPrimary(), "replica set"); + } finally { + rst.stopSet(); + } +})(); diff --git a/jstests/with_mongot/e2e/views/mongot_indexed_views_upgrade_downgrade.js b/jstests/with_mongot/e2e/views/mongot_indexed_views_upgrade_downgrade.js index 13ed6cd337d..1a4159f5f3d 100644 --- a/jstests/with_mongot/e2e/views/mongot_indexed_views_upgrade_downgrade.js +++ b/jstests/with_mongot/e2e/views/mongot_indexed_views_upgrade_downgrade.js @@ -35,7 +35,51 @@ const vectorSearchIndexDef = { const adminDB = testDb.getMongo().getDB("admin"); +/** + * Drops all v4 2dsphere indexes from all collections in all databases. + * This is necessary before downgrading FCV below 8.3, as v4 indexes are not supported. + */ +function dropAllV4_2dsphereIndexes() { + const mongo = adminDB.getMongo(); + const dbNames = mongo.getDBNames(); + + for (const dbName of dbNames) { + const db = mongo.getDB(dbName); + // Use getCollectionInfos to get only collections (not views) + const collInfos = db.getCollectionInfos({type: "collection"}); + + for (const collInfo of collInfos) { + const collName = collInfo.name; + // Skip system collections + if (collName.startsWith("system.")) { + continue; + } + + const coll = db.getCollection(collName); + const indexes = coll.getIndexes(); + + for (const index of indexes) { + // Check if this is a v4 2dsphere index + if (index.key) { + const keyFields = Object.keys(index.key); + const has2dsphere = keyFields.some((field) => index.key[field] === "2dsphere"); + + if (has2dsphere && index["2dsphereIndexVersion"] === 4) { + jsTest.log.info( + "Dropping v4 2dsphere index: " + index.name + " on collection " + dbName + "." + collName, + ); + assert.commandWorked(coll.dropIndex(index.name)); + } + } + } + } + } +} + function upgradeDowngradeFCV(commands) { + // Drop any v4 2dsphere indexes before downgrading, as they are not supported below FCV 8.3. + dropAllV4_2dsphereIndexes(); + // Downgrade to lastLTSFCV (8.0). assert.commandWorked(adminDB.runCommand({setFeatureCompatibilityVersion: lastLTSFCV, confirm: true})); 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 bcaa5159d7c..fa93072731c 100644 --- a/src/mongo/db/commands/set_feature_compatibility_version_command.cpp +++ b/src/mongo/db/commands/set_feature_compatibility_version_command.cpp @@ -62,6 +62,8 @@ #include "mongo/db/global_catalog/ddl/sharding_ddl_util.h" #include "mongo/db/global_catalog/ddl/shardsvr_join_ddl_coordinators_request_gen.h" #include "mongo/db/global_catalog/type_shard_identity.h" +#include "mongo/db/index_builds/index_builds_coordinator.h" +#include "mongo/db/index_names.h" #include "mongo/db/logical_time.h" #include "mongo/db/namespace_string.h" #include "mongo/db/operation_context.h" @@ -1552,6 +1554,48 @@ private: "Please run ReplSetReconfig to remove priority ports prior to downgrade", replConfig.getCountOfMembersWithPriorityPort() == 0); } + + // Check for v4 2dsphere indexes when downgrading below 8.3 + if (feature_flags::gFeatureFlag2dsphereIndexVersion4 + .isDisabledOnTargetFCVButEnabledOnOriginalFCV(requestedVersion, originalVersion)) { + static const std::string kIndexVersionFieldName("2dsphereIndexVersion"); + for (const auto& dbName : DatabaseHolder::get(opCtx)->getNames()) { + Lock::DBLock dbLock(opCtx, dbName, MODE_IS); + catalog::forEachCollectionFromDb( + opCtx, dbName, MODE_IS, [&](const Collection* collection) -> bool { + auto indexCatalog = collection->getIndexCatalog(); + auto indexIterator = indexCatalog->getIndexIterator( + IndexCatalog::InclusionPolicy::kReady | + IndexCatalog::InclusionPolicy::kUnfinished); + while (indexIterator->more()) { + const IndexCatalogEntry* entry = indexIterator->next(); + const IndexDescriptor* descriptor = entry->descriptor(); + const BSONObj& infoObj = descriptor->infoObj(); + + // Check if this is a 2dsphere index + if (descriptor->getAccessMethodName() == IndexNames::GEO_2DSPHERE || + descriptor->getAccessMethodName() == + IndexNames::GEO_2DSPHERE_BUCKET) { + BSONElement versionElt = infoObj[kIndexVersionFieldName]; + if (versionElt.isNumber() && versionElt.numberInt() == 4) { + uasserted( + ErrorCodes::CannotDowngrade, + fmt::format( + "Cannot downgrade the cluster when there are 2dsphere " + "indexes with version 4. Version 4 indexes require " + "FCV 8.3 or higher. Please drop the index(es) before " + "downgrading. First detected index: {} on collection " + "{} (UUID: {}).", + descriptor->indexName(), + collection->ns().toStringForErrorMsg(), + collection->uuid().toString())); + } + } + } + return true; + }); + } + } } // Remove cluster parameters from the clusterParameters collections which are not enabled on diff --git a/src/mongo/db/geo/geometry_container.cpp b/src/mongo/db/geo/geometry_container.cpp index 64e541eefec..4c0ebb17ce1 100644 --- a/src/mongo/db/geo/geometry_container.cpp +++ b/src/mongo/db/geo/geometry_container.cpp @@ -1081,6 +1081,48 @@ Status GeometryContainer::parseFromQuery(const BSONElement& elem) { return status; } +Status GeometryContainer::parseForS2Version(const BSONElement& elem, + bool skipValidation, + boost::optional indexVersion) { + bool isLegacyIndex = indexVersion && *indexVersion < S2_INDEX_VERSION_4; + auto tryGeoJSON = [&]() { + return parseFromGeoJSON(skipValidation); + }; + auto tryLegacyPoint = [&]() { + _point.reset(new PointWithCRS()); + return GeoParser::parseLegacyPoint(elem, _point.get(), true); + }; + uassert(11617900, + "2dsphere index field must be an array or an object", + elem.type() == BSONType::object || elem.type() == BSONType::array); + + if (elem.type() == BSONType::array) { + // For arrays, only try legacy point parsing, regardless of index version. + // { location: [1, 2] } or { location: [1, 2, 3] } + return tryLegacyPoint(); + } + + // Element is an object. + // We may have legacy point and/or GeoJSON. Indexes pre-V4 were generated by + // attempting legacy point parsing first if a legacy point existed in the BSON, even if GeOJSON + // also existed. + if (isLegacyIndex) { + return elem.Obj().firstElement().isNumber() ? tryLegacyPoint() : tryGeoJSON(); + } + + // In indexes V4 and later, prioritize using the GeoJSON in the BSON, rather than the legacy + // point. Fallback to the legacy parser only if necessary (if the GeoJSON is not present). + Status status = tryGeoJSON(); + if (status == ErrorCodes::BadValue) { + Status legacyStatus = tryLegacyPoint(); + // If legacy parsing succeeds, use it; otherwise keep the original GeoJSON error. + if (legacyStatus.isOK()) { + status = legacyStatus; + } + } + return status; +} + // Examples: // { location: } // { location: [1, 2] } @@ -1089,46 +1131,16 @@ Status GeometryContainer::parseFromQuery(const BSONElement& elem) { // // "elem" is the element that contains geo data. e.g. "location": [1, 2] // We need the type information to determine whether it's legacy point. -Status GeometryContainer::parseFromStorage(const BSONElement& elem, bool skipValidation) { +Status GeometryContainer::parseFromStorage(const BSONElement& elem, + bool skipValidation, + boost::optional indexVersion) { if (!elem.isABSONObj()) { return Status(ErrorCodes::BadValue, str::stream() << "geo element must be an array or object: " << elem); } _geoElm = elem; - Status status = Status::OK(); - if (BSONType::object == elem.type()) { - // GeoJSON - // { location: { type: “Point”, coordinates: [...] } } - status = parseFromGeoJSON(skipValidation); - - // It's possible that we are dealing with a legacy point. e.g - // { location: {x: 1, y: 2, type: “Point” } } - // { location: {x: 1, y: 2} } - if (status == ErrorCodes::BadValue) { - // We must reset _point each time we attempt to re-parse, since it may retain info from - // previous attempts. - _point.reset(new PointWithCRS()); - Status legacyParsingStatus = GeoParser::parseLegacyPoint(elem, _point.get(), true); - if (legacyParsingStatus.isOK()) { - status = legacyParsingStatus; - } else { - // Return the original error status, as we may be dealing with an invalid GeoJSON - // document. e.g. {type: "Point", coordinates: "hello"} - return status; - } - } - } else { - // Legacy point - // { location: [1, 2] } - // { location: [1, 2, 3] } - // Allow more than two dimensions or extra fields, like [1, 2, 3] - // We must reset _point each time we attempt to re-parse, since it may retain info from - // previous attempts. - _point.reset(new PointWithCRS()); - status = GeoParser::parseLegacyPoint(elem, _point.get(), true); - } - + Status status = parseForS2Version(elem, skipValidation, indexVersion); if (!status.isOK()) return status; diff --git a/src/mongo/db/geo/geometry_container.h b/src/mongo/db/geo/geometry_container.h index 655d595cdb2..731cddd3a13 100644 --- a/src/mongo/db/geo/geometry_container.h +++ b/src/mongo/db/geo/geometry_container.h @@ -35,6 +35,7 @@ #include "mongo/bson/bsonobj.h" #include "mongo/bson/dotted_path/dotted_path_support.h" #include "mongo/db/geo/shapes.h" +#include "mongo/db/index/s2_common.h" #include "mongo/util/modules.h" #include @@ -49,6 +50,8 @@ #include #include +#include + namespace mongo { @@ -70,7 +73,9 @@ public: /** * Loads an empty GeometryContainer from stored geometry. */ - Status parseFromStorage(const BSONElement& elem, bool skipValidation = false); + Status parseFromStorage(const BSONElement& elem, + bool skipValidation = false, + boost::optional indexVersion = boost::none); /** * Is the geometry any of {Point, Line, Polygon}? @@ -153,6 +158,9 @@ private: class R2BoxRegion; Status parseFromGeoJSON(bool skipValidation = false); + Status parseForS2Version(const BSONElement& elem, + bool skipValidation, + boost::optional indexVersion); // Does 'this' intersect with the provided type? bool intersects(const S2Cell& otherPoint) const; diff --git a/src/mongo/db/index/BUILD.bazel b/src/mongo/db/index/BUILD.bazel index 929f3604ff2..72da337c662 100644 --- a/src/mongo/db/index/BUILD.bazel +++ b/src/mongo/db/index/BUILD.bazel @@ -22,6 +22,7 @@ mongo_cc_library( "//src/mongo/db:server_base", "//src/mongo/db/geo:geometry", "//src/mongo/db/geo:geoparser", + "//src/mongo/db/query:query_knobs", "//src/mongo/db/query/collation:collator_interface", "//src/mongo/db/storage/key_string", "//src/third_party/s2", diff --git a/src/mongo/db/index/index_access_method.h b/src/mongo/db/index/index_access_method.h index e044698c3a2..aadf0ddcd2c 100644 --- a/src/mongo/db/index/index_access_method.h +++ b/src/mongo/db/index/index_access_method.h @@ -548,6 +548,36 @@ public: const KeyStringSet& multikeyMetadataKeys, const MultikeyPaths& multikeyPaths) const; + /** + * Checks if this access method implements alternative explanations for a missing index entry. + * This allows access methods to return false early before seeking the record from the record + * store, avoiding expensive operations. + * + * Implementations should override this if checkMissingIndexEntryAlternative is overriden. + */ + virtual bool shouldCheckMissingIndexEntryAlternative(OperationContext* opCtx, + const IndexCatalogEntry& entry) const { + return false; + } + + /** + * Checks if a missing index entry has an alternative explanation (e.g., version mismatch). + * This allows access methods to handle validation-specific logic without exposing it to + * generic validation code. + * + * Returns boost::none if there's no alternative explanation (normal missing entry error). + * Otherwise returns a pair of (error message, warning message) to be added to validation + * results. + */ + virtual boost::optional> checkMissingIndexEntryAlternative( + OperationContext* opCtx, + const IndexCatalogEntry& entry, + const key_string::Value& missingKey, + const RecordId& recordId, + const BSONObj& document) const { + return boost::none; + } + /** * Provides direct access to the SortedDataInterface. This should not be used to insert * documents into an index, except for testing purposes. diff --git a/src/mongo/db/index/s2_access_method.cpp b/src/mongo/db/index/s2_access_method.cpp index cb32be14d86..88c15c4cc35 100644 --- a/src/mongo/db/index/s2_access_method.cpp +++ b/src/mongo/db/index/s2_access_method.cpp @@ -38,8 +38,12 @@ #include "mongo/db/index/expression_params.h" #include "mongo/db/index/s2_key_generator.h" #include "mongo/db/index_names.h" +#include "mongo/db/record_id.h" #include "mongo/db/shard_role/shard_catalog/index_catalog_entry.h" #include "mongo/db/shard_role/shard_catalog/index_descriptor.h" +#include "mongo/db/shard_role/transaction_resources.h" +#include "mongo/db/storage/key_string/key_string.h" +#include "mongo/db/version_context.h" #include "mongo/logv2/log.h" #include "mongo/util/assert_util.h" #include "mongo/util/str.h" @@ -97,11 +101,25 @@ S2AccessMethod::S2AccessMethod(IndexCatalogEntry* btreeState, } } +std::string formatAllowedVersions(const std::set& allowedVersions) { + std::string result; + bool first = true; + for (long long version : allowedVersions) { + if (!first) { + result += ","; + } + result += std::to_string(version); + first = false; + } + return result; +} + StatusWith cannotCreateIndexStatus(BSONElement indexVersionElt, const std::string& message, const std::string& expectedVersions = str::stream() << S2_INDEX_VERSION_1 << "," << S2_INDEX_VERSION_2 - << "," << S2_INDEX_VERSION_3, + << "," << S2_INDEX_VERSION_3 << "," + << S2_INDEX_VERSION_4, const std::string& extraMessage = "") { return {ErrorCodes::CannotCreateIndex, str::stream() << message << " { " << kIndexVersionFieldName << " : " << indexVersionElt @@ -109,15 +127,23 @@ StatusWith cannotCreateIndexStatus(BSONElement indexVersionElt, << extraMessage}; } -StatusWith S2AccessMethod::_fixSpecHelper(const BSONObj& specObj, - boost::optional expectedVersion) { - // If the spec object doesn't have field "2dsphereIndexVersion", add {2dsphereIndexVersion: 3}, - // which is the default for newly-built indexes. +StatusWith S2AccessMethod::_fixSpecHelper( + const BSONObj& specObj, boost::optional> allowedVersions) { + // If the spec object doesn't have field "2dsphereIndexVersion", add the default version + // based on the feature flag. BSONElement indexVersionElt = specObj[kIndexVersionFieldName]; + long long defaultVersion = static_cast(index2dsphere::getDefaultS2IndexVersion()); if (indexVersionElt.eoo()) { + // Validate the default version against allowed versions if provided. + if (allowedVersions && !allowedVersions->contains(defaultVersion)) { + return {ErrorCodes::CannotCreateIndex, + str::stream() << "Default geo index version " << defaultVersion + << " is not in the allowed set: [" + << formatAllowedVersions(*allowedVersions) << "]"}; + } BSONObjBuilder bob; bob.appendElements(specObj); - bob.append(kIndexVersionFieldName, S2_INDEX_VERSION_3); + bob.append(kIndexVersionFieldName, defaultVersion); return bob.obj(); } @@ -131,23 +157,30 @@ StatusWith S2AccessMethod::_fixSpecHelper(const BSONObj& specObj, return cannotCreateIndexStatus(indexVersionElt, "Invalid value for geo index version"); } - const auto indexVersion = indexVersionElt.safeNumberLong(); + long long indexVersion = indexVersionElt.safeNumberLong(); - if (expectedVersion) { - // If we have an expectedVersion, we must be in timeseries. - if (indexVersion != *expectedVersion) { - return cannotCreateIndexStatus(indexVersionElt, - "unsupported geo index version", - std::to_string(*expectedVersion), - " for timeseries"); - } + if (allowedVersions && !allowedVersions->contains(indexVersion)) { + // If we have allowedVersions, validate that the index version is in the set. + return cannotCreateIndexStatus(indexVersionElt, + "unsupported geo index version", + formatAllowedVersions(*allowedVersions), + ""); } else { - // Index version must be either 1, 2 or 3. + // Index version must be either 1, 2, 3, or 4. switch (indexVersion) { case S2_INDEX_VERSION_1: case S2_INDEX_VERSION_2: case S2_INDEX_VERSION_3: break; + case S2_INDEX_VERSION_4: { + // Gate version 4 behind feature flag. + if (index2dsphere::getDefaultS2IndexVersion() != S2_INDEX_VERSION_4) { + return Status(ErrorCodes::CannotCreateIndex, + "2dsphereIndexVersion 4 requires feature flag " + "'featureFlag2dsphereIndexVersion4' to be enabled"); + } + break; + } default: return cannotCreateIndexStatus(indexVersionElt, "unsupported geo index version"); } @@ -160,6 +193,104 @@ StatusWith S2AccessMethod::fixSpec(const BSONObj& specObj) { return S2AccessMethod::_fixSpecHelper(specObj); } +// static +KeyStringSet S2AccessMethod::generateKeysForValidation(const BSONObj& indexSpec, + const CollatorInterface* collator, + const BSONObj& document, + Ordering ordering, + const boost::optional& recordId, + key_string::Version keyStringVersion) { + S2IndexingParams params; + index2dsphere::initialize2dsphereParams(indexSpec, collator, ¶ms); + // Force version 4 for validation comparison + params.indexVersion = S2_INDEX_VERSION_4; + + SharedBufferFragmentBuilder pool(key_string::HeapBuilder::kHeapAllocatorDefaultBytes); + KeyStringSet keys; + MultikeyPaths multikeyPaths; + + BSONObj keyPattern = indexSpec.getObjectField("key"); + index2dsphere::getS2Keys(pool, + document, + keyPattern, + params, + &keys, + &multikeyPaths, + keyStringVersion, + SortedDataIndexAccessMethod::GetKeysContext::kAddingKeys, + ordering, + recordId); + + return keys; +} + +// static +bool S2AccessMethod::isVersion3(const BSONObj& indexSpec) { + BSONElement versionElt = indexSpec["2dsphereIndexVersion"]; + return versionElt.isNumber() && versionElt.numberInt() == S2_INDEX_VERSION_3; +} + +bool S2AccessMethod::shouldCheckMissingIndexEntryAlternative(OperationContext* opCtx, + const IndexCatalogEntry& entry) const { + // Only proceed with expensive record lookup for version 3 indexes, which may need + // to be checked for version 4 upgrade scenarios. + return isVersion3(entry.descriptor()->infoObj()); +} + +boost::optional> +S2AccessMethod::checkMissingIndexEntryAlternative(OperationContext* opCtx, + const IndexCatalogEntry& entry, + const key_string::Value& missingKey, + const RecordId& recordId, + const BSONObj& document) const { + // Only check for version 3 to version 4 upgrade scenarios. + if (!isVersion3(entry.descriptor()->infoObj())) { + return boost::none; + } + + try { + // Generate version 4 keys for this document. + KeyStringSet keysV4 = + generateKeysForValidation(entry.descriptor()->infoObj(), + entry.getCollator(), + document, + getSortedDataInterface()->getOrdering(), + recordId, + getSortedDataInterface()->getKeyStringVersion()); + + // Check if version 4 keys exist in the index. If they do, this indicates the + // validation failure was caused by SERVER-84794 and the index should be + // upgraded to v4. + if (!keysV4.empty()) { + auto sortedDataInterface = getSortedDataInterface(); + auto& ru = *shard_role_details::getRecoveryUnit(opCtx); + auto cursor = sortedDataInterface->newCursor(opCtx, ru); + bool foundMatchingKey = + std::any_of(keysV4.begin(), keysV4.end(), [&](const auto& keyV4) { + // seekForKeyString checks if the key exists in the index and + // returns the KeyStringEntry with the RecordId if found. + auto ksEntry = cursor->seekForKeyString(ru, keyV4.getView()); + return ksEntry && ksEntry->loc == recordId; + }); + + if (foundMatchingKey) { + const std::string indexName = entry.descriptor()->indexName(); + std::string errorMsg = "Index '" + indexName + + "' was created with 2dsphereIndexVersion 3, but validation indicates it should " + "be " + "rebuilt with version 4. Please drop and recreate this index with version 4."; + std::string warningMsg = + "Index '" + indexName + "' needs to be rebuilt with 2dsphereIndexVersion 4."; + return std::make_pair(errorMsg, warningMsg); + } + } + } catch (...) { + // If key generation fails with version 4, continue with normal error reporting. + } + + return boost::none; +} + void S2AccessMethod::validateDocument(const CollectionPtr& collection, const BSONObj& obj, const BSONObj& keyPattern) const { diff --git a/src/mongo/db/index/s2_access_method.h b/src/mongo/db/index/s2_access_method.h index de31b01d9fc..ff701a47667 100644 --- a/src/mongo/db/index/s2_access_method.h +++ b/src/mongo/db/index/s2_access_method.h @@ -46,6 +46,7 @@ #include "mongo/util/shared_buffer_fragment.h" #include +#include #include @@ -60,12 +61,13 @@ public: /** * Helper for 'fixSpec' which validates the index and returns a copy tweaked to conform to the - * expected format. If expectedVersion is specified, the index version must match. + * expected format. If allowedVersions is specified, the index version (or default version if + * not specified) must be in the allowed set. * * Returns a non-OK status if 'specObj' is invalid. */ static StatusWith _fixSpecHelper( - const BSONObj& specObj, boost::optional expectedVersion = boost::none); + const BSONObj& specObj, boost::optional> allowedVersions = boost::none); /** * Takes an index spec object for this index and returns a copy tweaked to conform to the @@ -77,6 +79,39 @@ public: */ static StatusWith fixSpec(const BSONObj& specObj); + /** + * Public API for generating S2 index keys for validation purposes. + */ + static KeyStringSet generateKeysForValidation(const BSONObj& indexSpec, + const CollatorInterface* collator, + const BSONObj& document, + Ordering ordering, + const boost::optional& recordId, + key_string::Version keyStringVersion); + + /** + * Public API for checking if an S2 index is version 3. + */ + static bool isVersion3(const BSONObj& indexSpec); + + /** + * For S2 indexes, this only returns true for version 3 indexes that may need to be checked for + * version 4 upgrade scenarios. + */ + bool shouldCheckMissingIndexEntryAlternative(OperationContext* opCtx, + const IndexCatalogEntry& entry) const override; + + /** + * Checks if a missing index entry is actually present when using version 4 key generation, + * indicating the index needs to be upgraded from version 3 to version 4. + */ + boost::optional> checkMissingIndexEntryAlternative( + OperationContext* opCtx, + const IndexCatalogEntry& entry, + const key_string::Value& missingKey, + const RecordId& recordId, + const BSONObj& document) const override; + protected: S2AccessMethod(IndexCatalogEntry* btreeState, std::unique_ptr btree, diff --git a/src/mongo/db/index/s2_bucket_access_method.h b/src/mongo/db/index/s2_bucket_access_method.h index 06ae0178d19..2db16203ae8 100644 --- a/src/mongo/db/index/s2_bucket_access_method.h +++ b/src/mongo/db/index/s2_bucket_access_method.h @@ -29,7 +29,6 @@ #pragma once -#include "mongo/base/status.h" #include "mongo/base/status_with.h" #include "mongo/bson/bsonobj.h" #include "mongo/db/index/s2_access_method.h" @@ -38,6 +37,8 @@ #include "mongo/db/storage/sorted_data_interface.h" #include "mongo/util/modules.h" +#include + namespace mongo { // Public: instantiated in index_access_method.cpp (index_builds module) and fixSpec() called from @@ -56,7 +57,8 @@ public: * Returns a non-OK status if 'specObj' is invalid. */ static StatusWith fixSpec(const BSONObj& specObj) { - return S2AccessMethod::_fixSpecHelper(specObj, /*expectedVersion*/ S2_INDEX_VERSION_3); + std::set allowedVersions = {S2_INDEX_VERSION_4, S2_INDEX_VERSION_3}; + return S2AccessMethod::_fixSpecHelper(specObj, allowedVersions); } }; diff --git a/src/mongo/db/index/s2_common.cpp b/src/mongo/db/index/s2_common.cpp index 4495531e8be..aec24022e01 100644 --- a/src/mongo/db/index/s2_common.cpp +++ b/src/mongo/db/index/s2_common.cpp @@ -41,6 +41,8 @@ #include "mongo/db/geo/geoconstants.h" #include "mongo/db/geo/geometry_container.h" #include "mongo/db/query/collation/collator_interface.h" +#include "mongo/db/query/query_feature_flags_gen.h" +#include "mongo/db/server_options.h" #include #include @@ -233,9 +235,17 @@ void initialize2dsphereParams(const BSONObj& infoObj, str::stream() << "unsupported geo index version { " << kIndexVersionFieldName << " : " << out->indexVersion << " }, only support versions: [" << S2_INDEX_VERSION_1 << "," << S2_INDEX_VERSION_2 << "," - << S2_INDEX_VERSION_3 << "]", - out->indexVersion == S2_INDEX_VERSION_3 || out->indexVersion == S2_INDEX_VERSION_2 || - out->indexVersion == S2_INDEX_VERSION_1); + << S2_INDEX_VERSION_3 << "," << S2_INDEX_VERSION_4 << "]", + out->indexVersion == S2_INDEX_VERSION_4 || out->indexVersion == S2_INDEX_VERSION_3 || + out->indexVersion == S2_INDEX_VERSION_2 || out->indexVersion == S2_INDEX_VERSION_1); +} + +S2IndexVersion getDefaultS2IndexVersion(const VersionContext& versionContext) { + const auto fcvSnapshot = serverGlobalParams.featureCompatibility.acquireFCVSnapshot(); + return feature_flags::gFeatureFlag2dsphereIndexVersion4.isEnabledUseLatestFCVWhenUninitialized( + versionContext, fcvSnapshot) + ? S2_INDEX_VERSION_4 + : S2_INDEX_VERSION_3; } } // namespace index2dsphere } // namespace mongo diff --git a/src/mongo/db/index/s2_common.h b/src/mongo/db/index/s2_common.h index 6a63151a156..40fe568b35b 100644 --- a/src/mongo/db/index/s2_common.h +++ b/src/mongo/db/index/s2_common.h @@ -33,6 +33,7 @@ #include "mongo/bson/ordering.h" #include "mongo/db/query/collation/collator_interface.h" #include "mongo/db/storage/key_string/key_string.h" +#include "mongo/db/version_context.h" #include "mongo/util/modules.h" #include @@ -59,7 +60,11 @@ enum S2IndexVersion { // The third version of the S2 index, introduced in MongoDB 3.2.0. Introduced // performance improvements and changed the key type from string to numeric - S2_INDEX_VERSION_3 = 3 + S2_INDEX_VERSION_3 = 3, + + // The fourth version of the S2 index. Changed parsing order for object-type + // geometry elements to try GeoJSON parsing before legacy point parsing. + S2_INDEX_VERSION_4 = 4 }; struct S2IndexingParams { @@ -100,5 +105,16 @@ void S2CellIdToIndexKeyStringAppend(const S2CellId& cellId, void initialize2dsphereParams(const BSONObj& infoObj, const CollatorInterface* collator, S2IndexingParams* out); + +/** + * Returns the default S2 index version based on the feature flag. + * If the feature flag for version 4 is enabled, returns S2_INDEX_VERSION_4, + * otherwise returns S2_INDEX_VERSION_3. + * + * @param versionContext The version context to use for feature flag checks. + * Defaults to kVersionContextIgnored_UNSAFE. + */ +S2IndexVersion getDefaultS2IndexVersion( + const VersionContext& versionContext = kVersionContextIgnored_UNSAFE); } // namespace index2dsphere } // namespace mongo diff --git a/src/mongo/db/index/s2_key_generator.cpp b/src/mongo/db/index/s2_key_generator.cpp index 9c21b403d66..9431251868b 100644 --- a/src/mongo/db/index/s2_key_generator.cpp +++ b/src/mongo/db/index/s2_key_generator.cpp @@ -64,7 +64,7 @@ Status S2GetKeysForElement(const BSONElement& element, const S2IndexingParams& params, std::vector* out) { GeometryContainer geoContainer; - Status status = geoContainer.parseFromStorage(element); + Status status = geoContainer.parseFromStorage(element, false, params.indexVersion); if (!status.isOK()) return status; @@ -224,7 +224,7 @@ bool getS2BucketGeoKeys(const BSONObj& document, BSONArrayBuilder coordinates(shape.subarrayStart("coordinates")); for (BSONElementSet::iterator i = elements.begin(); i != elements.end(); ++i) { GeometryContainer container; - auto status = container.parseFromStorage(*i, false); + auto status = container.parseFromStorage(*i, false, params.indexVersion); uassert(183934, str::stream() << "Can't extract geo keys: " << status.reason(), status.isOK()); diff --git a/src/mongo/db/query/query_feature_flags.idl b/src/mongo/db/query/query_feature_flags.idl index 6dbcfeed7bc..95b28363bf6 100644 --- a/src/mongo/db/query/query_feature_flags.idl +++ b/src/mongo/db/query/query_feature_flags.idl @@ -382,6 +382,13 @@ feature_flags: default: false fcv_gated: false + featureFlag2dsphereIndexVersion4: + description: "Feature flag to enable v4 of 2dsphereIndexVersion." + cpp_varname: gFeatureFlag2dsphereIndexVersion4 + default: true + fcv_gated: true + version: 8.3 + featureFlagCostBasedRanker: description: "Feature flag to enable the cost-based ranker for query plans." cpp_varname: gFeatureFlagCostBasedRanker diff --git a/src/mongo/db/validate/index_consistency.cpp b/src/mongo/db/validate/index_consistency.cpp index def48dd0471..4d5712e7d9f 100644 --- a/src/mongo/db/validate/index_consistency.cpp +++ b/src/mongo/db/validate/index_consistency.cpp @@ -245,14 +245,43 @@ void KeyStringIndexConsistency::addIndexEntryErrors(OperationContext* opCtx, // Inform which indexes have inconsistencies and add the BSON objects of the inconsistent index // entries to the results vector. int numMissingIndexEntryErrors = _missingIndexEntries.size(); + + // Track which indexes should get custom error messages from their access methods. + std::map> indexesWithCustomErrors; + for (const auto& [missingIndexKey, missingRecordId] : _missingIndexEntries) { + const IndexInfo& info = *missingIndexKey.first; + const std::string& indexName = info.indexName; + + // Check if the access method has an alternative explanation for this missing entry. + const auto entry = + _validateState->getCollection()->getIndexCatalog()->findIndexByName(opCtx, indexName); + if (entry) { + auto accessMethod = entry->accessMethod()->asSortedData(); + if (accessMethod) { + if (accessMethod->shouldCheckMissingIndexEntryAlternative(opCtx, *entry)) { + auto record = _validateState->getSeekRecordStoreCursor()->seekExact( + opCtx, missingRecordId); + if (record) { + BSONObj document = record->data.toBson(); + auto alternative = accessMethod->checkMissingIndexEntryAlternative( + opCtx, *entry, missingIndexKey.second, missingRecordId, document); + if (alternative) { + indexesWithCustomErrors[indexName] = *alternative; + continue; // Skip normal error reporting for this entry. + } + } + } + } + } + + // Mark the error as an inconsistency. _foundInconsistency(opCtx, missingIndexKey, missingRecordId, *results, /*isMissing=*/true); - const std::string& indexName = missingIndexKey.first->indexName; if (!results->getIndexResultsMap().at(indexName).isValid()) { continue; } @@ -261,6 +290,12 @@ void KeyStringIndexConsistency::addIndexEntryErrors(OperationContext* opCtx, results->getIndexResultsMap().at(indexName).addError(ss.str(), false); } + // Add custom error messages for indexes that need upgrades. + for (const auto& [indexName, messages] : indexesWithCustomErrors) { + results->getIndexResultsMap().at(indexName).addError(messages.first, false); + results->addWarning(messages.second); + } + int numExtraIndexEntryErrors = 0; for (const auto& item : _extraIndexEntries) { numExtraIndexEntryErrors += item.second.size(); diff --git a/src/mongo/dbtests/validate_tests.cpp b/src/mongo/dbtests/validate_tests.cpp index 11f35741970..d043d06890d 100644 --- a/src/mongo/dbtests/validate_tests.cpp +++ b/src/mongo/dbtests/validate_tests.cpp @@ -41,6 +41,7 @@ #include "mongo/db/dbhelpers.h" #include "mongo/db/index/index_access_method.h" #include "mongo/db/index/multikey_paths.h" +#include "mongo/db/index/s2_common.h" #include "mongo/db/index_builds/index_build_interceptor.h" #include "mongo/db/index_builds/index_build_test_helpers.h" #include "mongo/db/index_builds/index_builds_common.h" @@ -4230,6 +4231,133 @@ public: } }; +// Test that validates an S2 index created with v4 code but persisted as v3 gets reported as +// needing upgrade to v4. +class ValidateS2IndexVersion3NeedsUpgrade : public ValidateBase { +public: + ValidateS2IndexVersion3NeedsUpgrade() : ValidateBase(/*full=*/false, /*background=*/false) {} + + void run() { + // Create a new collection and insert a document with a GeoJSON Point. + lockDb(MODE_X); + + { + beginTransaction(); + ASSERT_OK(_db->dropCollection(&_opCtx, _nss)); + _db->createCollection(&_opCtx, _nss); + commitTransaction(); + } + + // Insert the document with location field. + BSONObj originalDoc = BSON( + "_id" << 1 << "location" + << BSON("latitude" << 38.7348661 << "longitude" << -9.1447487 << "coordinates" + << BSON_ARRAY(-9.1447487 << 38.7348661) << "type" + << "Point")); + { + beginTransaction(); + insertDocument(originalDoc); + commitTransaction(); + } + + // Create a 2dsphere index. This will default to v4 and generate v4 keys. + const auto indexName = "location_2dsphere"; + auto status = + createIndexFromSpec(BSON("name" << indexName << "key" << BSON("location" << "2dsphere") + << "v" << static_cast(kIndexVersion))); + ASSERT_OK(status); + + // Now modify the durable catalog to change the index version from v4 to v3. + // This simulates an index that was created with v4 code but persisted as v3. + CollectionWriter writer(&_opCtx, coll()->ns()); + { + beginTransaction(); + auto collMetadata = durable_catalog::getParsedCatalogEntry( + &_opCtx, coll()->getCatalogId(), MDBCatalog::get(&_opCtx)) + ->metadata; + int offset = collMetadata->findIndexOffset(indexName); + ASSERT_GTE(offset, 0); + + auto& indexMetadata = collMetadata->indexes[offset]; + // Modify the spec to change version from v4 to v3. + BSONObjBuilder specBuilder; + for (BSONObjIterator bi(indexMetadata.spec); bi.more();) { + BSONElement e = bi.next(); + if (e.fieldNameStringData() == "2dsphereIndexVersion") { + specBuilder.append("2dsphereIndexVersion", S2_INDEX_VERSION_3); + } else { + specBuilder.append(e); + } + } + indexMetadata.spec = specBuilder.obj(); + writer.getWritableCollection(&_opCtx)->replaceMetadata(&_opCtx, + std::move(collMetadata)); + commitTransaction(); + } + + // Reload the index from the modified catalog. + { + beginTransaction(); + auto writableCatalog = writer.getWritableCollection(&_opCtx)->getIndexCatalog(); + auto entry = writableCatalog->findIndexByName(&_opCtx, indexName); + writableCatalog->refreshEntry(&_opCtx, + writer.getWritableCollection(&_opCtx), + entry, + CreateIndexEntryFlags::kIsReady); + commitTransaction(); + } + + releaseDb(); + + // Run validate and check that it reports the index needs to be upgraded to v4. + { + ValidateResults results; + + ASSERT_OK(CollectionValidation::validate( + &_opCtx, + _nss, + ValidationOptions{CollectionValidation::ValidateMode::kForeground, + CollectionValidation::RepairMode::kNone, + kLogDiagnostics}, + &results)); + + ScopeGuard dumpOnErrorGuard([&] { + StorageDebugUtil::printValidateResults(results); + StorageDebugUtil::printCollectionAndIndexTableEntries(&_opCtx, coll()->ns()); + }); + + // Validation should fail because the index has v4 keys but is marked as v3. + ASSERT_FALSE(results.isValid()); + + // Check that the index results contain the upgrade message. + auto indexResultsIt = results.getIndexResultsMap().find(indexName); + ASSERT(indexResultsIt != results.getIndexResultsMap().end()); + const auto& indexResults = indexResultsIt->second; + + // Check that there's an error and warning message about needing to upgrade to v4. + bool foundError = + std::any_of(indexResults.getErrors().begin(), + indexResults.getErrors().end(), + [](const auto& error) { + return error.find("2dsphereIndexVersion 3") != std::string::npos && + error.find("version 4") != std::string::npos; + }); + ASSERT_TRUE(foundError) << "Expected error message about upgrading to v4"; + + bool foundWarning = + std::any_of(results.getWarnings().begin(), + results.getWarnings().end(), + [&indexName](const auto& warning) { + return warning.find(indexName) != std::string::npos && + warning.find("2dsphereIndexVersion 4") != std::string::npos; + }); + ASSERT_TRUE(foundWarning) << "Expected warning about rebuilding with v4"; + + dumpOnErrorGuard.dismiss(); + } + } +}; + class ValidateInvalidBSONOnClusteredCollection : public ValidateBase { public: explicit ValidateInvalidBSONOnClusteredCollection(bool background) @@ -4895,6 +5023,9 @@ public: add(); + // Test that validates S2 index version upgrade detection. + add(); + // Tests that validation works on clustered collections. add(false); add(true);