GODRIVER-3032 Don't retry writes on pre-4.4 mongos (#1915)

Author: prestonvasquez

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": [

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:

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

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()