WIP feat(DRIVERS-3239): add exponential backoff in operation retry loop#4806
WIP feat(DRIVERS-3239): add exponential backoff in operation retry loop#4806baileympearson wants to merge 7 commits intomainfrom
Conversation
c69eb98 to
db39675
Compare
5365bbd to
39bdcdb
Compare
| @@ -1147,9 +1157,9 @@ export function applySession( | |||
| 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.
74ba8b2 to
61cdfc2
Compare
61cdfc2 to
fed9604
Compare
62f20fc to
77bd87a
Compare
77bd87a to
7ba7426
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