From 60234b9f296c931ec028d4461f191e4a4c41c9a6 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Tue, 2 Jan 2024 10:08:05 -0500 Subject: [PATCH] CDRIVER-4754 pre-4.4 mongos writeConcernError does not determine retryability Synced with mongodb/specifications#1486 --- src/libmongoc/src/mongoc/mongoc-client.c | 3 +- src/libmongoc/src/mongoc/mongoc-cluster.c | 2 +- src/libmongoc/src/mongoc/mongoc-collection.c | 2 +- .../src/mongoc/mongoc-error-private.h | 4 +- src/libmongoc/src/mongoc/mongoc-error.c | 20 +- .../src/mongoc/mongoc-write-command.c | 2 +- .../unified/insertOne-serverErrors.json | 303 ++++++++++++++++++ 7 files changed, 324 insertions(+), 12 deletions(-) diff --git a/src/libmongoc/src/mongoc/mongoc-client.c b/src/libmongoc/src/mongoc/mongoc-client.c index 15517e6dde..8db232afe4 100644 --- a/src/libmongoc/src/mongoc/mongoc-client.c +++ b/src/libmongoc/src/mongoc/mongoc-client.c @@ -1712,8 +1712,7 @@ _mongoc_client_retryable_write_command_with_stream ( ret = mongoc_cluster_run_command_monitored ( &client->cluster, &parts->assembled, reply, error); - _mongoc_write_error_handle_labels ( - ret, error, reply, server_stream->sd->max_wire_version); + _mongoc_write_error_handle_labels (ret, error, reply, server_stream->sd); if (is_retryable) { _mongoc_write_error_update_if_unsupported_storage_engine ( diff --git a/src/libmongoc/src/mongoc/mongoc-cluster.c b/src/libmongoc/src/mongoc/mongoc-cluster.c index f0e75b4846..ef24ac81c6 100644 --- a/src/libmongoc/src/mongoc/mongoc-cluster.c +++ b/src/libmongoc/src/mongoc/mongoc-cluster.c @@ -530,7 +530,7 @@ _handle_txn_error_labels (bool cmd_ret, } _mongoc_write_error_handle_labels ( - cmd_ret, cmd_err, reply, cmd->server_stream->sd->max_wire_version); + cmd_ret, cmd_err, reply, cmd->server_stream->sd); } /* diff --git a/src/libmongoc/src/mongoc/mongoc-collection.c b/src/libmongoc/src/mongoc/mongoc-collection.c index bdd594895a..9862ec0464 100644 --- a/src/libmongoc/src/mongoc/mongoc-collection.c +++ b/src/libmongoc/src/mongoc/mongoc-collection.c @@ -3548,7 +3548,7 @@ mongoc_collection_find_and_modify_with_opts ( if (parts.is_retryable_write) { _mongoc_write_error_handle_labels ( - ret, error, reply_ptr, server_stream->sd->max_wire_version); + ret, error, reply_ptr, server_stream->sd); } if (is_retryable) { diff --git a/src/libmongoc/src/mongoc/mongoc-error-private.h b/src/libmongoc/src/mongoc/mongoc-error-private.h index 30a09f007c..e76173fc30 100644 --- a/src/libmongoc/src/mongoc/mongoc-error-private.h +++ b/src/libmongoc/src/mongoc/mongoc-error-private.h @@ -19,6 +19,8 @@ #include #include +#include "mongoc-server-description.h" + BSON_BEGIN_DECLS typedef enum { @@ -72,7 +74,7 @@ void _mongoc_write_error_handle_labels (bool cmd_ret, const bson_error_t *cmd_err, bson_t *reply, - int32_t server_max_wire_version); + mongoc_server_description_t *sd); bool _mongoc_error_is_shutdown (bson_error_t *error); diff --git a/src/libmongoc/src/mongoc/mongoc-error.c b/src/libmongoc/src/mongoc/mongoc-error.c index 4df06340d4..4d262dd01a 100644 --- a/src/libmongoc/src/mongoc/mongoc-error.c +++ b/src/libmongoc/src/mongoc/mongoc-error.c @@ -20,6 +20,7 @@ #include "mongoc-error-private.h" #include "mongoc-rpc-private.h" #include "mongoc-client-private.h" +#include "mongoc-server-description-private.h" bool mongoc_error_has_label (const bson_t *reply, const char *label) @@ -102,7 +103,7 @@ void _mongoc_write_error_handle_labels (bool cmd_ret, const bson_error_t *cmd_err, bson_t *reply, - int32_t server_max_wire_version) + mongoc_server_description_t *sd) { bson_error_t error; @@ -115,14 +116,21 @@ _mongoc_write_error_handle_labels (bool cmd_ret, return; } - if (server_max_wire_version >= WIRE_VERSION_RETRYABLE_WRITE_ERROR_LABEL) { + if (sd->max_wire_version >= WIRE_VERSION_RETRYABLE_WRITE_ERROR_LABEL) { return; } - /* check for a server error. */ - if (_mongoc_cmd_check_ok_no_wce ( - reply, MONGOC_ERROR_API_VERSION_2, &error)) { - return; + /* Check for a server error. Do not consult writeConcernError for pre-4.4 + * mongos. */ + if (sd->type == MONGOC_SERVER_MONGOS) { + if (_mongoc_cmd_check_ok (reply, MONGOC_ERROR_API_VERSION_2, &error)) { + return; + } + } else { + if (_mongoc_cmd_check_ok_no_wce ( + reply, MONGOC_ERROR_API_VERSION_2, &error)) { + return; + } } if (_mongoc_write_error_is_retryable (&error)) { diff --git a/src/libmongoc/src/mongoc/mongoc-write-command.c b/src/libmongoc/src/mongoc/mongoc-write-command.c index c290eae24a..413b78df14 100644 --- a/src/libmongoc/src/mongoc/mongoc-write-command.c +++ b/src/libmongoc/src/mongoc/mongoc-write-command.c @@ -750,7 +750,7 @@ _mongoc_write_opmsg (mongoc_write_command_t *command, if (parts.is_retryable_write) { _mongoc_write_error_handle_labels ( - ret, error, &reply, server_stream->sd->max_wire_version); + ret, error, &reply, server_stream->sd); } /* Add this batch size so we skip these documents next time */ diff --git a/src/libmongoc/tests/json/retryable_writes/unified/insertOne-serverErrors.json b/src/libmongoc/tests/json/retryable_writes/unified/insertOne-serverErrors.json index 0d0d639cd4..89827fcf3f 100644 --- a/src/libmongoc/tests/json/retryable_writes/unified/insertOne-serverErrors.json +++ b/src/libmongoc/tests/json/retryable_writes/unified/insertOne-serverErrors.json @@ -165,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 + } + ] + } + ] } ] }