DRIVERS-3391: Clarify withTransaction CSOT timeout error with backoff+jitter#1890
DRIVERS-3391: Clarify withTransaction CSOT timeout error with backoff+jitter#1890nhachicha wants to merge 3 commits intomongodb:masterfrom
Conversation
nhachicha
changed the title
nhachicha
changed the title
|
@nhachicha @baileympearson Can we add a DRIVERS ticket to the backpressure epic to track and communicate these changes? |
baileympearson
changed the title
|
|
||
| Tests in this section MUST only run against replica sets and sharded clusters with server versions 4.4 or higher. | ||
|
|
||
| It is recommended that drivers run these tests with |
There was a problem hiding this comment.
Are you seeing flakiness with these tests? Node hasn't had issues and we've had these changes merged for ~2 months, and I don't know that Python has either (@sleepyStick ?)
There was a problem hiding this comment.
I don't think python has noticed any issues with this
| 1. `command_started` and `command_failed` events for an `insert` command. | ||
| 2. `command_started` and `command_failed` events for an `abortTransaction` command. | ||
|
|
||
| #### withTransaction applies a single timeout across retries due to transient errors |
There was a problem hiding this comment.
Is this test fundamentally any different from the unified test timeoutMS is not refreshed for each operation in the callback in transactions-convenient.yml? If there is a difference and we do need a new test - would it be possible to write this as a unified test?
| if (Date.now() - startTime >= timeout) { | ||
| throw getCSOTTimeoutIfSet() != null ? createCSOTMongoTimeoutException(error) : createLegacyMongoTimeoutException(e); | ||
| } | ||
|
|
There was a problem hiding this comment.
| if (Date.now() - startTime >= timeout) { | |
| throw getCSOTTimeoutIfSet() != null ? createCSOTMongoTimeoutException(error) : createLegacyMongoTimeoutException(e); | |
| } | |
| if (Date.now() - startTime >= timeout) { | |
| throw makeTimeoutError(error); | |
| } |
and then maybe define makeTimeoutError above similar to this?
function makeTimeoutError() {
return getCSOTTimeoutIfSet() != null ? createCSOTMongoTimeoutException(error) : createLegacyMongoTimeoutException(e);
}
We can then just call this wherever we raise timeout errors (there are three places in the pseudocode, I think).
| @@ -220,9 +220,12 @@ withTransaction(callback, options) { | |||
| this.abortTransaction(); | |||
There was a problem hiding this comment.
It is out of diff: but we also need to raise to clarify the error handling behavior on line 206.
| @@ -220,9 +220,12 @@ withTransaction(callback, options) { | |||
| this.abortTransaction(); | |||
There was a problem hiding this comment.
Also out of diff, but can you also update the prose description accordingly?
4 participants
As discussed in https://mongodb.slack.com/archives/C09M7C9EGF4/p1770144090977949