WIP feat(DRIVERS-3239): add exponential backoff in operation retry loop #4806
Conversation
baileympearson
force-pushed
the
DRIVERS-3239
branch
3 times, most recently
from
c69eb98 to
db39675
Compare
baileympearson
force-pushed
the
DRIVERS-3239
branch
2 times, most recently
from
5365bbd to
39bdcdb
Compare
| // `autocommit` must always be false to differentiate from retryable writes | ||
| command.autocommit = false; | ||
|
|
||
| if (session.transaction.state === TxnState.STARTING_TRANSACTION) { |
There was a problem hiding this comment.
From the transactions spec:
The session transitions to the "transaction in progress" state after completing the first command within a transaction —
even on error.
Technically we weren't spec compliant - we transitioned a transaction to "in progress" here (in applySession), which is called when constructing the final command document. In practice, I think this is effectively the same thing: If we unconditionally transition a transaction into "in progress" after an operation finishes, transitioning the transaction to "in progress" has the same effect (so long as a transaction is not used concurrently, which they never are because transactions are not thread safe).
With client backpressure, we now need to omit RetryableErrors from transitioning transactions to in progress, because if we retry the first command in a transaction we must include startTransaction on the retry. This change, plus the change in updateSessionFromResponse handle this scenario.
| this.throwIfAborted(); | ||
| } | ||
| } catch (error) { | ||
| if (options.session != null && !(error instanceof MongoServerError)) { |
There was a problem hiding this comment.
edge case: if we encounter a network error (such as a failCommand with closeConnection=true) we never get a server response to update a session with, but still need to update the session's transaction, if the session is in a transaction.
| * @param operation - The operation to execute | ||
| * */ | ||
| async function tryOperation<T extends AbstractOperation, TResult = ResultTypeFromOperation<T>>( | ||
| async function executeOperationWithRetries< |
There was a problem hiding this comment.
better name imo.
baileympearson
force-pushed
the
DRIVERS-3239
branch
from
74ba8b2 to
61cdfc2
Compare
Description
Summary of Changes
Notes for Reviewers
What is the motivation for this change?
Release Highlight
Release notes highlight
Double check the following
npm run check:lint)type(NODE-xxxx)[!]: descriptionfeat(NODE-1234)!: rewriting everything in coffeescript