Skip to content

Conversation

@nhachicha
Copy link
Contributor

@nhachicha nhachicha self-assigned this Dec 3, 2025
@stIncMale stIncMale self-requested a review December 3, 2025 18:11
@nhachicha nhachicha requested a review from Copilot December 4, 2025 13:53
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 exponential backoff with jitter for transaction retries in the MongoDB Java driver's convenient transaction API (withTransaction). The implementation follows the specification with a growth factor of 1.5, base delay of 5ms, and maximum delay of 500ms.

Key Changes:

  • Added ExponentialBackoff utility class to calculate retry delays using exponential backoff with random jitter
  • Modified ClientSessionImpl.withTransaction() to apply exponential backoff before each transaction retry attempt
  • Enhanced timeout handling to check limits before applying backoff delays

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
driver-core/src/main/com/mongodb/internal/ExponentialBackoff.java New utility class implementing exponential backoff algorithm with factory methods for transaction and command retries
driver-core/src/test/unit/com/mongodb/internal/ExponentialBackoffTest.java Comprehensive unit tests verifying backoff calculations, jitter application, and max delay capping
driver-sync/src/main/com/mongodb/client/internal/ClientSessionImpl.java Integration of exponential backoff into transaction retry logic with timeout checks
driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java Functional test validating exponential backoff behavior with simulated transient errors
Comments suppressed due to low confidence (1)

driver-core/src/main/com/mongodb/internal/ExponentialBackoff.java:1

  • The commented-out wouldExceedTimeLimit() method and its associated tests should be removed if they are not being used. Keeping dead code reduces code maintainability and can confuse future developers about the intended design.
/*

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

// Expected delays (without jitter): 5ms, 7.5ms, 11.25ms
// With jitter, actual delays will be between 0 and these values
// 3 retries with backoff should take at least a few milliseconds
assertTrue(elapsedTime > 5, "Expected backoff delays to be applied");
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assertion threshold of 5ms is extremely fragile and may cause test flakiness on slow or heavily loaded CI systems. The test could fail even when backoff is working correctly due to timing variations. Consider either removing this timing assertion entirely (since the retry count assertion already validates the behavior) or using a more lenient threshold with a clear comment explaining the expected minimum delay calculation.

Suggested change
assertTrue(elapsedTime > 5, "Expected backoff delays to be applied");
// Timing assertion removed to avoid test flakiness on slow CI systems.

Copilot uses AI. Check for mistakes.
throw withTransactionTimeoutContext.hasTimeoutMS()
? new MongoOperationTimeoutException("Transaction retry exceeded the timeout limit")
: new MongoTimeoutException("Transaction retry time limit exceeded");

Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Extra blank line creates inconsistent spacing. Remove the blank line at 272 for consistent code formatting.

Suggested change

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants