SERVER-119633 Add idl checks for removed server parameters (#48030)

GitOrigin-RevId: 4db32f5cd9b2893f8cbaf3e2ebdc62f9eb4586cb
2026-03-19 13:14:10 -05:00 · 2026-03-19 13:14:10 -05:00 · f78e6d673f
commit f78e6d673f
parent 854562272b
7 changed files with 338 additions and 5 deletions
--- a/bazel/wrapper_hook/lint.py
+++ b/bazel/wrapper_hook/lint.py
@ -403,6 +403,7 @@ def run_rules_lint(bazel_bin: str, args: list[str]):

    files_with_targets = list_files_with_targets(bazel_bin)
    lr.list_files_without_targets(files_with_targets, "C++", "cpp", ["src/mongo"])
+    lr.list_files_without_targets(files_with_targets, "idl", "idl", ["src"])
    lr.list_files_without_targets(
        files_with_targets,
        "javascript",
--- a/buildscripts/idl/compatibility_rules.yml
+++ b/buildscripts/idl/compatibility_rules.yml
@ -361,3 +361,130 @@ RENAMED_COMPLEX_ACCESS_CHECKS:
  get_single_user: "get_authenticated_user"
  get_authenticated_usernames: "get_authenticated_username"
  get_impersonated_usernames: "get_impersonated_username"
+
+# List of server parameters that have been removed and should not trigger compatibility errors.
+# Parameters removed in 8.0 can remain here indefinitely as they're no longer supported.
+# Parameters removed in later versions should have associated tickets for documentation/deprecation.
+IGNORE_REMOVED_PARAMETERS_LIST:
+  # Parameters removed between 8.0.18 and 8.2.4, we are assuming these have been removed long enough
+  # to not cause any problems
+  # These can be deleted from this list when we no longer check against 8.0 (this should be after we branch 9.0)
+
+  # Change streams parameters
+  - "changeCollectionTruncateMarkersMinBytes"
+  # TTL parameters
+  - "ttlCollLowPrioritySubpassLimit"
+  # Storage engine concurrency parameters (8.0 only)
+  - "lowPriorityAdmissionBypassThreshold"
+  # Query optimizer parameters
+  - "internalQueryPlanEvaluationWorksSbe"
+  - "internalQueryPlanEvaluationCollFractionSbe"
+  - "internalQuerySlotBasedExecutionHashAggForceIncreasedSpilling"
+  - "enableUnionWithVectorSearch"
+  - "internalQueryCardinalityEstimatorMode"
+  # Cascades optimizer parameters
+  - "internalCascadesOptimizerDisableScan"
+  - "internalCascadesOptimizerDisableIndexes"
+  - "internalCascadesOptimizerDisableMergeJoinRIDIntersect"
+  - "internalCascadesOptimizerDisableHashJoinRIDIntersect"
+  - "internalCascadesOptimizerDisableGroupByAndUnionRIDIntersect"
+  - "internalCascadesOptimizerDisableSargableWhenNoIndexes"
+  - "internalCascadesOptimizerFastIndexNullHandling"
+  - "internalCascadesOptimizerSampleChunks"
+  - "internalCascadesOptimizerRepeatableSample"
+  - "internalCascadesOptimizerSampleSizeMin"
+  - "internalCascadesOptimizerSampleSizeMax"
+  - "internalCascadesOptimizerSampleIndexedFields"
+  - "internalCascadesOptimizerSampleTwoFields"
+  - "internalCascadesOptimizerEnableSqrtSampleSize"
+  - "internalCascadesOptimizerDisableYieldingTolerantPlans"
+  - "internalCascadesOptimizerMinIndexEqPrefixes"
+  - "internalCascadesOptimizerMaxIndexEqPrefixes"
+  - "internalCascadesOptimizerEnableNotPushdown"
+  - "internalCascadesOptimizerSamplingCEFallBackForFilterNode"
+  - "internalCascadesOptimizerDisableFastPath"
+  - "internalCascadesOptimizerEnableParameterization"
+  # Column scan parameters
+  - "internalQueryMaxNumberOfFieldsToChooseUnfilteredColumnScan"
+  - "internalQueryMaxNumberOfFieldsToChooseFilteredColumnScan"
+  - "internalCostModelCoefficients"
+  - "internalQueryColumnScanMinCollectionSizeBytes"
+  - "internalQueryColumnScanMinAvgDocSizeBytes"
+  - "internalQueryColumnRowstoreScanMinBatchSize"
+  - "internalQueryColumnRowstoreScanMaxBatchSize"
+  - "internalQueryColumnScanMinNumColumnFilters"
+  # Query execution parameters
+  - "internalUseRoaringBitmapsForRecordIDDeduplication"
+  - "deprioritizeUnboundedUserCollectionScans"
+  - "deprioritizeUnboundedUserIndexScans"
+  - "internalQueryEnableSbeForNaryExpression"
+  # Replication parameters
+  - "forceRollbackViaRefetch"
+  - "tenantApplierBatchSizeBytes"
+  - "tenantApplierBatchSizeOps"
+  - "minOplogEntriesPerThread"
+  - "tenantApplierThreadCount"
+  - "tenantMigrationGarbageCollectionDelayMS"
+  - "tenantMigrationExternalKeysRemovalBufferSecs"
+  - "tenantMigrationOplogBufferPeekCacheSize"
+  - "tenantMigrationOplogFetcherBatchSize"
+  - "maxTenantMigrationRecipientThreadPoolSize"
+  - "minTenantMigrationRecipientThreadPoolSize"
+  - "maxTenantMigrationDonorServiceThreadPoolSize"
+  - "minTenantMigrationDonorServiceThreadPoolSize"
+  - "importQuorumTimeoutSeconds"
+  - "maxShardSplitDonorServiceThreadPoolSize"
+  - "minShardSplitDonorServiceThreadPoolSize"
+  - "shardSplitTimeoutMS"
+  - "tenantMigrationBlockingStateTimeoutMS"
+  - "tenantMigrationDisableX509Auth"
+  - "tenantMigrationExcludeDonorHostTimeoutMS"
+  - "shardSplitGarbageCollectionDelayMS"
+  # Sharding parameters
+  - "autoMergerMaxTimeProcessingChunksMS:"
+  - "rangeDeleterHighPriority"
+  # Global index parameters
+  - "globalIndexClonerServiceMaxThreadCount"
+  - "globalIndexClonerServiceFetchBatchMaxSizeBytes"
+  # Resharding parameters
+  - "reshardingCollectionClonerBatchSizeInBytes"
+  - "reshardingOplogFetcherSleepMillis"
+  # Resource consumption parameters
+  - "aggregateOperationResourceConsumptionMetrics"
+  - "profileOperationResourceConsumptionMetrics"
+  - "documentUnitSizeBytes"
+  - "indexEntryUnitSizeBytes"
+  - "totalUnitWriteSizeBytes"
+  # Storage parameters
+  - "maintainValidCursorsAcrossSBEReadCommands"
+  - "maintainValidCursorsAcrossSBEYield"
+  - "wiredTigerMaxCacheOverflowSizeGB"
+  # Catalog parameters
+  - "catalogCacheIndexMaxEntries"
+  # Scripting parameters
+  - "disableJavaScriptJIT"
+  # Service executor parameters
+  - "initialServiceExecutorUseDedicatedThread"
+  - "fixedServiceExecutorThreadLimit"
+
+  # Parameters removed since 8.2.4
+
+  # this was converted to a Incremental Feature Rollout and was locked behind a feature flag
+  - "internalQueryUnifiedWriteExecutor"
+  # These parameters were part of an unreleased project and were locked behind a feature flag
+  - "AbortOldestTransactionSessionCheckoutTimeoutMilliseconds"
+  - "AbortOldestTransactionSessionKillLimitPerBatch"
+  - "AbortOldestTransactionMemoryClearLimitPerBatch"
+  # Idl file was not part of the build graph and was removed in a drive-by code cleanup
+  - "connectionAcquisitionToWireLoggingRate"
+  # Confirmed that no atlas clusters had this parameter enabled when it was removed
+  - "enableTemporarilyUnavailableExceptions"
+  # Confirmed that no atlas clusters had this parameter enabled when it was removed
+  - "ShadingTaskExecutorPoolBaseEstablishmentBackoffMS"
+  - "ShardingTaskExecutorPoolMaxEstablishmentBackoffMS"
+  # This was a test only parameter.
+  - "planRankerMode"
+
+  # anything added below this comment is added after the initial parameter check was added.
+  # Please be specic about why this parameter should be removed instead of deprecated.
+  # Ideally this list should not grow and parameters are not removed.
--- a/buildscripts/idl/idl_check_compatibility.py
+++ b/buildscripts/idl/idl_check_compatibility.py
@ -82,6 +82,7 @@ RENAMED_COMPLEX_ACCESS_CHECKS: dict[str, str] = rules["RENAMED_COMPLEX_ACCESS_CH
 ALLOWED_NEW_COMPLEX_ACCESS_CHECKS: dict[str, list[str]] = rules["ALLOWED_NEW_COMPLEX_ACCESS_CHECKS"]
 CHANGED_ACCESS_CHECKS_TYPE: dict[str, list[str]] = rules["CHANGED_ACCESS_CHECKS_TYPE"]
 ALLOW_FIELD_VALUE_REMOVAL_LIST: dict[str, list[str]] = rules["ALLOW_FIELD_VALUE_REMOVAL_LIST"]
+IGNORE_REMOVED_PARAMETERS_LIST: list[str] = rules["IGNORE_REMOVED_PARAMETERS_LIST"]

 SKIPPED_FILES = [
    "unittest.idl",
@ -186,6 +187,9 @@ def get_new_commands(
    new_commands: dict[str, syntax.Command] = dict()
    new_command_file: dict[str, syntax.IDLParsedSpec] = dict()
    new_command_file_path: dict[str, str] = dict()
+    new_parameters: dict[str, syntax.ServerParameter] = dict()
+    new_parameters_file: dict[str, syntax.IDLParsedSpec] = dict()
+    new_parameters_file_path: dict[str, str] = dict()

    for dirpath, _, filenames in os.walk(new_idl_dir):
        for new_filename in filenames:
@ -226,7 +230,27 @@ def get_new_commands(
                    new_command_file[new_cmd.command_name] = new_idl_file
                    new_command_file_path[new_cmd.command_name] = new_idl_file_path

-    return new_commands, new_command_file, new_command_file_path
+                for new_param in new_idl_file.spec.server_parameters:
+                    # Ignore imported parameters as they will be processed in their own file.
+                    for name in [new_param.name] + new_param.deprecated_name:
+                        if name in new_parameters:
+                            ctxt.add_duplicate_parameter_name_error(
+                                name, new_idl_dir, new_idl_file_path
+                            )
+                            continue
+
+                        new_parameters[name] = new_param
+                        new_parameters_file[name] = new_idl_file
+                        new_parameters_file_path[name] = new_idl_file_path
+
+    return (
+        new_commands,
+        new_command_file,
+        new_command_file_path,
+        new_parameters,
+        new_parameters_file,
+        new_parameters_file_path,
+    )


 def get_chained_struct(
@ -1717,14 +1741,20 @@ def check_compatibility(
    """Check IDL compatibility between old and new IDL commands."""
    ctxt = IDLCompatibilityContext(old_idl_dir, new_idl_dir, IDLCompatibilityErrorCollection())

-    new_commands, new_command_file, new_command_file_path = get_new_commands(
-        ctxt, new_idl_dir, new_import_directories
-    )
+    (
+        new_commands,
+        new_command_file,
+        new_command_file_path,
+        new_parameters,
+        new_parameters_file,
+        new_parameters_file_path,
+    ) = get_new_commands(ctxt, new_idl_dir, new_import_directories)

    # Check new commands' compatibility with old ones.
    # Note, a command can be added to V1 at any time, it's ok if a
    # new command has no corresponding old command.
    old_commands: dict[str, syntax.Command] = dict()
+    old_parameters: dict[str, syntax.ServerParameter] = dict()
    for dirpath, _, filenames in os.walk(old_idl_dir):
        for old_filename in filenames:
            if not old_filename.endswith(".idl") or old_filename in SKIPPED_FILES:
@ -1822,6 +1852,29 @@ def check_compatibility(
                        ctxt, old_cmd.access_check, new_cmd.access_check, old_cmd, new_idl_file_path
                    )

+                # Make sure no server parameters were removed
+                for old_param in old_idl_file.spec.server_parameters:
+                    # we don't care if test only parameters are removed
+                    if old_param.test_only:
+                        continue
+
+                    for name in [old_param.name] + old_param.deprecated_name:
+                        if name in old_commands:
+                            ctxt.add_duplicate_parameter_name_error(
+                                name, old_idl_dir, old_idl_file_path
+                            )
+                            continue
+
+                        old_parameters[name] = old_param
+
+                        if name not in new_parameters:
+                            # Can't remove a parameter unless it's in the ignore list
+                            if (
+                                not IGNORE_REMOVED_PARAMETERS_LIST
+                                or name not in IGNORE_REMOVED_PARAMETERS_LIST
+                            ):
+                                ctxt.add_parameter_removed_error(name, old_idl_file_path)
+
    ctxt.errors.dump_errors()
    return ctxt.errors

--- a/buildscripts/idl/idl_compatibility_errors.py
+++ b/buildscripts/idl/idl_compatibility_errors.py
@ -127,6 +127,8 @@ ERROR_ID_NEW_COMMAND_PARAM_FIELD_ADDED_AS_STABLE = "ID0087"
 ERROR_ID_NEW_COMMAND_TYPE_FIELD_ADDED_AS_STABLE = "ID0088"
 ERROR_ID_NEW_COMMAND_TYPE_FIELD_ADDED_AS_UNSTABLE_REQUIRED = "ID0089"
 ERROR_ID_NEW_COMMAND_PARAM_FIELD_ADDED_AS_UNSTABLE_REQUIRED = "ID0090"
+ERROR_ID_DUPLICATE_PARAMETER_NAME = "ID0091"
+ERROR_ID_REMOVED_PARAMETER = "ID0092"


 class IDLCompatibilityCheckerError(Exception):
@ -305,6 +307,15 @@ class IDLCompatibilityContext(object):
            file,
        )

+    def add_parameter_removed_error(self, parameter_name: str, file: str) -> None:
+        """Add an error about a parameter that was removed."""
+        self._add_error(
+            ERROR_ID_REMOVED_PARAMETER,
+            parameter_name,
+            "The parameter '%s' was present in the stable API but was removed." % (parameter_name),
+            file,
+        )
+
    def add_command_strict_true_error(self, command_name: str, file: str) -> None:
        """Add an error about a command that changes from strict: false to strict: true."""
        self._add_error(
@ -324,6 +335,17 @@ class IDLCompatibilityContext(object):
            file,
        )

+    def add_duplicate_parameter_name_error(
+        self, parameter_name: str, dir_name: str, file: str
+    ) -> None:
+        """Add an error about a duplicate parameter name within a directory."""
+        self._add_error(
+            ERROR_ID_DUPLICATE_PARAMETER_NAME,
+            parameter_name,
+            "'%s' has duplicate parameter: '%s'" % (dir_name, parameter_name),
+            file,
+        )
+
    def add_reply_field_not_subset_error(
        self, command_name: str, field_name: str, type_name: str, file: str
    ) -> None:
--- a/buildscripts/idl/tests/compatibility_test_fail/new/server_parameters_test_new.idl
+++ b/buildscripts/idl/tests/compatibility_test_fail/new/server_parameters_test_new.idl
@ -0,0 +1,52 @@
+# Copyright (C) 2024-present MongoDB, Inc.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the Server Side Public License, version 1,
+# as published by MongoDB, Inc.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# Server Side Public License for more details.
+#
+# You should have received a copy of the Server Side Public License
+# along with this program. If not, see
+# <http://www.mongodb.com/licensing/server-side-public-license>.
+#
+# As a special exception, the copyright holders give permission to link the
+# code of portions of this program with the OpenSSL library under certain
+# conditions as described in each individual source file and distribute
+# linked combinations including the program with the OpenSSL library. You
+# must comply with the Server Side Public License in all respects for
+# all of the code used other than as permitted herein. If you modify file(s)
+# with this exception, you may extend this exception to your version of the
+# file(s), but you are not obligated to do so. If you do not wish to do so,
+# delete this exception statement from your version. If you delete this
+# exception statement from all source files in the program, then also delete
+# it in the license file.
+#
+
+global:
+    cpp_namespace: "mongo"
+
+imports:
+    - "mongo/db/basic_types.idl"
+
+server_parameters:
+    testDuplicateParameter:
+        description: "This parameter exists and will cause duplicate error"
+        set_at: [startup]
+        cpp_varname: gTestDuplicateParameter
+        cpp_vartype: int
+        default: 50
+        redact: false
+
+commands:
+    testDuplicateParameter:
+        description: "This command has the same name as a server parameter"
+        command_name: testDuplicateParameter
+        namespace: ignored
+        cpp_name: testDuplicateParameter
+        strict: true
+        api_version: "1"
+        reply_type: OkReply
--- a/buildscripts/idl/tests/compatibility_test_fail/old/server_parameters_test_old.idl
+++ b/buildscripts/idl/tests/compatibility_test_fail/old/server_parameters_test_old.idl
@ -0,0 +1,61 @@
+# Copyright (C) 2024-present MongoDB, Inc.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the Server Side Public License, version 1,
+# as published by MongoDB, Inc.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# Server Side Public License for more details.
+#
+# You should have received a copy of the Server Side Public License
+# along with this program. If not, see
+# <http://www.mongodb.com/licensing/server-side-public-license>.
+#
+# As a special exception, the copyright holders give permission to link the
+# code of portions of this program with the OpenSSL library under certain
+# conditions as described in each individual source file and distribute
+# linked combinations including the program with the OpenSSL library. You
+# must comply with the Server Side Public License in all respects for
+# all of the code used other than as permitted herein. If you modify file(s)
+# with this exception, you may extend this exception to your version of the
+# file(s), but you are not obligated to do so. If you do not wish to do so,
+# delete this exception statement from your version. If you delete this
+# exception statement from all source files in the program, then also delete
+# it in the license file.
+#
+
+global:
+    cpp_namespace: "mongo"
+
+imports:
+    - "mongo/db/basic_types.idl"
+
+server_parameters:
+    testRemovedParameter:
+        description: "This parameter will be removed in the new version"
+        set_at: [startup, runtime]
+        cpp_varname: gTestRemovedParameter
+        cpp_vartype: AtomicWord<int>
+        default: 100
+        validator: {gte: 0}
+        redact: false
+
+    testDuplicateParameter:
+        description: "This parameter exists and will cause duplicate error"
+        set_at: [startup]
+        cpp_varname: gTestDuplicateParameter
+        cpp_vartype: int
+        default: 50
+        redact: false
+
+commands:
+    testDuplicateParameter:
+        description: "This command has the same name as a server parameter"
+        command_name: testDuplicateParameter
+        namespace: ignored
+        cpp_name: testDuplicateParameter
+        strict: true
+        api_version: "1"
+        reply_type: OkReply
--- a/buildscripts/idl/tests/test_compatibility.py
+++ b/buildscripts/idl/tests/test_compatibility.py
@ -2221,7 +2221,24 @@ class TestIDLCompatibilityChecker(unittest.TestCase):
            "newUnstableRequiredParameterAdded",
        )

-        self.assertEqual(error_collection.count(), 214)
+        removed_parameter_error = error_collection.get_error_by_error_id(
+            idl_compatibility_errors.ERROR_ID_REMOVED_PARAMETER
+        )
+        self.assertTrue(
+            removed_parameter_error.error_id == idl_compatibility_errors.ERROR_ID_REMOVED_PARAMETER
+        )
+        self.assertRegex(str(removed_parameter_error), "testRemovedParameter")
+
+        duplicate_parameter_name_error = error_collection.get_error_by_error_id(
+            idl_compatibility_errors.ERROR_ID_DUPLICATE_PARAMETER_NAME
+        )
+        self.assertTrue(
+            duplicate_parameter_name_error.error_id
+            == idl_compatibility_errors.ERROR_ID_DUPLICATE_PARAMETER_NAME
+        )
+        self.assertRegex(str(duplicate_parameter_name_error), "testDuplicateParameter")
+
+        self.assertEqual(error_collection.count(), 216)

    def test_generic_argument_compatibility_pass(self):
        """Tests that compatible old and new generic_argument.idl files should pass."""