diff --git a/bazel/wrapper_hook/lint.py b/bazel/wrapper_hook/lint.py index 20f4adc3faf..5ef6fd564b2 100644 --- 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", diff --git a/buildscripts/idl/compatibility_rules.yml b/buildscripts/idl/compatibility_rules.yml index 619d009178c..87f016cb5b6 100644 --- 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. diff --git a/buildscripts/idl/idl_check_compatibility.py b/buildscripts/idl/idl_check_compatibility.py index d42be7c219a..30bea79f8f1 100644 --- 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 diff --git a/buildscripts/idl/idl_compatibility_errors.py b/buildscripts/idl/idl_compatibility_errors.py index 8e8c6df8a1a..97c2d444741 100644 --- 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: diff --git 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 new file mode 100644 index 00000000000..4390b49f4f8 --- /dev/null +++ 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 +# . +# +# 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 diff --git 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 new file mode 100644 index 00000000000..2e18bc3f39f --- /dev/null +++ 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 +# . +# +# 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 + 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 diff --git a/buildscripts/idl/tests/test_compatibility.py b/buildscripts/idl/tests/test_compatibility.py index d0de6d41c5b..81b2a6e54ee 100644 --- 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."""