From 85b0c0e3c13cbb7c005147a95292185ca1fc4b1d Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Mon, 29 Jan 2024 12:58:15 -0600 Subject: [PATCH] PYTHON-4018 Clarify exactly what code/label fields drivers should inspect to determine retryability (#1489) --- pymongo/mongo_client.py | 12 +- .../unified/insertOne-serverErrors.json | 322 +++++++++++++++++- 2 files changed, 320 insertions(+), 14 deletions(-) diff --git a/pymongo/mongo_client.py b/pymongo/mongo_client.py index 4991d59f1..5b4474a62 100644 --- a/pymongo/mongo_client.py +++ b/pymongo/mongo_client.py @@ -83,6 +83,7 @@ from pymongo.errors import ( PyMongoError, ServerSelectionTimeoutError, WaitQueueTimeoutError, + WriteConcernError, ) from pymongo.lock import _HAS_REGISTER_AT_FORK, _create_lock, _release_locks from pymongo.monitoring import ConnectionClosedReason @@ -2141,7 +2142,7 @@ def _retryable_error_doc(exc: PyMongoError) -> Optional[Mapping[str, Any]]: return None -def _add_retryable_write_error(exc: PyMongoError, max_wire_version: int) -> None: +def _add_retryable_write_error(exc: PyMongoError, max_wire_version: int, is_mongos: bool) -> None: doc = _retryable_error_doc(exc) if doc: code = doc.get("code", 0) @@ -2158,7 +2159,10 @@ def _add_retryable_write_error(exc: PyMongoError, max_wire_version: int) -> None for label in doc.get("errorLabels", []): exc._add_error_label(label) else: - if code in helpers._RETRYABLE_ERROR_CODES: + # Do not consult writeConcernError for pre-4.4 mongos. + if isinstance(exc, WriteConcernError) and is_mongos: + pass + elif code in helpers._RETRYABLE_ERROR_CODES: exc._add_error_label("RetryableWriteError") # Connection errors are always retryable except NotPrimaryError and WaitQueueTimeoutError which is @@ -2392,12 +2396,14 @@ class _ClientConnectionRetryable(Generic[T]): """ try: max_wire_version = 0 + is_mongos = False self._server = self._get_server() supports_session = ( self._session is not None and self._server.description.retryable_writes_supported ) with self._client._checkout(self._server, self._session) as conn: max_wire_version = conn.max_wire_version + is_mongos = conn.is_mongos if self._retryable and not supports_session: # A retry is not possible because this server does # not support sessions raise the last error. @@ -2408,7 +2414,7 @@ class _ClientConnectionRetryable(Generic[T]): if not self._retryable: raise # Add the RetryableWriteError label, if applicable. - _add_retryable_write_error(exc, max_wire_version) + _add_retryable_write_error(exc, max_wire_version, is_mongos) raise def _read(self) -> T: diff --git a/test/retryable_writes/unified/insertOne-serverErrors.json b/test/retryable_writes/unified/insertOne-serverErrors.json index 77245a819..89827fcf3 100644 --- a/test/retryable_writes/unified/insertOne-serverErrors.json +++ b/test/retryable_writes/unified/insertOne-serverErrors.json @@ -3,10 +3,16 @@ "schemaVersion": "1.0", "runOnRequirements": [ { - "minServerVersion": "3.6", + "minServerVersion": "4.0", "topologies": [ "replicaset" ] + }, + { + "minServerVersion": "4.1.7", + "topologies": [ + "sharded" + ] } ], "createEntities": [ @@ -55,16 +61,7 @@ "description": "InsertOne succeeds after retryable writeConcernError", "runOnRequirements": [ { - "minServerVersion": "4.0", - "topologies": [ - "replicaset" - ] - }, - { - "minServerVersion": "4.1.7", - "topologies": [ - "sharded-replicaset" - ] + "minServerVersion": "4.3.1" } ], "operations": [ @@ -168,6 +165,309 @@ ] } ] + }, + { + "description": "RetryableWriteError label is added based on top-level code in pre-4.4 server response", + "runOnRequirements": [ + { + "minServerVersion": "4.2", + "maxServerVersion": "4.2.99", + "topologies": [ + "replicaset", + "sharded" + ] + } + ], + "operations": [ + { + "name": "failPoint", + "object": "testRunner", + "arguments": { + "client": "client0", + "failPoint": { + "configureFailPoint": "failCommand", + "mode": { + "times": 2 + }, + "data": { + "failCommands": [ + "insert" + ], + "errorCode": 189 + } + } + } + }, + { + "name": "insertOne", + "object": "collection0", + "arguments": { + "document": { + "_id": 3, + "x": 33 + } + }, + "expectError": { + "errorLabelsContain": [ + "RetryableWriteError" + ] + } + } + ], + "expectEvents": [ + { + "client": "client0", + "events": [ + { + "commandStartedEvent": { + "command": { + "insert": "coll", + "documents": [ + { + "_id": 3, + "x": 33 + } + ] + }, + "commandName": "insert", + "databaseName": "retryable-writes-tests" + } + }, + { + "commandStartedEvent": { + "command": { + "insert": "coll", + "documents": [ + { + "_id": 3, + "x": 33 + } + ] + }, + "commandName": "insert", + "databaseName": "retryable-writes-tests" + } + } + ] + } + ], + "outcome": [ + { + "collectionName": "coll", + "databaseName": "retryable-writes-tests", + "documents": [ + { + "_id": 1, + "x": 11 + }, + { + "_id": 2, + "x": 22 + } + ] + } + ] + }, + { + "description": "RetryableWriteError label is added based on writeConcernError in pre-4.4 mongod response", + "runOnRequirements": [ + { + "minServerVersion": "4.2", + "maxServerVersion": "4.2.99", + "topologies": [ + "replicaset" + ] + } + ], + "operations": [ + { + "name": "failPoint", + "object": "testRunner", + "arguments": { + "client": "client0", + "failPoint": { + "configureFailPoint": "failCommand", + "mode": { + "times": 2 + }, + "data": { + "failCommands": [ + "insert" + ], + "writeConcernError": { + "code": 91, + "errmsg": "Replication is being shut down" + } + } + } + } + }, + { + "name": "insertOne", + "object": "collection0", + "arguments": { + "document": { + "_id": 3, + "x": 33 + } + }, + "expectError": { + "errorLabelsContain": [ + "RetryableWriteError" + ] + } + } + ], + "expectEvents": [ + { + "client": "client0", + "events": [ + { + "commandStartedEvent": { + "command": { + "insert": "coll", + "documents": [ + { + "_id": 3, + "x": 33 + } + ] + }, + "commandName": "insert", + "databaseName": "retryable-writes-tests" + } + }, + { + "commandStartedEvent": { + "command": { + "insert": "coll", + "documents": [ + { + "_id": 3, + "x": 33 + } + ] + }, + "commandName": "insert", + "databaseName": "retryable-writes-tests" + } + } + ] + } + ], + "outcome": [ + { + "collectionName": "coll", + "databaseName": "retryable-writes-tests", + "documents": [ + { + "_id": 1, + "x": 11 + }, + { + "_id": 2, + "x": 22 + }, + { + "_id": 3, + "x": 33 + } + ] + } + ] + }, + { + "description": "RetryableWriteError label is not added based on writeConcernError in pre-4.4 mongos response", + "runOnRequirements": [ + { + "minServerVersion": "4.2", + "maxServerVersion": "4.2.99", + "topologies": [ + "sharded" + ] + } + ], + "operations": [ + { + "name": "failPoint", + "object": "testRunner", + "arguments": { + "client": "client0", + "failPoint": { + "configureFailPoint": "failCommand", + "mode": { + "times": 1 + }, + "data": { + "failCommands": [ + "insert" + ], + "writeConcernError": { + "code": 91, + "errmsg": "Replication is being shut down" + } + } + } + } + }, + { + "name": "insertOne", + "object": "collection0", + "arguments": { + "document": { + "_id": 3, + "x": 33 + } + }, + "expectError": { + "errorLabelsOmit": [ + "RetryableWriteError" + ] + } + } + ], + "expectEvents": [ + { + "client": "client0", + "events": [ + { + "commandStartedEvent": { + "command": { + "insert": "coll", + "documents": [ + { + "_id": 3, + "x": 33 + } + ] + }, + "commandName": "insert", + "databaseName": "retryable-writes-tests" + } + } + ] + } + ], + "outcome": [ + { + "collectionName": "coll", + "databaseName": "retryable-writes-tests", + "documents": [ + { + "_id": 1, + "x": 11 + }, + { + "_id": 2, + "x": 22 + }, + { + "_id": 3, + "x": 33 + } + ] + } + ] } ] }