diff --git a/testdata/retryable-writes/unified/insertOne-serverErrors.json b/testdata/retryable-writes/unified/insertOne-serverErrors.json index 74d078377a..f404adcaf4 100644 --- a/testdata/retryable-writes/unified/insertOne-serverErrors.json +++ b/testdata/retryable-writes/unified/insertOne-serverErrors.json @@ -377,6 +377,99 @@ } ] }, + { + "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 + } + ] + } + ] + }, { "description": "InsertOne succeeds after connection failure", "operations": [ diff --git a/testdata/retryable-writes/unified/insertOne-serverErrors.yml b/testdata/retryable-writes/unified/insertOne-serverErrors.yml index b69864d7ab..95fa71ec79 100644 --- a/testdata/retryable-writes/unified/insertOne-serverErrors.yml +++ b/testdata/retryable-writes/unified/insertOne-serverErrors.yml @@ -157,6 +157,44 @@ tests: # writeConcernError doesn't prevent the server from applying the write - { _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 + # Trigger the fail point only once since a RetryableWriteError label + # will not be added and the write will not be retried. + 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: + errorLabelsOmit: [ "RetryableWriteError" ] + expectEvents: + - client: *client0 + events: + - commandStartedEvent: *insertCommandStartedEvent + outcome: + - collectionName: *collectionName + databaseName: *databaseName + documents: + - { _id: 1, x: 11 } + - { _id: 2, x: 22 } + # writeConcernError doesn't prevent the server from applying the write + - { _id: 3, x: 33 } - description: 'InsertOne succeeds after connection failure' operations: diff --git a/x/mongo/driver/errors.go b/x/mongo/driver/errors.go index f4bd46deb5..9191b26d25 100644 --- a/x/mongo/driver/errors.go +++ b/x/mongo/driver/errors.go @@ -140,7 +140,7 @@ func (wce WriteCommandError) Error() string { } // Retryable returns true if the error is retryable -func (wce WriteCommandError) Retryable(wireVersion *description.VersionRange) bool { +func (wce WriteCommandError) Retryable(serverKind description.ServerKind, wireVersion *description.VersionRange) bool { for _, label := range wce.Labels { if label == RetryableWriteError { return true @@ -153,7 +153,7 @@ func (wce WriteCommandError) Retryable(wireVersion *description.VersionRange) bo if wce.WriteConcernError == nil { return false } - return wce.WriteConcernError.Retryable() + return wce.WriteConcernError.Retryable(serverKind, wireVersion) } // HasErrorLabel returns true if the error contains the specified label. @@ -186,7 +186,14 @@ func (wce WriteConcernError) Error() string { } // Retryable returns true if the error is retryable -func (wce WriteConcernError) Retryable() bool { +func (wce WriteConcernError) Retryable(serverKind description.ServerKind, wireVersion *description.VersionRange) bool { + if serverKind == description.ServerKindMongos && wireVersion.Max < 9 { + // For a pre-4.4 mongos response, we can trust that mongos will have already + // retried the operation if necessary. Drivers should not retry to avoid + // "excessive retrying". + return false + } + for _, code := range retryableCodes { if wce.Code == int64(code) { return true diff --git a/x/mongo/driver/operation.go b/x/mongo/driver/operation.go index 968b2f258c..934604d700 100644 --- a/x/mongo/driver/operation.go +++ b/x/mongo/driver/operation.go @@ -820,7 +820,7 @@ func (op Operation) Execute(ctx context.Context) error { } connDesc := conn.Description() - retryableErr := tt.Retryable(connDesc.WireVersion) + retryableErr := tt.Retryable(connDesc.Kind, connDesc.WireVersion) preRetryWriteLabelVersion := connDesc.WireVersion != nil && connDesc.WireVersion.Max < 9 inTransaction := op.Client != nil && !(op.Client.Committing || op.Client.Aborting) && op.Client.TransactionRunning()