Skip to content

Conversation

@sanych-sun
Copy link
Member

PR to implement RetryabilityHelper.GetRetryDelayMs. Please do not merge, depending on the comment/question in the Driver's PR:
mongodb/specifications#1862 (comment)

Copilot AI review requested due to automatic review settings December 19, 2025 01:01
@sanych-sun sanych-sun requested a review from a team as a code owner December 19, 2025 01:01
@sanych-sun sanych-sun requested review from adelinowona and papafe and removed request for adelinowona December 19, 2025 01:01
@sanych-sun sanych-sun added the chore Non–user-facing code changes (tests, build scripts, etc.). label Dec 19, 2025
Copy link
Contributor

Copilot AI left a 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 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.

Ensure.IsGreaterThan(backoffMax, backoffInitial, nameof(backoffMax));

var j = __backoffRandom.NextDouble();
return (int)(j * Math.Min(backoffMax, backoffInitial * Math.Pow(backoffBase, attempt - 1)));
Copy link

Copilot AI Dec 19, 2025

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.

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 uses AI. Check for mistakes.
@sanych-sun sanych-sun requested a review from Copilot December 19, 2025 01:11
Copy link
Contributor

Copilot AI left a 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();
Copy link

Copilot AI Dec 19, 2025

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.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Non–user-facing code changes (tests, build scripts, etc.).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant