-
Notifications
You must be signed in to change notification settings - Fork 246
DRIVERS-1934: withTransaction API retries too frequently #1851
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
Conversation
| callbacks. | ||
|
|
||
| A previous design had no limits for retrying commits or entire transactions. The callback is always able indicate that | ||
| A previous design had no limits for retrying commits or entire transactions. The callback is always able to indicate that |
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.
Maybe I read it wrong, or maybe its a typo? Not sure.
| [abortTransaction](../transactions/transactions.md#aborttransaction) on the session. | ||
| 2. If the callback's error includes a "TransientTransactionError" label and the elapsed time of `withTransaction` is | ||
| less than 120 seconds, jump back to step two. | ||
| less than 120 seconds, sleep for `jitter * min(BACKOFF_INITIAL * (1.25**retry), BACKOFF_MAX)` where: |
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.
I know python uses ** as the exponential operator. I don't believe that is a standard across other programming languages. Is it clear that its exponent? Is there a different preferred symbol for exponent? (I know math commonly uses ^ but it typically also means bitwise XOR in code so I felt like that could be confusing.)
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.
** seems fine to me, but I might also be biased because that's what Node does 🙂. But I don't think people will have trouble understanding what this means.
source/transactions-convenient-api/transactions-convenient-api.md
Outdated
Show resolved
Hide resolved
source/transactions-convenient-api/transactions-convenient-api.md
Outdated
Show resolved
Hide resolved
source/transactions-convenient-api/transactions-convenient-api.md
Outdated
Show resolved
Hide resolved
source/transactions-convenient-api/transactions-convenient-api.md
Outdated
Show resolved
Hide resolved
source/transactions-convenient-api/transactions-convenient-api.md
Outdated
Show resolved
Hide resolved
source/transactions-convenient-api/transactions-convenient-api.md
Outdated
Show resolved
Hide resolved
ShaneHarvey
left a comment
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.
LGTM
comandeo-mongo
left a comment
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.
LGTM! Maybe we can add a link to the reference implementation to PRs description.
Good suggestion! I added a link to the python PR in the PR description now! |
Please complete the following before merging:
clusters).
python implementation: mongodb/mongo-python-driver#2600