Skip to content

Commit b6f3aa6

Browse files
jmikolajmikola
committed
CDRIVER-2936 apply w:majority when retrying commit
Also apply a wtimeout of 10 seconds if unspecified. Support extra WC fields in bson_lookup_write_concern() Sync transaction tests with mongodb/specifications@ba1b071
1 parent 52748cf commit b6f3aa6

File tree

9 files changed

+1201
-57
lines changed

9 files changed

+1201
-57
lines changed

‎src/libmongoc/src/mongoc/mongoc-client-session-private.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
#define TRANSIENT_TXN_ERR "TransientTransactionError"
2727
#define UNKNOWN_COMMIT_RESULT "UnknownTransactionCommitResult"
2828

29+
#define MONGOC_DEFAULT_WTIMEOUT_FOR_COMMIT_RETRY 10000
30+
2931
struct _mongoc_transaction_opt_t {
3032
mongoc_read_concern_t *read_concern;
3133
mongoc_write_concern_t *write_concern;

‎src/libmongoc/src/mongoc/mongoc-client-session.c

Lines changed: 115 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -72,12 +72,6 @@ txn_opts_copy (const mongoc_transaction_opt_t *src,
7272
}
7373

7474

75-
typedef enum {
76-
TXN_COMMIT,
77-
TXN_ABORT,
78-
} mongoc_txn_intent_t;
79-
80-
8175
static void
8276
copy_labels_plus_unknown_commit_result (const bson_t *src, bson_t *dst)
8377
{
@@ -110,12 +104,8 @@ copy_labels_plus_unknown_commit_result (const bson_t *src, bson_t *dst)
110104

111105

112106
static bool
113-
txn_finish (mongoc_client_session_t *session,
114-
mongoc_txn_intent_t intent,
115-
bson_t *reply,
116-
bson_error_t *error)
107+
txn_abort (mongoc_client_session_t *session, bson_t *reply, bson_error_t *error)
117108
{
118-
const char *cmd_name;
119109
bson_t cmd = BSON_INITIALIZER;
120110
bson_t opts = BSON_INITIALIZER;
121111
bson_error_t err_local;
@@ -126,8 +116,6 @@ txn_finish (mongoc_client_session_t *session,
126116

127117
_mongoc_bson_init_if_set (reply);
128118

129-
cmd_name = (intent == TXN_COMMIT ? "commitTransaction" : "abortTransaction");
130-
131119
if (!mongoc_client_session_append (session, &opts, err_ptr)) {
132120
GOTO (done);
133121
}
@@ -143,7 +131,7 @@ txn_finish (mongoc_client_session_t *session,
143131
}
144132
}
145133

146-
BSON_APPEND_INT32 (&cmd, cmd_name, 1);
134+
BSON_APPEND_INT32 (&cmd, "abortTransaction", 1);
147135

148136
/* will be reinitialized by mongoc_client_write_command_with_opts */
149137
bson_destroy (&reply_local);
@@ -157,14 +145,114 @@ txn_finish (mongoc_client_session_t *session,
157145
bson_destroy (&reply_local);
158146
r = mongoc_client_write_command_with_opts (
159147
session->client, "admin", &cmd, &opts, &reply_local, err_ptr);
148+
}
149+
150+
if (!r) {
151+
/* we won't return an error from abortTransaction, so warn */
152+
MONGOC_WARNING ("Error in abortTransaction: %s", err_ptr->message);
153+
}
154+
155+
done:
156+
bson_destroy (&reply_local);
157+
bson_destroy (&cmd);
158+
bson_destroy (&opts);
159+
160+
return r;
161+
}
162+
163+
164+
static mongoc_write_concern_t *
165+
create_commit_retry_wc (const mongoc_write_concern_t *existing_wc)
166+
{
167+
mongoc_write_concern_t *wc;
168+
int32_t wtimeout;
169+
170+
wc = existing_wc ? mongoc_write_concern_copy (existing_wc)
171+
: mongoc_write_concern_new ();
172+
173+
wtimeout = mongoc_write_concern_get_wtimeout (wc);
174+
175+
/* Transactions spec: "If the modified write concern does not include a
176+
* wtimeout value, drivers MUST also apply wtimeout: 10000 to the write
177+
* concern in order to avoid waiting forever if the majority write concern
178+
* cannot be satisfied." */
179+
if (wtimeout <= 0) {
180+
wtimeout = MONGOC_DEFAULT_WTIMEOUT_FOR_COMMIT_RETRY;
181+
}
182+
183+
/* Transactions spec: "If the transaction is using a write concern that is
184+
* not the server default, any other write concern options MUST be left as-is
185+
* when applying w:majority. */
186+
mongoc_write_concern_set_wmajority (wc, wtimeout);
187+
188+
return wc;
189+
}
190+
191+
192+
static bool
193+
txn_commit (mongoc_client_session_t *session,
194+
bool explicitly_retrying,
195+
bson_t *reply,
196+
bson_error_t *error)
197+
{
198+
bson_t cmd = BSON_INITIALIZER;
199+
bson_t opts = BSON_INITIALIZER;
200+
bson_error_t err_local;
201+
bson_error_t *err_ptr = error ? error : &err_local;
202+
bson_t reply_local = BSON_INITIALIZER;
203+
mongoc_write_err_type_t error_type;
204+
bool r = false;
205+
bool retrying_after_error = false;
206+
mongoc_write_concern_t *retry_wc = NULL;
207+
208+
_mongoc_bson_init_if_set (reply);
209+
210+
BSON_APPEND_INT32 (&cmd, "commitTransaction", 1);
211+
212+
retry:
213+
if (!mongoc_client_session_append (session, &opts, err_ptr)) {
214+
GOTO (done);
215+
}
216+
217+
/* Transactions Spec: "When commitTransaction is retried, either by the
218+
* driver's internal retry-once logic or explicitly by the user calling
219+
* commitTransaction again, drivers MUST apply w:majority to the write
220+
* concern of the commitTransaction command." */
221+
if (!retry_wc && (retrying_after_error || explicitly_retrying)) {
222+
retry_wc = create_commit_retry_wc (session->txn.opts.write_concern
223+
? session->txn.opts.write_concern
224+
: session->client->write_concern);
225+
}
226+
227+
if (retry_wc || session->txn.opts.write_concern) {
228+
if (!mongoc_write_concern_append (
229+
retry_wc ? retry_wc : session->txn.opts.write_concern, &opts)) {
230+
bson_set_error (err_ptr,
231+
MONGOC_ERROR_TRANSACTION,
232+
MONGOC_ERROR_TRANSACTION_INVALID_STATE,
233+
"Invalid transaction write concern");
234+
GOTO (done);
235+
}
236+
}
160237

161-
error_type = _mongoc_write_error_get_type (r, err_ptr, &reply_local);
238+
/* will be reinitialized by mongoc_client_write_command_with_opts */
239+
bson_destroy (&reply_local);
240+
r = mongoc_client_write_command_with_opts (
241+
session->client, "admin", &cmd, &opts, &reply_local, err_ptr);
242+
243+
/* Transactions Spec: "Drivers MUST retry the commitTransaction command once
244+
* after it fails with a retryable error", same for abort */
245+
error_type = _mongoc_write_error_get_type (r, err_ptr, &reply_local);
246+
if (!retrying_after_error && error_type == MONGOC_WRITE_ERR_RETRY) {
247+
retrying_after_error = true; /* retry after error only once */
248+
bson_reinit (&opts);
249+
GOTO (retry);
162250
}
163251

164252
/* Transactions Spec: "add the UnknownTransactionCommitResult error label
165253
* when commitTransaction fails with a network error, server selection
166254
* error, or write concern failed / timeout." */
167-
if (intent == TXN_COMMIT && reply) {
255+
if (reply) {
168256
if ((!r && err_ptr->domain == MONGOC_ERROR_SERVER_SELECTION) ||
169257
error_type == MONGOC_WRITE_ERR_RETRY ||
170258
error_type == MONGOC_WRITE_ERR_WRITE_CONCERN) {
@@ -177,16 +265,17 @@ txn_finish (mongoc_client_session_t *session,
177265
bson_steal (reply, &reply_local);
178266
bson_init (&reply_local);
179267
}
180-
181-
} else if (intent == TXN_ABORT && !r) {
182-
/* we won't return an error from abortTransaction, so warn */
183-
MONGOC_WARNING ("Error in %s: %s", cmd_name, err_ptr->message);
184268
}
185269

186270
done:
187271
bson_destroy (&reply_local);
188272
bson_destroy (&cmd);
189273
bson_destroy (&opts);
274+
275+
if (retry_wc) {
276+
mongoc_write_concern_destroy (retry_wc);
277+
}
278+
190279
return r;
191280
}
192281

@@ -811,15 +900,18 @@ mongoc_client_session_commit_transaction (mongoc_client_session_t *session,
811900
_mongoc_bson_init_if_set (reply);
812901
r = true;
813902
break;
814-
case MONGOC_TRANSACTION_IN_PROGRESS:
815903
case MONGOC_TRANSACTION_COMMITTED:
904+
case MONGOC_TRANSACTION_IN_PROGRESS: {
905+
bool explicitly_retrying =
906+
(session->txn.state == MONGOC_TRANSACTION_COMMITTED);
816907
/* in MONGOC_TRANSACTION_ENDING we add txnNumber and autocommit: false
817908
* to the commitTransaction command, but if it fails with network error
818909
* we add UnknownTransactionCommitResult not TransientTransactionError */
819910
session->txn.state = MONGOC_TRANSACTION_ENDING;
820-
r = txn_finish (session, TXN_COMMIT, reply, error);
911+
r = txn_commit (session, explicitly_retrying, reply, error);
821912
session->txn.state = MONGOC_TRANSACTION_COMMITTED;
822913
break;
914+
}
823915
case MONGOC_TRANSACTION_ENDING:
824916
MONGOC_ERROR ("commit called in invalid state MONGOC_TRANSACTION_ENDING");
825917
abort ();
@@ -854,7 +946,7 @@ mongoc_client_session_abort_transaction (mongoc_client_session_t *session,
854946
case MONGOC_TRANSACTION_IN_PROGRESS:
855947
session->txn.state = MONGOC_TRANSACTION_ENDING;
856948
/* Transactions Spec: ignore errors from abortTransaction command */
857-
txn_finish (session, TXN_ABORT, NULL, NULL);
949+
txn_abort (session, NULL, NULL);
858950
session->txn.state = MONGOC_TRANSACTION_ABORTED;
859951
RETURN (true);
860952
case MONGOC_TRANSACTION_COMMITTED:

‎src/libmongoc/tests/json/transactions/commit.json

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,10 @@
315315
},
316316
"startTransaction": null,
317317
"autocommit": false,
318-
"writeConcern": null
318+
"writeConcern": {
319+
"w": "majority",
320+
"wtimeout": 10000
321+
}
319322
},
320323
"command_name": "commitTransaction",
321324
"database_name": "admin"
@@ -331,7 +334,10 @@
331334
},
332335
"startTransaction": null,
333336
"autocommit": false,
334-
"writeConcern": null
337+
"writeConcern": {
338+
"w": "majority",
339+
"wtimeout": 10000
340+
}
335341
},
336342
"command_name": "commitTransaction",
337343
"database_name": "admin"
@@ -350,6 +356,7 @@
350356
},
351357
{
352358
"description": "write concern error on commit",
359+
"skipReason": "SERVER-37458 Mongos does not yet support writeConcern on commit",
353360
"operations": [
354361
{
355362
"name": "startTransaction",

‎src/libmongoc/tests/json/transactions/error-labels.json

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -747,7 +747,10 @@
747747
},
748748
"startTransaction": null,
749749
"autocommit": false,
750-
"writeConcern": null
750+
"writeConcern": {
751+
"w": "majority",
752+
"wtimeout": 10000
753+
}
751754
},
752755
"command_name": "commitTransaction",
753756
"database_name": "admin"
@@ -763,7 +766,10 @@
763766
},
764767
"startTransaction": null,
765768
"autocommit": false,
766-
"writeConcern": null
769+
"writeConcern": {
770+
"w": "majority",
771+
"wtimeout": 10000
772+
}
767773
},
768774
"command_name": "commitTransaction",
769775
"database_name": "admin"
@@ -879,7 +885,10 @@
879885
},
880886
"startTransaction": null,
881887
"autocommit": false,
882-
"writeConcern": null
888+
"writeConcern": {
889+
"w": "majority",
890+
"wtimeout": 10000
891+
}
883892
},
884893
"command_name": "commitTransaction",
885894
"database_name": "admin"
@@ -895,7 +904,10 @@
895904
},
896905
"startTransaction": null,
897906
"autocommit": false,
898-
"writeConcern": null
907+
"writeConcern": {
908+
"w": "majority",
909+
"wtimeout": 10000
910+
}
899911
},
900912
"command_name": "commitTransaction",
901913
"database_name": "admin"
@@ -1024,7 +1036,8 @@
10241036
"startTransaction": null,
10251037
"autocommit": false,
10261038
"writeConcern": {
1027-
"w": "majority"
1039+
"w": "majority",
1040+
"wtimeout": 10000
10281041
}
10291042
},
10301043
"command_name": "commitTransaction",
@@ -1042,7 +1055,8 @@
10421055
"startTransaction": null,
10431056
"autocommit": false,
10441057
"writeConcern": {
1045-
"w": "majority"
1058+
"w": "majority",
1059+
"wtimeout": 10000
10461060
}
10471061
},
10481062
"command_name": "commitTransaction",
@@ -1172,7 +1186,8 @@
11721186
"startTransaction": null,
11731187
"autocommit": false,
11741188
"writeConcern": {
1175-
"w": "majority"
1189+
"w": "majority",
1190+
"wtimeout": 10000
11761191
}
11771192
},
11781193
"command_name": "commitTransaction",
@@ -1306,7 +1321,8 @@
13061321
"startTransaction": null,
13071322
"autocommit": false,
13081323
"writeConcern": {
1309-
"w": "majority"
1324+
"w": "majority",
1325+
"wtimeout": 10000
13101326
}
13111327
},
13121328
"command_name": "commitTransaction",

0 commit comments

Comments
 (0)