JAVA-5950 Update Transactions Convenient API with exponential backoff on retries #1852

nhachicha · 2025-12-09T13:47:53Z

Relevant specification changes:

DRIVERS-1934: withTransaction API retries too frequently (#1851)
DRIVERS-1934: clarify drivers back off before all transaction retries (#1868) - the change has not yet been addressed in this PR, because it was introduced later.

…ff on retries

Copilot

Pull request overview

This PR implements exponential backoff with jitter for transaction retries in MongoDB's withTransaction convenience API. The implementation adds a configurable backoff mechanism that applies delays between retry attempts when transient transaction errors occur, following the MongoDB specification with a growth factor of 1.5 for transactions.

Key Changes

Introduces ExponentialBackoff utility class with factory methods for transaction retries (5ms base, 500ms max, 1.5x growth) and command retries (100ms base, 10s max, 2.0x growth)
Integrates backoff logic into ClientSessionImpl.withTransaction() to delay between retry attempts
Adjusts test configuration to verify backoff behavior with multiple retry attempts

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 with jitter using ThreadLocalRandom
driver-sync/src/main/com/mongodb/client/internal/ClientSessionImpl.java	Adds backoff delay before transaction retries and uses CSOT timeout when available
driver-core/src/test/unit/com/mongodb/internal/ExponentialBackoffTest.java	Comprehensive unit tests validating backoff calculations, growth factors, and maximum caps
driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java	New functional test verifying exponential backoff behavior and adjusted existing test configuration

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

driver-sync/src/main/com/mongodb/client/internal/ClientSessionImpl.java

Copilot · 2025-12-09T14:25:58Z

driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java

+            AtomicInteger retryCount = new AtomicInteger(0);
+
+            session.withTransaction(() -> {
+                retryCount.incrementAndGet();  // Count the attempt before the operation that might fail


The test verifies the retry count but does not validate that exponential backoff delays are actually applied. Consider measuring elapsed time and asserting minimum expected delays to ensure backoff is functioning correctly. For example, with 3 retries at delays of ~5ms, ~7.5ms, and ~11.25ms, the total elapsed time should be at least the sum of minimum expected delays.

ExponentialBackoffTest covers these unit tests already.

…Impl.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…s exceeded (ex operationContext.getTimeoutContext().getReadTimeoutMS())

dariakp · 2025-12-17T19:43:50Z

@nhachicha Please take note of mongodb/specifications#1868

stIncMale

I haven't reviewed ExponentialBackoffTest, because it depends on ExponentialBackoff, where I left many suggestions.
I haven't reviewed ClientSessionImpl, because it has to implement the new specification change DRIVERS-1934: clarify drivers back off before all transaction retries (#1868).
The last reviewed commit is 90ec4d5.

stIncMale · 2026-01-02T22:15:51Z

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

+     *
+     * @return ExponentialBackoff configured for command retries
+     */
+    public static ExponentialBackoff forCommandRetry() {


Let's not introduce unused code that we foresee to be needed in the future. In this PR, we only need backoffs for the convenient transaction API.

stIncMale · 2026-01-02T22:32:06Z

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

+    public static ExponentialBackoff forTransactionRetry() {
+        return new ExponentialBackoff(
+                TRANSACTION_BASE_BACKOFF_MS,
+                TRANSACTION_MAX_BACKOFF_MS,
+                TRANSACTION_BACKOFF_GROWTH
+        );
+    }


Within this PR, I failed to find any benefits of expressing the backoff computation as behavior of an object (an instance of ExponentialBackoff), rather than just a static method; regardless of the approach, the ClientSessionImpl.withTransaction method has to maintain one new local variable: either the lazily initialized transactionBackoff, or the transactionAttempt (that's how it is named in the spec, but we are free to use a different name). Given this, I propose to go with the more straightforward approach of expressing the backoff computation as a static method¹, rather than as an object behavior.

If in the future we observe that this is not enough and the "object" approach is needed, we'll be able to change the approach. But we will have a clear reason for that.

Also, storing all the constants in each instance of ExponentialBackoff as instance fields is unnecessary. If we have to use the "object" approach in the future, we better implement it in such a way that does not duplicate constants as instance fields in each object instance (an interface / abstract class with two implementations is one way to achieve that).

P.S. Some other review comments I left are written within the current "object" approach (as opposed to he proposed "method" approach). If the suggestion in this comment is applied, those comments should not be automatically discarded, but rather each should be examined and adopted, if applicable, to the "method" approach.

¹ If in the future we need another static method for command retries, we will be able to move the computational logic in a private static method, and call that method from two public methods, passing the suitable constants as method arguments.

stIncMale · 2026-01-02T22:35:45Z

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

+ * limitations under the License.
+ */
+
+package com.mongodb.internal;


Maybe the existing com.mongodb.internal.time package is a better place for ExponentialBackoff?

stIncMale · 2026-01-02T22:39:56Z

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

+ *   <li>{@link #forTransactionRetry()} - For withTransaction retries (5ms base, 500ms max, 1.5 growth)</li>
+ *   <li>{@link #forCommandRetry()} - For command retries with overload (100ms base, 10000ms max, 2.0 growth)</li>


Let's not duplicate method-level documentation in the class-level documentation. Any duplication opens up opportunities for inconsistencies. We duplicate public API documentation for the sake of user convenience (though I has always been opposed to that, because it have lead to inconsistencies, and will lead to them again, which harm users), but internally, there seem to be no reason to duplicate documentation at all.

stIncMale · 2026-01-02T23:00:58Z

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

+
+    /**
+     * Creates a backoff instance configured for withTransaction retries.
+     * Uses: 5ms base, 500ms max, 1.5 growth factor.


If you insist that documenting this internally is valuable, which I doubt, let's do that in a way that prevents the documentation and the actual values from becoming inconsistent:

Suggested change

* Uses: 5ms base, 500ms max, 1.5 growth factor.

* Uses: {@value TRANSACTION_BASE_BACKOFF_MS} ms base, {@value TRANSACTION_MAX_BACKOFF_MS} ms max,

* {@value TRANSACTION_BACKOFF_GROWTH} growth factor.

P.S. Notice that I used a space between the numerical value and unit symbol. This is in accordance with the "5.4.3 Formatting the value of a quantity" section in The International System of Units (SI) Brochure released by The International Bureau of Weights and Measures (BIPM).

stIncMale · 2026-01-03T10:13:57Z

driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java

+            session.withTransaction(() -> {
+                retryCount.incrementAndGet();  // Count the attempt before the operation that might fail
+                collection.insertOne(session, Document.parse("{ _id : 'backoff-test' }"));
+                return retryCount;
+            });


Let's simplify:

Suggested change

session.withTransaction(() -> {

retryCount.incrementAndGet(); // Count the attempt before the operation that might fail

collection.insertOne(session, Document.parse("{ _id : 'backoff-test' }"));

return retryCount;

});

session.withTransaction(() -> {

retryCount.incrementAndGet(); // Count the attempt before the operation that might fail

return collection.insertOne(session, Document.parse("{ _id : 'backoff-test' }"));

});

stIncMale · 2026-01-03T10:21:06Z

driver-sync/src/main/com/mongodb/client/internal/ClientSessionImpl.java

+                    } catch (InterruptedException e) {
+                        Thread.currentThread().interrupt();
+                        throw new MongoClientException("Transaction retry interrupted", e);


Let's use InterruptionUtil.interruptAndCreateMongoInterruptedException.

stIncMale · 2026-01-03T10:23:17Z

driver-sync/src/main/com/mongodb/client/internal/ClientSessionImpl.java

+                    long backoffMs = transactionBackoff.calculateDelayMs();
+                    try {
+                        if (backoffMs > 0) {
+                            Thread.sleep(backoffMs);
+                        }
+                    } catch (InterruptedException e) {
+                        Thread.currentThread().interrupt();
+                        throw new MongoClientException("Transaction retry interrupted", e);
+                    }


Let's extract this code to a private static method. The withTransaction method is already too long, we should not make it longer without good reason.

stIncMale · 2026-01-03T10:23:48Z

driver-sync/src/main/com/mongodb/client/internal/ClientSessionImpl.java

        try {
            outer:
            while (true) {
+                // Apply exponential backoff before retrying transaction


I don't think this comment adds anything of value to a reader. Let's remove it.

stIncMale · 2026-01-03T10:28:09Z

driver-sync/src/main/com/mongodb/client/internal/ClientSessionImpl.java

    @Override
    public <T> T withTransaction(final TransactionBody<T> transactionBody, final TransactionOptions options) {
        notNull("transactionBody", transactionBody);
        long startTime = ClientSessionClock.INSTANCE.now();


[just a comment on a code that wasn't changed in this PR]

I have just noticed this ClientSessionClock - it uses non-monotonic clock. Horrendous.

nhachicha added 2 commits December 3, 2025 17:45

JAVA-5950 - Update Transactions Convenient API with exponential backo…

bd98229

…ff on retries

Simplifying test, clean up.

72cd714

nhachicha self-assigned this Dec 9, 2025

nhachicha mentioned this pull request Dec 9, 2025

JAVA-5950 Update Transactions Convenient API with exponential backoff on retries #1848

Closed

Fixing test

573c86c

nhachicha requested a review from Copilot December 9, 2025 14:25

Copilot AI reviewed Dec 9, 2025

View reviewed changes

nhachicha marked this pull request as ready for review December 9, 2025 15:24

nhachicha requested a review from a team as a code owner December 9, 2025 15:24

nhachicha requested review from stIncMale and strogiyotec and removed request for strogiyotec December 9, 2025 15:24

nhachicha and others added 2 commits December 9, 2025 15:32

Update driver-sync/src/main/com/mongodb/client/internal/ClientSession…

ecf9b4d

…Impl.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

retrigger checks

68dbece

vbabanin self-requested a review December 9, 2025 16:11

nhachicha added 13 commits December 9, 2025 17:01

retrigger checks

7cfc4c4

retrigger checks

cd96a49

retrigger checks

82dc8b1

test cleanup

558bfad

retrigger checks

fbe881e

Test cleanup

d89c714

retrigger checks

2eccff0

Update the implementation according to the spec

c406ae7

Added prose test

a28ce98

Flaky test

27ddef9

Remove extra Test annotation

3bef60a

Throwing correct exception when CSOT is used

b051701

Simplifying implementation by relying on CSOT to throw when timeout i…

90ec4d5

…s exceeded (ex operationContext.getTimeoutContext().getReadTimeoutMS())

stIncMale requested changes Jan 3, 2026

View reviewed changes

		* <li>{@link #forTransactionRetry()} - For withTransaction retries (5ms base, 500ms max, 1.5 growth)</li>
		* <li>{@link #forCommandRetry()} - For command retries with overload (100ms base, 10000ms max, 2.0 growth)</li>

	* Uses: 5ms base, 500ms max, 1.5 growth factor.
	* Uses: {@value TRANSACTION_BASE_BACKOFF_MS} ms base, {@value TRANSACTION_MAX_BACKOFF_MS} ms max,
	* {@value TRANSACTION_BACKOFF_GROWTH} growth factor.

JAVA-5950 Update Transactions Convenient API with exponential backoff on retries #1852

Are you sure you want to change the base?

JAVA-5950 Update Transactions Convenient API with exponential backoff on retries #1852

Conversation

nhachicha commented Dec 9, 2025 • edited by stIncMale 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 9, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dariakp commented Dec 17, 2025

Uh oh!

stIncMale 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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nhachicha commented Dec 9, 2025 •

edited by stIncMale

Loading