CSHARP-5712: withTransaction API retries too frequently #1841

sanych-sun · 2025-12-19T01:01:30Z

No description provided.

Copilot

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 GetRetryDelayMs method with exponential backoff calculation and random jitter
Removes unused MongoDB.Driver.Core.Connections import 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.

src/MongoDB.Driver/Core/Operations/RetryabilityHelper.cs

Copilot · 2025-12-19T01:02:56Z

src/MongoDB.Driver/Core/Operations/RetryabilityHelper.cs

+            Ensure.IsGreaterThan(backoffMax, backoffInitial, nameof(backoffMax));
+
+            var j = __backoffRandom.NextDouble();
+            return (int)(j * Math.Min(backoffMax, backoffInitial * Math.Pow(backoffBase, attempt - 1)));


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.

Suggested change

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);

Copilot

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.

src/MongoDB.Driver/Core/Operations/RetryabilityHelper.cs

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/MongoDB.Driver/TransactionExecutor.cs

papafe

Looks good!

papafe · 2026-01-05T18:11:46Z

tests/MongoDB.Driver.Tests/ClientSessionHandleTests.cs

            }

            var subject = CreateSubject(coreSession: mockCoreSession.Object, clock: mockClock.Object);
+            SetupTransactionState(subject, true);


Is this related to the ticket?

Yes, changes were rolled back. Thank you for noticing this.

BorisDog

Partial review

src/MongoDB.Driver/Core/Misc/IRandomNumberGenerator.cs

BorisDog · 2026-01-05T23:07:53Z

src/MongoDB.Driver/TransactionExecutor.cs

-                {
-                    return callbackOutcome.Result;
+                    var delay = GetRetryDelay(randomNumberGenerator, attempt);
+                    if (HasTimedOut(operationContext, delay))


IsTimedOut?

src/MongoDB.Driver/TransactionExecutor.cs

src/MongoDB.Driver/Core/Misc/IRandomNumberGenerator.cs

src/MongoDB.Driver/Core/Misc/ThreadStaticRandom.cs

src/MongoDB.Driver/TransactionExecutor.cs

src/MongoDB.Driver/Core/Misc/RandomNumberGenerator.cs

tests/MongoDB.Driver.Tests/Core/Misc/DefaultRandomTests.cs

papafe · 2026-01-06T09:03:42Z

tests/MongoDB.Driver.Tests/ClientSessionHandleTests.cs

+            var backoffTime = await ExecuteWithTransactionAsync(1);
+
+            var difference = backoffTime - noBackoffTime - TimeSpan.FromSeconds(2.2);
+            difference.Should().BeLessThan(TimeSpan.FromMilliseconds(1));


Are we using a 1 millisecond window here instead of a 1 second because we're "faking" the prose test?

Yep. We do not interact with the server, so timing should be more strict. 1ms could be small though, we might will need to adjust it in future.

src/MongoDB.Driver/Core/Misc/DefaultRandom.cs

BorisDog · 2026-01-06T19:01:03Z

src/MongoDB.Driver/Core/Misc/DefaultRandom.cs

+
+        return sb.ToString();
+#else
+        return string.Create(length, legalCharacters, (buffer, charset) =>


tests/MongoDB.Driver.Tests/Core/Misc/DefaultRandomTests.cs

src/MongoDB.Driver/TransactionExecutor.cs

BorisDog · 2026-01-06T23:21:18Z

src/MongoDB.Driver/TransactionExecutor.cs

+                return callback(clientSession, cancellationToken);
            }
-            catch (Exception ex)
+            catch (Exception) when (IsTransactionInStartingOrInProgressState(clientSession))


Will
catch when (IsTransactionInStartingOrInProgressState(clientSession))
work?

The entire method was removed.

src/MongoDB.Driver/TransactionExecutor.cs

BorisDog · 2026-01-07T00:04:12Z

tests/MongoDB.Driver.Tests/ClientSessionHandleTests.cs

+            var noBackoffTime = await ExecuteWithTransactionAsync(0);
+            var backoffTime = await ExecuteWithTransactionAsync(1);
+
+            var difference = backoffTime - noBackoffTime - TimeSpan.FromSeconds(2.2);


Will
backoffTimeMS.Should().BeApproximately(noBackoffTimeMS + 2200, 1);
be more readable?

BorisDog · 2026-01-07T00:05:51Z

tests/MongoDB.Driver.Tests/Core/Operations/RetryabilityHelperTests.cs

+        [InlineData(1, 2, 100, -1, "backoffMax")]
+        [InlineData(1, 2, 100, 0, "backoffMax")]
+        [InlineData(1, 2, 100, 50, "backoffMax")]
+


BorisDog · 2026-01-07T00:09:34Z

tests/MongoDB.Driver.Tests/Core/Operations/RetryabilityHelperTests.cs

+        [InlineData(2, 1.5, 100, 10000, 0, 150)]
+        [InlineData(3, 1.5, 100, 10000, 0, 225)]
+        [InlineData(9999, 1.5, 100, 10000, 0, 10000)]
+        public void GetRetryDelayMs_should_return_expected_results(int attempt, double backoffBase, int backoffInitial, int backoffMax, int expectedRangeMin, int expectedRangeMax)


minor: expected_result

BorisDog

Minor suggestion

BorisDog · 2026-01-08T23:17:53Z

src/MongoDB.Driver/TransactionExecutor.cs

                {
-                    return callbackOutcome.Result; // assume callback intentionally ended the transaction
-                }
+                    if (IsTransactionInStartingOrInProgressState(clientSession))


Similar to ShouldRetryTransaction consider ShouldAbortTransaction(out AbortTransactionOptions)

CSHARP-5712: withTransaction API retries too frequently

1069e25

Copilot AI review requested due to automatic review settings December 19, 2025 01:01

sanych-sun requested a review from a team as a code owner December 19, 2025 01:01

sanych-sun requested review from adelinowona and papafe and removed request for adelinowona December 19, 2025 01:01

sanych-sun added the chore Non–user-facing code changes (tests, build scripts, etc.). label Dec 19, 2025

Copilot AI reviewed Dec 19, 2025

View reviewed changes

pr

9577f33

sanych-sun requested a review from Copilot December 19, 2025 01:11

Copilot AI reviewed Dec 19, 2025

View reviewed changes

src/MongoDB.Driver/Core/Operations/RetryabilityHelper.cs Outdated Show resolved Hide resolved

CSHARP-5712: withTransaction API retries too frequently

c2cef3b

sanych-sun added feature Adds new user-facing functionality. and removed chore Non–user-facing code changes (tests, build scripts, etc.). labels Dec 19, 2025

sanych-sun requested review from BorisDog and Copilot December 19, 2025 23:21

Copilot AI reviewed Dec 19, 2025

View reviewed changes

src/MongoDB.Driver/TransactionExecutor.cs Outdated Show resolved Hide resolved

sanych-sun added 2 commits December 19, 2025 15:56

Pr

c5c3dcd

pr

fcc6211

papafe reviewed Jan 5, 2026

View reviewed changes

sanych-sun added 2 commits January 5, 2026 13:58

pr

3d13518

pr

2d7db04

BorisDog requested changes Jan 5, 2026

View reviewed changes

Pr

ec5819d

sanych-sun requested a review from BorisDog January 5, 2026 23:32

BorisDog reviewed Jan 6, 2026

View reviewed changes

sanych-sun added 2 commits January 5, 2026 19:59

Refactor IRandomStringGenerator and IRandomNumberGenerator into IRandom.

e93a147

Pr

ae36281

sanych-sun requested a review from BorisDog January 6, 2026 04:18

papafe reviewed Jan 6, 2026

View reviewed changes

BorisDog requested changes Jan 7, 2026

View reviewed changes

sanych-sun added 3 commits January 7, 2026 10:12

pr

a806040

pr

dccac18

pr

b532afb

sanych-sun requested a review from BorisDog January 8, 2026 20:24

BorisDog requested changes Jan 8, 2026

View reviewed changes

-            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);

CSHARP-5712: withTransaction API retries too frequently #1841

Are you sure you want to change the base?

CSHARP-5712: withTransaction API retries too frequently #1841

Uh oh!

Conversation

sanych-sun commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

papafe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BorisDog left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BorisDog left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

sanych-sun commented Dec 19, 2025 •

edited

Loading