-
Notifications
You must be signed in to change notification settings - Fork 244
DRIVERS-3436 - Refine withTransaction timeout error wrapping semantics and label propagation in spec and prose tests #1920
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
3398035
dc02bb2
77ab714
847816e
1d4ec77
3df2210
d410325
8ec2180
e416f1f
6d97107
3bbcdc0
893f9d4
5444a68
f1c0623
f674dc7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
stIncMale marked this conversation as resolved.
Show resolved
Hide resolved
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -123,8 +123,9 @@ This method should perform the following sequence of actions: | |
|
|
||
| 2. If `transactionAttempt` > 0: | ||
|
|
||
| 1. If elapsed time + `backoffMS` > `TIMEOUT_MS`, then raise the previously encountered error (see Note 1 below). If | ||
| the elapsed time of `withTransaction` is less than TIMEOUT_MS, calculate the backoffMS to be | ||
| 1. If elapsed time + `backoffMS` > `TIMEOUT_MS`, then propagate the previously encountered error (see | ||
stIncMale marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| [propagation section](transactions-convenient-api.md#timeout-error-propagation-mechanism) below). If the | ||
nhachicha marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| elapsed time of `withTransaction` is less than TIMEOUT_MS, calculate the backoffMS to be | ||
| `jitter * min(BACKOFF_INITIAL * 1.5 ** (transactionAttempt - 1), BACKOFF_MAX)`. sleep for `backoffMS`. | ||
|
|
||
| 1. jitter is a random float between \[0, 1), optionally including 1, depending on what is most natural for the | ||
|
|
@@ -163,8 +164,7 @@ This method should perform the following sequence of actions: | |
| committed a transaction, propagate the callback's error to the caller of `withTransaction` and return | ||
| immediately. | ||
|
|
||
| 4. Otherwise, propagate the callback's error (see Note 1 below) to the caller of `withTransaction` and return | ||
| immediately. | ||
| 4. Otherwise, propagate the callback's error to the caller of `withTransaction` and return immediately. | ||
|
|
||
| 8. If the ClientSession is in the "no transaction", "transaction aborted", or "transaction committed" state, assume the | ||
| callback intentionally aborted or committed the transaction and return immediately. | ||
|
|
@@ -180,20 +180,19 @@ This method should perform the following sequence of actions: | |
|
|
||
| 2. If the `commitTransaction` error includes a "TransientTransactionError" label, jump back to step two. | ||
|
|
||
| 3. Otherwise, propagate the `commitTransaction` error (see Note 1 below) to the caller of `withTransaction` and | ||
| return immediately. | ||
| 3. Otherwise, propagate the `commitTransaction` error to the caller of `withTransaction` and return immediately. | ||
|
|
||
| 11. The transaction was committed successfully. Return immediately. | ||
|
|
||
| ______________________________________________________________________ | ||
| ###### Timeout Error propagation mechanism | ||
nhachicha marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| **Note 1:** When the `TIMEOUT_MS` (calculated in step [1.3](#sequence-of-actions)) is reached we MUST report a timeout | ||
| error wrapping the last error that was encountered which triggered the retry behavior. If `timeoutMS` is set, then | ||
| timeout error is a special type which is defined in CSOT | ||
| When the `TIMEOUT_MS` (calculated in step [1.3](#sequence-of-actions)) is reached we MUST report a timeout error | ||
| wrapping the previously encountered error. If `timeoutMS` is set, then timeout error is a special type which is defined | ||
| in CSOT | ||
| [specification](https://github.com/mongodb/specifications/blob/master/source/client-side-operations-timeout/client-side-operations-timeout.md#errors) | ||
| , If `timeoutMS` is not set, then propagate it as timeout error if the language allows to expose the underlying error as | ||
| a cause of a timeout error (see `makeTimeoutError` below in [pseudo-code](#pseudo-code)). If timeout error is thrown | ||
| then it SHOULD expose error label(s) from the transient error. | ||
| , If `timeoutMS` is not set, then propagate it as timeout error if the language allows to expose the previously | ||
| encountered error as a cause of a timeout error (see `makeTimeoutError` below in [pseudo-code](#pseudo-code)). If | ||
| timeout error is thrown then it SHOULD copy all error label(s) from the previously encountered retriable error. | ||
nhachicha marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
stIncMale marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| ##### Pseudo-code | ||
|
|
||
|
|
@@ -264,9 +263,6 @@ withTransaction(callback, options) { | |
| * {ok:1, writeConcernError: {code: 50, codeName: "MaxTimeMSExpired"}} | ||
| */ | ||
| lastError = error; | ||
| if (Date.now() - startTime >= timeout) { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was removed because in sections 7.4 & 10.3 of the [sequence-of-actions|https://github.com/mongodb/specifications/blob/master/source/transactions-convenient-api/transactions-convenient-api.md#sequence-of-actions] the errors are not retriable, and they are mainly the reason for returning immediately from
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. However, in section 10.1.1 ( |
||
| throw makeTimeoutError(error); | ||
| } | ||
| if (!isMaxTimeMSExpiredError(error) && | ||
| error.hasErrorLabel("UnknownTransactionCommitResult")) { | ||
| continue retryCommit; | ||
|
|
@@ -348,8 +344,8 @@ An earlier design also considered using the callback's return value to indicate | |
| of two ways: | ||
|
|
||
| - The callback aborts the transaction directly and returns to `withTransaction`, which will then return to its caller. | ||
| - The callback raises an error without the "TransientTransactionError" label, in which case `withTransaction` will abort | ||
| the transaction and return to its caller. | ||
| - The callback propagates an error without the "TransientTransactionError" label, in which case `withTransaction` will | ||
nhachicha marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| abort the transaction and return to its caller. | ||
|
|
||
| ### Applications are responsible for passing ClientSession for operations within a transaction | ||
|
|
||
|
|
@@ -440,6 +436,9 @@ provides an implementation of a technique already described in the MongoDB 4.0 d | |
|
|
||
| ## Changelog | ||
|
|
||
| - 2026-04-02: [DRIVERS-3436](https://github.com/mongodb/specifications/pull/1920) Refine withTransaction timeout error | ||
| wrapping semantics and label propagation in spec and prose tests. | ||
|
|
||
| - 2026-03-03: Clarify exponential backoff jitter upper bound. | ||
|
|
||
| - 2026-02-20: Fix initial backoff and growth value parameters in "Design Rationale" section. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this point also supposed to call out the propagation mechanism?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch 👍 I fixed the prose and the spec.
TL;DR
UnknownTransactionCommitResultis retriable in the commit loop if we don't exceed the timeout, so it makes sense to wrap it into a Timeout error if we exceed the timeout and want to throw and return (as described in section 10.1.1)