From 91d960141d975bc124604a99bb0ec7941e63aea2 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Tue, 2 Jan 2024 10:15:01 -0500 Subject: [PATCH] DRIVERS-1641: pre-4.4 mongos writeConcernError does not determine retryability Also clarify that pre-4.4 writeErrors[].code should never be used to determine retryability. This is intentionally untested because failCommand does not provide a way to mock writeErrors. --- source/retryable-writes/retryable-writes.rst | 16 +- .../tests/unified/insertOne-serverErrors.json | 307 ++++++++++++++++++ .../tests/unified/insertOne-serverErrors.yml | 106 ++++++ 3 files changed, 426 insertions(+), 3 deletions(-) diff --git a/source/retryable-writes/retryable-writes.rst b/source/retryable-writes/retryable-writes.rst index c8d8b4a25b..943f0729d4 100644 --- a/source/retryable-writes/retryable-writes.rst +++ b/source/retryable-writes/retryable-writes.rst @@ -252,7 +252,8 @@ The RetryableWriteError label might be added to an error in a variety of ways: errors that meet the following criteria if the retryWrites option is set to true on the client performing the relevant operation: - - a server error response with any the following codes: + - a mongod or mongos response with any the following error codes in the + top-level ``code`` field: .. list-table:: :header-rows: 1 @@ -284,8 +285,14 @@ The RetryableWriteError label might be added to an error in a variety of ways: * - ExceededTimeLimit - 262 - - a server response with a write concern error response containing any of the - previously listed codes + - a mongod response with any of the previously listed codes in the + ``writeConcernError.code`` field. + + Drivers MUST NOT add a RetryableWriteError label based on the following: + + - any ``writeErrors[].code` fields in a mongod or mongos response + + - the ``writeConcernError.code`` field in a mongos response The criteria for retryable errors is similar to the discussion in the SDAM spec's section on `Error Handling`_, but includes additional error codes. See @@ -850,6 +857,9 @@ inconsistent with the server and potentially confusing to developers. Changelog ========= +:2024-01-02: Do not use ``writeConcernError.code`` in pre-4.4 mongos response to + determine retryability. Do not use ``writeErrors[].code`` in + pre-4.4 server responses to determine retryability. :2023-12-06: Clarify that writes are not retried within transactions. :2023-10-02: When CSOT is not enabled, one retry attempt occurs. :2023-08-26: Require that in a sharded cluster the server on which the diff --git a/source/retryable-writes/tests/unified/insertOne-serverErrors.json b/source/retryable-writes/tests/unified/insertOne-serverErrors.json index aef30fda2e..786d0339f3 100644 --- a/source/retryable-writes/tests/unified/insertOne-serverErrors.json +++ b/source/retryable-writes/tests/unified/insertOne-serverErrors.json @@ -160,6 +160,313 @@ ] } ] + }, + { + "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": 1 + }, + "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 + }, + { + "_id": 3, + "x": 33 + } + ] + } + ] + }, + { + "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": 1 + }, + "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 + } + ] + } + ] } ] } diff --git a/source/retryable-writes/tests/unified/insertOne-serverErrors.yml b/source/retryable-writes/tests/unified/insertOne-serverErrors.yml index 231569fb0d..643411c4c2 100644 --- a/source/retryable-writes/tests/unified/insertOne-serverErrors.yml +++ b/source/retryable-writes/tests/unified/insertOne-serverErrors.yml @@ -73,3 +73,109 @@ tests: - { _id: 1, x: 11 } - { _id: 2, x: 22 } - { _id: 3, x: 33 } # The write was still applied + + - 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 # PrimarySteppedDown + - name: insertOne + object: *collection0 + arguments: + document: { _id: 3, x: 33 } + expectError: + errorLabelsContain: [ "RetryableWriteError" ] + expectEvents: + - client: *client0 + events: + - commandStartedEvent: &insertCommandStartedEvent + command: + insert: *collectionName + documents: [{ _id: 3, x: 33 }] + commandName: insert + databaseName: *databaseName + - commandStartedEvent: *insertCommandStartedEvent + outcome: + - collectionName: *collectionName + databaseName: *databaseName + documents: + - { _id: 1, x: 11 } + - { _id: 2, x: 22 } + #- { _id: 3, x: 33 } + + - 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: &failInsertWithWriteConcernError + configureFailPoint: failCommand + mode: { times: 1 } + data: + failCommands: [ "insert" ] + writeConcernError: + code: 91 # ShutdownInProgress + errmsg: "Replication is being shut down" + - name: insertOne + object: *collection0 + arguments: + document: { _id: 3, x: 33 } + expectError: + errorLabelsContain: [ "RetryableWriteError" ] + expectEvents: + - client: *client0 + events: + - commandStartedEvent: *insertCommandStartedEvent + - commandStartedEvent: *insertCommandStartedEvent + outcome: + - collectionName: *collectionName + databaseName: *databaseName + 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: *failInsertWithWriteConcernError + - name: insertOne + object: *collection0 + arguments: + document: { _id: 3, x: 33 } + expectError: + errorLabelsOmit: [ "RetryableWriteError" ] + expectEvents: + - client: *client0 + events: + - commandStartedEvent: *insertCommandStartedEvent + outcome: + - collectionName: *collectionName + databaseName: *databaseName + documents: + - { _id: 1, x: 11 } + - { _id: 2, x: 22 } + - { _id: 3, x: 33 } # The write was still applied