-
Notifications
You must be signed in to change notification settings - Fork 1.3k
CSHARP-5712: withTransaction API retries too frequently #1841
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull request overview
This PR implements the RetryabilityHelper.GetRetryDelayMs method to control retry delays with exponential backoff, addressing the issue of transactions retrying too frequently. The implementation adds jitter to prevent thundering herd problems and includes comprehensive parameter validation.
Key Changes
- Adds
GetRetryDelayMsmethod with exponential backoff calculation and random jitter - Removes unused
MongoDB.Driver.Core.Connectionsimport from test file - Adds comprehensive test coverage for valid inputs, edge cases, and parameter validation
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/MongoDB.Driver/Core/Operations/RetryabilityHelper.cs | Implements the core retry delay calculation with exponential backoff and jitter |
| tests/MongoDB.Driver.Tests/Core/Operations/RetryabilityHelperTests.cs | Adds unit tests for the new method and removes unused import |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Ensure.IsGreaterThan(backoffMax, backoffInitial, nameof(backoffMax)); | ||
|
|
||
| var j = __backoffRandom.NextDouble(); | ||
| return (int)(j * Math.Min(backoffMax, backoffInitial * Math.Pow(backoffBase, attempt - 1))); |
Copilot
AI
Dec 19, 2025
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.
The calculation backoffInitial * Math.Pow(backoffBase, attempt - 1) may overflow for large attempt values, which would then be capped by Math.Min. Consider adding overflow protection or documenting the maximum safe attempt value to prevent unexpected behavior.
| return (int)(j * Math.Min(backoffMax, backoffInitial * Math.Pow(backoffBase, attempt - 1))); | |
| // compute the largest exponent such that backoffInitial * backoffBase^exponent <= backoffMax | |
| var maxExponent = Math.Log(backoffMax / (double)backoffInitial, backoffBase); | |
| var effectiveExponent = attempt - 1; | |
| double delayWithoutJitter; | |
| if (effectiveExponent >= maxExponent) | |
| { | |
| delayWithoutJitter = backoffMax; | |
| } | |
| else | |
| { | |
| delayWithoutJitter = backoffInitial * Math.Pow(backoffBase, effectiveExponent); | |
| } | |
| return (int)(j * delayWithoutJitter); |
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.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #if NET6_0_OR_GREATER | ||
| var j = Random.Shared.NextDouble(); | ||
| #else | ||
| var j = (new Random()).NextDouble(); |
Copilot
AI
Dec 19, 2025
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.
Creating a new Random instance for each call can lead to predictable values when called in quick succession due to time-based seeding. Consider using a static ThreadLocal instance for .NET Framework targets to ensure better randomness in concurrent scenarios.
PR to implement
RetryabilityHelper.GetRetryDelayMs. Please do not merge, depending on the comment/question in the Driver's PR:mongodb/specifications#1862 (comment)