JAVA-5950 Update Transactions Convenient API with exponential backoff on retries by nhachicha · Pull Request #1899 · mongodb/mongo-java-driver

nhachicha · 2026-02-25T13:18:17Z

Original PR accidentally closed #1852, it has outstanding review comments for @stIncMale to go over when re-reviewing.

Relevant specification changes:

DRIVERS-1934: withTransaction API retries too frequently (#1851)
DRIVERS-1934: clarify drivers back off before all transaction retries (#1868)
DRIVERS-3364: Fix Retry Backoff is Enforced prose test.
DRIVERS-3391: Clarify withTransaction CSOT timeout error with backoff+jitter - this change has not yet been implemented in this PR.
DRIVERS-3370: Retry backoff jitter interval may include 1.0 (#1905) - nothing to change in the PR.

JAVA-5950, JAVA-6046, JAVA-6093, JAVA-6113 (only the part about transactions-convenient-api)

…ff on retries

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

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

…odb/specifications#1868

…tionProseTest.java Co-authored-by: Valentin Kovalenko <valentin.male.kovalenko@gmail.com>

Co-authored-by: Valentin Kovalenko <valentin.male.kovalenko@gmail.com>

…mmediate retry without backoff.

- Adding logic for DRIVERS-3391

…lBackoffTest.java Co-authored-by: Valentin Kovalenko <valentin.male.kovalenko@gmail.com>

…tionProseTest.java Co-authored-by: Valentin Kovalenko <valentin.male.kovalenko@gmail.com>

stIncMale · 2026-03-06T23:04:23Z

The commit corresponding to the last reviewed commit is the previous PR is fda498c.

stIncMale

I haven't finished reviewing the changes.

stIncMale · 2026-03-09T20:16:09Z

driver-core/src/test/unit/com/mongodb/internal/time/ExponentialBackoffTest.java

+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class ExponentialBackoffTest {


Original suggestion #1852 (comment) was auto-resolved, but the corresponding change 1655dad hasn't been included in the current PR:

Suggested change

public class ExponentialBackoffTest {

class ExponentialBackoffTest {

stIncMale · 2026-03-09T20:21:37Z

driver-core/src/test/unit/com/mongodb/internal/time/ExponentialBackoffTest.java

+                String.format("Attempt %d: backoff should be 0-%d ms, got: %d", attemptNumber,
+                        expectedBackoff, backoff));


Original suggestion #1852 (comment) was auto-resolved, but the corresponding change f5274cb hasn't been fully included in the current PR:

Suggested change

String.format("Attempt %d: backoff should be 0-%d ms, got: %d", attemptNumber,

expectedBackoff, backoff));

String.format("Attempt %d: backoff should be between 0 ms and %d ms, got: %d", attemptNumber,

expectedBackoff, backoff));

stIncMale · 2026-03-09T20:40:02Z

driver-core/src/test/unit/com/mongodb/internal/time/ExponentialBackoffTest.java

+    @Test
+    void testCalculateTransactionBackoffMsRespectsMaximum() {
+        // Even at high attempt numbers, backoff should never exceed ExponentialBackoff.TRANSACTION_MAX_MS
+        for (int attemptNumber = 1; attemptNumber < 26; attemptNumber++) {


Let's make the number of iterations here based on EXPECTED_BACKOFFS_MAX_VALUES.length (this way it is more clear than when a magic number like 26 is used)

Suggested change

for (int attemptNumber = 1; attemptNumber < 26; attemptNumber++) {

for (int attemptNumber = 1; attemptNumber < EXPECTED_BACKOFFS_MAX_VALUES.length * 2; attemptNumber++) {

stIncMale · 2026-03-09T21:25:28Z

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

        }
    }

+    private long measureTransactionLatency(final double jitter, final BsonDocument failPointDocument) throws InterruptedException {


Let's either specify the units returned in the method name, like measureTransactionLatencyMs, or, better, return Duration, and let the calling code do the conversion by calling, for example, Duration.toMillis, if needed.

stIncMale · 2026-03-09T21:35:59Z

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

    public void testRetryBackoffIsEnforced() throws InterruptedException {
-        // Run with jitter = 0 (no backoff)
-        ExponentialBackoff.setTestJitterSupplier(() -> 0.0);
-
-        BsonDocument failPointDocument = BsonDocument.parse("{'configureFailPoint': 'failCommand', 'mode': {'times': 13}, "
-                + "'data': {'failCommands': ['commitTransaction'], 'errorCode': 251}}");
-
-        long noBackoffTime;
-        try (ClientSession session = client.startSession();
-             FailPoint ignored = FailPoint.enable(failPointDocument, getPrimary())) {
-            StartTime startTime = StartTime.now();
-            session.withTransaction(() -> collection.insertOne(session, Document.parse("{}")));
-            noBackoffTime = startTime.elapsed().toMillis();
-        } finally {
-            // Clear the test jitter supplier to avoid affecting other tests
-            ExponentialBackoff.clearTestJitterSupplier();
-        }
-
-        // Run with jitter = 1 (full backoff)
-        ExponentialBackoff.setTestJitterSupplier(() -> 1.0);
-
-        failPointDocument = BsonDocument.parse("{'configureFailPoint': 'failCommand', 'mode': {'times': 13}, "
+        final BsonDocument failPointDocument = BsonDocument.parse("{'configureFailPoint': 'failCommand', 'mode': {'times': 13}, "
                + "'data': {'failCommands': ['commitTransaction'], 'errorCode': 251}}");

-        long withBackoffTime;
-        try (ClientSession session = client.startSession();
-             FailPoint ignored = FailPoint.enable(failPointDocument, getPrimary())) {
-            StartTime startTime = StartTime.now();
-            session.withTransaction(() -> collection.insertOne(session, Document.parse("{}")));
-            withBackoffTime = startTime.elapsed().toMillis();
-        } finally {
-            ExponentialBackoff.clearTestJitterSupplier();
-        }
+        long noBackoffTime = measureTransactionLatency(0.0, failPointDocument);
+        long withBackoffTime = measureTransactionLatency(1.0, failPointDocument);


Code deduplication in this test was achieved by extracting the duplicated code into measureTransactionLatency. There was a separate comment about the duplicated string that represents the failpoint document, but that comment wasn't about saving the computing resources by avoiding to parse the document more than once.

Having failPointDocument outside of the measureTransactionLatency method hurts readability. It may in principle be tolerable if the performance improvement its provides is worth the compromise. However, that's not the case here, as performance is mostly irrelevant.

Let's move the whole final BsonDocument failPointDocument = BsonDocument.parse... into failPointDocument.

stIncMale · 2026-03-11T02:38:23Z

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

    public <T> T withTransaction(final TransactionBody<T> transactionBody, final TransactionOptions options) {
        notNull("transactionBody", transactionBody);
        TimeoutContext withTransactionTimeoutContext = createTimeoutContext(options);
+        final boolean hasTimeoutMS = withTransactionTimeoutContext.hasTimeoutMS();


[just information]

Us using both timeoutOrAlternative (or getTimeout) and hasTimeoutMS now got me thinking if it's obvious that getTimeout() is null iff getTimeoutSettings().getTimeoutMS() is null. It is not, I had to spend time analyzing the constructors and how they are called to verify that. To make this invariant explicit, and to enforce it, I created #1908.

stIncMale · 2026-03-11T02:56:02Z

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

        }
    }
+
+    private static MongoException timeoutException(final boolean hasTimeoutMS, final Throwable cause) {


Let's rename hasTimeoutMS to something like timeoutMsConfigured or csotConfigured? That way the name does not beg the question "what is the thing that has the timeoutMS?".

The same applies to the hasTimeoutMS parameter of the backoff method, and to the local variable hasTimeoutMS in the withTransaction method.

stIncMale · 2026-03-11T04:33:54Z

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

    private TimeoutContext(final TimeoutSettings timeoutSettings, @Nullable final Timeout timeout) {
        this(false, timeoutSettings, timeout);
    }


When we removed the copyTimeoutContext method in this PR, the constructor TimeoutContext(final TimeoutSettings timeoutSettings, @Nullable final Timeout timeout) became dead code. Let's remove the constructor.

stIncMale · 2026-03-11T04:42:59Z

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

+        final boolean hasTimeoutMS = withTransactionTimeoutContext.hasTimeoutMS();
+        Timeout withTransactionTimeout = withTransactionTimeoutContext.timeoutOrAlternative(
+                assertNotNull(TimeoutContext.startTimeout(MAX_RETRY_TIME_LIMIT_MS)));


Since we now have to call hasTimeoutMS, the timeoutOrAlternative we added is longer useful, as it was introduced specifically to avoid doing the explicit hasTimeoutMS check.

Let's remove timeoutOrAlternative, and change the code here as follows (note also how the local variable hasTimeoutMS is no longer final, because in this codebase we don't mark local variables as final):

Suggested change

final boolean hasTimeoutMS = withTransactionTimeoutContext.hasTimeoutMS();

Timeout withTransactionTimeout = withTransactionTimeoutContext.timeoutOrAlternative(

assertNotNull(TimeoutContext.startTimeout(MAX_RETRY_TIME_LIMIT_MS)));

boolean hasTimeoutMS = withTransactionTimeoutContext.hasTimeoutMS();

Timeout withTransactionTimeout = assertNotNull(hasTimeoutMS

? withTransactionTimeoutContext.getTimeout()

: TimeoutContext.startTimeout(MAX_RETRY_TIME_LIMIT_MS));

stIncMale · 2026-03-11T04:45:01Z

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

+    public Timeout timeoutOrAlternative(final Timeout alternative) {
+        return timeout == null ? alternative : timeout;
+    }
+


Let's remove this new method. See #1899 (comment) for the details.

Suggested change

public Timeout timeoutOrAlternative(final Timeout alternative) {

return timeout == null ? alternative : timeout;

}

strogiyotec and others added 30 commits February 25, 2026 13:00

Version: bump 5.7.0-beta0

7b27940

Version: bump 5.7.0-SNAPSHOT

1094102

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

024e420

…ff on retries

Simplifying test, clean up.

8071de6

Fixing test

0f6e118

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

19f296f

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

retrigger checks

455f352

retrigger checks

85ec5ef

retrigger checks

5f2157a

retrigger checks

f94d9c7

test cleanup

8c322c6

retrigger checks

14f339d

Test cleanup

f593950

retrigger checks

d12e1f3

Update the implementation according to the spec

26da297

Added prose test

a1d5bca

Flaky test

e8857e0

Remove extra Test annotation

b8d7abf

Throwing correct exception when CSOT is used

c2a91bf

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

8957883

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

Fixing implementation according to spec changes in JAVA-6046 and mong…

e008a43

…odb/specifications#1868

Update driver-sync/src/test/functional/com/mongodb/client/WithTransac…

a54d95a

…tionProseTest.java Co-authored-by: Valentin Kovalenko <valentin.male.kovalenko@gmail.com>

Update driver-sync/src/test/functional/com/mongodb/client/WithTransac…

553d6ba

…tionProseTest.java Co-authored-by: Valentin Kovalenko <valentin.male.kovalenko@gmail.com>

Update driver-sync/src/test/functional/com/mongodb/client/WithTransac…

c0136e1

…tionProseTest.java Co-authored-by: Valentin Kovalenko <valentin.male.kovalenko@gmail.com>

Update driver-sync/src/test/functional/com/mongodb/client/WithTransac…

09a1291

…tionProseTest.java Co-authored-by: Valentin Kovalenko <valentin.male.kovalenko@gmail.com>

Update driver-sync/src/test/functional/com/mongodb/client/WithTransac…

520fead

…tionProseTest.java Co-authored-by: Valentin Kovalenko <valentin.male.kovalenko@gmail.com>

Update driver-sync/src/test/functional/com/mongodb/client/WithTransac…

478e242

…tionProseTest.java Co-authored-by: Valentin Kovalenko <valentin.male.kovalenko@gmail.com>

Update driver-sync/src/test/functional/com/mongodb/client/WithTransac…

365f054

…tionProseTest.java Co-authored-by: Valentin Kovalenko <valentin.male.kovalenko@gmail.com>

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

098b4b4

Co-authored-by: Valentin Kovalenko <valentin.male.kovalenko@gmail.com>

PR feedback

4df0f94

nhachicha and others added 13 commits February 25, 2026 13:00

remove annotation

9f34468

Fix/improve ClientSessionImpl, remove ClientSessionClock

88071e0

PR feedback

e59c4cb

Reduce flakiness in CSOT tests using withTransaction, by forcing an i…

9a925e1

…mmediate retry without backoff.

Using correct jitter value in CSOT tests using withTransaction

4bbef70

Guarding setting a custom jitter for only sync tests

1c97dc7

PR feedback

e32f90d

PR feedback

0b3ee81

Fixing unit test

fda498c

- PR feedback

e5ae458

- Adding logic for DRIVERS-3391

Update driver-core/src/test/unit/com/mongodb/internal/time/Exponentia…

24d4ee5

…lBackoffTest.java Co-authored-by: Valentin Kovalenko <valentin.male.kovalenko@gmail.com>

Update driver-sync/src/test/functional/com/mongodb/client/WithTransac…

4756404

…tionProseTest.java Co-authored-by: Valentin Kovalenko <valentin.male.kovalenko@gmail.com>

moved logic of ExponentialBackoff cleanup to correct class

f22198e

nhachicha self-assigned this Feb 25, 2026

Fixing tests

9dcd5c0

nhachicha marked this pull request as ready for review February 25, 2026 16:57

nhachicha requested a review from a team as a code owner February 25, 2026 16:57

nhachicha requested review from rozza and stIncMale and removed request for rozza February 25, 2026 16:57

stIncMale requested changes Mar 11, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

JAVA-5950 Update Transactions Convenient API with exponential backoff on retries#1899
nhachicha wants to merge 44 commits intomongodb:backpressurefrom
nhachicha:nh/backpressure_convenient_api

nhachicha commented Feb 25, 2026 •

edited by stIncMale

Loading

Uh oh!

stIncMale commented Mar 6, 2026 •

edited

Loading

Uh oh!

stIncMale left a comment

Uh oh!

stIncMale Mar 9, 2026

Uh oh!

stIncMale Mar 9, 2026

Uh oh!

stIncMale Mar 9, 2026

Uh oh!

stIncMale Mar 9, 2026

Uh oh!

stIncMale Mar 9, 2026

Uh oh!

stIncMale Mar 11, 2026

Uh oh!

stIncMale Mar 11, 2026

Uh oh!

stIncMale Mar 11, 2026

Uh oh!

stIncMale Mar 11, 2026

Uh oh!

stIncMale Mar 11, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	public class ExponentialBackoffTest {
	class ExponentialBackoffTest {

		String.format("Attempt %d: backoff should be 0-%d ms, got: %d", attemptNumber,
		expectedBackoff, backoff));

	for (int attemptNumber = 1; attemptNumber < 26; attemptNumber++) {
	for (int attemptNumber = 1; attemptNumber < EXPECTED_BACKOFFS_MAX_VALUES.length * 2; attemptNumber++) {

	public Timeout timeoutOrAlternative(final Timeout alternative) {
	return timeout == null ? alternative : timeout;
	}

Conversation

nhachicha commented Feb 25, 2026 • edited by stIncMale Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stIncMale commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

stIncMale Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nhachicha commented Feb 25, 2026 •

edited by stIncMale

Loading

stIncMale commented Mar 6, 2026 •

edited

Loading

stIncMale Mar 11, 2026 •

edited

Loading