Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
7b27940
Version: bump 5.7.0-beta0
strogiyotec Jan 16, 2026
1094102
Version: bump 5.7.0-SNAPSHOT
strogiyotec Jan 16, 2026
024e420
JAVA-5950 - Update Transactions Convenient API with exponential back…
nhachicha Dec 3, 2025
8071de6
Simplifying test, clean up.
nhachicha Dec 9, 2025
0f6e118
Fixing test
nhachicha Dec 9, 2025
19f296f
Update driver-sync/src/main/com/mongodb/client/internal/ClientSession…
nhachicha Dec 9, 2025
455f352
retrigger checks
nhachicha Dec 9, 2025
85ec5ef
retrigger checks
nhachicha Dec 9, 2025
5f2157a
retrigger checks
nhachicha Dec 9, 2025
f94d9c7
retrigger checks
nhachicha Dec 9, 2025
8c322c6
test cleanup
nhachicha Dec 9, 2025
14f339d
retrigger checks
nhachicha Dec 9, 2025
f593950
Test cleanup
nhachicha Dec 9, 2025
d12e1f3
retrigger checks
nhachicha Dec 10, 2025
26da297
Update the implementation according to the spec
nhachicha Dec 10, 2025
a1d5bca
Added prose test
nhachicha Dec 10, 2025
e8857e0
Flaky test
nhachicha Dec 10, 2025
b8d7abf
Remove extra Test annotation
nhachicha Dec 11, 2025
c2a91bf
Throwing correct exception when CSOT is used
nhachicha Dec 14, 2025
8957883
Simplifying implementation by relying on CSOT to throw when timeout i…
nhachicha Dec 14, 2025
e008a43
Fixing implementation according to spec changes in JAVA-6046 and http…
nhachicha Jan 13, 2026
a54d95a
Update driver-sync/src/test/functional/com/mongodb/client/WithTransac…
nhachicha Jan 13, 2026
553d6ba
Update driver-sync/src/test/functional/com/mongodb/client/WithTransac…
nhachicha Jan 14, 2026
c0136e1
Update driver-sync/src/test/functional/com/mongodb/client/WithTransac…
nhachicha Jan 14, 2026
09a1291
Update driver-sync/src/test/functional/com/mongodb/client/WithTransac…
nhachicha Jan 14, 2026
520fead
Update driver-sync/src/test/functional/com/mongodb/client/WithTransac…
nhachicha Jan 14, 2026
478e242
Update driver-sync/src/test/functional/com/mongodb/client/WithTransac…
nhachicha Jan 14, 2026
365f054
Update driver-sync/src/test/functional/com/mongodb/client/WithTransac…
nhachicha Jan 14, 2026
098b4b4
Update driver-core/src/main/com/mongodb/internal/ExponentialBackoff.java
nhachicha Jan 14, 2026
4df0f94
PR feedback
nhachicha Jan 15, 2026
9f34468
remove annotation
nhachicha Jan 16, 2026
88071e0
Fix/improve ClientSessionImpl, remove ClientSessionClock
stIncMale Jan 26, 2026
e59c4cb
PR feedback
nhachicha Jan 29, 2026
9a925e1
Reduce flakiness in CSOT tests using withTransaction, by forcing an i…
nhachicha Jan 29, 2026
4bbef70
Using correct jitter value in CSOT tests using withTransaction
nhachicha Jan 29, 2026
1c97dc7
Guarding setting a custom jitter for only sync tests
nhachicha Jan 29, 2026
e32f90d
PR feedback
nhachicha Feb 2, 2026
0b3ee81
PR feedback
nhachicha Feb 2, 2026
fda498c
Fixing unit test
nhachicha Feb 2, 2026
e5ae458
- PR feedback
nhachicha Feb 12, 2026
24d4ee5
Update driver-core/src/test/unit/com/mongodb/internal/time/Exponentia…
nhachicha Feb 12, 2026
4756404
Update driver-sync/src/test/functional/com/mongodb/client/WithTransac…
nhachicha Feb 12, 2026
f22198e
moved logic of ExponentialBackoff cleanup to correct class
nhachicha Feb 12, 2026
9dcd5c0
Fixing tests
nhachicha Feb 25, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions driver-core/src/main/com/mongodb/internal/TimeoutContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,10 @@ public Timeout timeoutIncludingRoundTrip() {
return timeout == null ? null : timeout.shortenBy(minRoundTripTimeMS, MILLISECONDS);
}

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

Comment on lines +171 to +174
Copy link
Member

@stIncMale stIncMale Mar 11, 2026

Choose a reason for hiding this comment

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

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

/**
* Returns the remaining {@code timeoutMS} if set or the {@code alternativeTimeoutMS}.
*
Expand All @@ -176,6 +180,7 @@ public Timeout timeoutIncludingRoundTrip() {
* @param alternativeTimeoutMS the alternative timeout.
* @return timeout to use.
*/
@VisibleForTesting(otherwise = PRIVATE)
public long timeoutOrAlternative(final long alternativeTimeoutMS) {
if (timeout == null) {
return alternativeTimeoutMS;
Expand Down Expand Up @@ -380,11 +385,6 @@ public TimeoutContext withAdditionalReadTimeout(final int additionalReadTimeout)
return new TimeoutContext(timeoutSettings.withReadTimeoutMS(newReadTimeout > 0 ? newReadTimeout : Long.MAX_VALUE));
}

// Creates a copy of the timeout context that can be reset without resetting the original.
public TimeoutContext copyTimeoutContext() {
return new TimeoutContext(getTimeoutSettings(), getTimeout());
}

@Override
public String toString() {
return "TimeoutContext{"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@
* To enable unit testing of classes that rely on System.nanoTime
*
* <p>This class is not part of the public API and may be removed or changed at any time</p>
*
* @deprecated Use {@link com.mongodb.internal.time.SystemNanoTime} in production code,
* and {@code Mockito.mockStatic} in test code to tamper with it.
*/
@Deprecated
public final class Time {
static final long CONSTANT_TIME = 42;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/*
* Copyright 2008-present MongoDB, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.mongodb.internal.time;

import com.mongodb.internal.VisibleForTesting;

import java.util.concurrent.ThreadLocalRandom;
import java.util.function.DoubleSupplier;

import static com.mongodb.assertions.Assertions.assertTrue;
import static com.mongodb.internal.VisibleForTesting.AccessModifier.PRIVATE;

/**
* Provides exponential backoff calculations with jitter for retry scenarios.
*/
public final class ExponentialBackoff {

// Constants for transaction retry backoff
@VisibleForTesting(otherwise = PRIVATE)
static final double TRANSACTION_BASE_MS = 5.0;
Copy link
Member

Choose a reason for hiding this comment

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

TRANSACTION_BASE_MS and TRANSACTION_GROWTH were made package-access and marked @VisibleForTesting(otherwise = PRIVATE), but they are not used in the test code.

@VisibleForTesting(otherwise = PRIVATE)
static final double TRANSACTION_MAX_MS = 500.0;
@VisibleForTesting(otherwise = PRIVATE)
static final double TRANSACTION_GROWTH = 1.5;

// TODO-JAVA-6079
private static DoubleSupplier testJitterSupplier = null;

private ExponentialBackoff() {
}

/**
* Calculate the backoff in milliseconds for transaction retries.
*
* @param attemptNumber 0-based attempt number
* @return The calculated backoff in milliseconds.
*/
public static long calculateTransactionBackoffMs(final int attemptNumber) {
assertTrue(attemptNumber > 0, "Attempt number must be greater than 0 in the context of transaction backoff calculation");
double jitter = testJitterSupplier != null
? testJitterSupplier.getAsDouble()
: ThreadLocalRandom.current().nextDouble();
return Math.round(jitter * Math.min(
TRANSACTION_BASE_MS * Math.pow(TRANSACTION_GROWTH, attemptNumber - 1),
TRANSACTION_MAX_MS));
}

/**
* Set a custom jitter supplier for testing purposes.
*
* @param supplier A DoubleSupplier that returns values in [0, 1) range.
Copy link
Member

Choose a reason for hiding this comment

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

We should include 1 in the interval: [0, 1], because tests specify 1. Ideally, instead of tests specifying the jitter in this PR, they should have disabled/enabled jitter: it is not conceivable that we will ever need any other behavior here than an switch (and in the highly unlikely situation that we need something else, we can always support that later). The only reason I am not pushing for a switch now is because we hopefully will introduce a switch in https://jira.mongodb.org/browse/JAVA-6079, and remove the temporary methods setTestJitterSupplier/clearTestJitterSupplier.

The same goes about the spec. If the spec was formulated in terms of a switch, neither this Slack discussion nor this https://jira.mongodb.org/browse/DRIVERS-3370 would need to exist.

P.S. Note that the specification not having been formulated in terms of a switch does not prevent us from implementing tests using a switch for jitter, because both approaches are equivalent for the current tests.

*/
@VisibleForTesting(otherwise = PRIVATE)
public static void setTestJitterSupplier(final DoubleSupplier supplier) {
testJitterSupplier = supplier;
}

/**
* Clear the test jitter supplier, reverting to default ThreadLocalRandom behavior.
*/
@VisibleForTesting(otherwise = PRIVATE)
public static void clearTestJitterSupplier() {
testJitterSupplier = null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,6 @@ public interface StartTime {
* @return a StartPoint, as of now
*/
static StartTime now() {
return TimePoint.at(System.nanoTime());
return TimePoint.at(SystemNanoTime.get());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* Copyright 2008-present MongoDB, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.mongodb.internal.time;

/**
* Avoid using this class directly and prefer using other program elements from {@link com.mongodb.internal.time}, if possible.
* <p>
* We do not use {@link System#nanoTime()} directly in the rest of the {@link com.mongodb.internal.time} package,
* and use {@link SystemNanoTime#get()} instead because we need to tamper with it via {@code Mockito.mockStatic},
* and mocking methods of {@link System} class is both impossible and unwise.
*/
public final class SystemNanoTime {
private SystemNanoTime() {
}

public static long get() {
return System.nanoTime();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,14 @@ static TimePoint at(@Nullable final Long nanos) {

@VisibleForTesting(otherwise = PRIVATE)
long currentNanos() {
return System.nanoTime();
return SystemNanoTime.get();
}

/**
* Returns the current {@link TimePoint}.
*/
static TimePoint now() {
return at(System.nanoTime());
return at(SystemNanoTime.get());
}

/**
Expand Down
Copy link
Member

Choose a reason for hiding this comment

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

I left multiple suggestions about removing comments in this file. All of those comments have the following in common - they explain straightforward code and repeat the assertions / assertion messages in the code. Were they generated with an AI tool?

Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/*
* Copyright 2008-present MongoDB, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.mongodb.internal.time;

import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;

public class ExponentialBackoffTest {
Copy link
Member

Choose a reason for hiding this comment

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

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 {

// Expected backoffs with jitter=1.0 and growth factor ExponentialBackoff.TRANSACTION_GROWTH
Copy link
Member

Choose a reason for hiding this comment

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

  • This is a field documentation comment, let's write it as such.
  • There seems to be no reason to mention ExponentialBackoff.TRANSACTION_GROWTH (it is especially surprising to see it given that other internal parameters are not mentioned). It's an internal parameter that even the tests can't change.
Suggested change
// Expected backoffs with jitter=1.0 and growth factor ExponentialBackoff.TRANSACTION_GROWTH
/**
* Expected {@linkplain ExponentialBackoff#calculateTransactionBackoffMs(int) backoffs} with 1.0 as
* {@link ExponentialBackoff#setTestJitterSupplier(DoubleSupplier) jiter}.
*/

private static final double[] EXPECTED_BACKOFFS_MAX_VALUES = {5.0, 7.5, 11.25, 16.875, 25.3125, 37.96875, 56.953125, 85.4296875, 128.14453125,
192.21679688, 288.32519531, 432.48779297, 500.0};

@Test
void testCalculateTransactionBackoffMs() {
// Test that the backoff sequence follows the expected pattern with growth factor ExponentialBackoff.TRANSACTION_GROWTH
// Expected sequence (without jitter): 5, 7.5, 11.25, ...
// With jitter, actual values will be between 0 and these maxima

for (int attemptNumber = 1; attemptNumber <= EXPECTED_BACKOFFS_MAX_VALUES.length; attemptNumber++) {
long backoff = ExponentialBackoff.calculateTransactionBackoffMs(attemptNumber);
long expectedBackoff = Math.round(EXPECTED_BACKOFFS_MAX_VALUES[attemptNumber - 1]);
assertTrue(backoff >= 0 && backoff <= expectedBackoff,
String.format("Attempt %d: backoff should be 0-%d ms, got: %d", attemptNumber,
Comment on lines +30 to +39
Copy link
Member

Choose a reason for hiding this comment

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

The comment at the top of the test method body does not seem to be useful: it explains the short and straightforward code of the test. Comments, like any other code, are not free: they have to be reviewed, maintained, read by code readers. Comments should not be used to explain what a trivial piece of code does. Let's remove the comment.

expectedBackoff, backoff));
Comment on lines +39 to +40
Copy link
Member

Choose a reason for hiding this comment

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

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

}
}

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

Choose a reason for hiding this comment

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

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++) {

long backoff = ExponentialBackoff.calculateTransactionBackoffMs(attemptNumber);
assertTrue(backoff >= 0 && backoff <= ExponentialBackoff.TRANSACTION_MAX_MS,
String.format("Attempt %d: backoff should be capped at %f ms, got: %d ms",
Comment on lines +45 to +50
Copy link
Member

Choose a reason for hiding this comment

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

The "Even at high attempt numbers, backoff should never exceed ExponentialBackoff.TRANSACTION_MAX_MS" comment adds no information, as it repeats both the straightforward assertion in the code, as well as the assertion message. Let's remove the comment.

attemptNumber, ExponentialBackoff.TRANSACTION_MAX_MS, backoff));
}
}

@Test
void testCustomJitter() {
// Test with jitter = 1.0
ExponentialBackoff.setTestJitterSupplier(() -> 1.0);
try {
for (int attemptNumber = 1; attemptNumber <= EXPECTED_BACKOFFS_MAX_VALUES.length; attemptNumber++) {
long backoff = ExponentialBackoff.calculateTransactionBackoffMs(attemptNumber);
long expected = Math.round(EXPECTED_BACKOFFS_MAX_VALUES[attemptNumber - 1]);
assertEquals(expected, backoff,
String.format("Attempt %d: with jitter=1.0, backoff should be %d ms", attemptNumber, expected));
}
} finally {
ExponentialBackoff.clearTestJitterSupplier();
}

// Test with jitter = 0, all backoffs should be 0
ExponentialBackoff.setTestJitterSupplier(() -> 0.0);
try {
for (int attemptNumber = 1; attemptNumber < EXPECTED_BACKOFFS_MAX_VALUES.length; attemptNumber++) {
long backoff = ExponentialBackoff.calculateTransactionBackoffMs(attemptNumber);
assertEquals(0, backoff, "With jitter=0, backoff should always be 0 ms");
}
} finally {
ExponentialBackoff.clearTestJitterSupplier();
}
}
Comment on lines +55 to +80
Copy link
Member

Choose a reason for hiding this comment

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

  • The "Test with..." comments are here seemingly to add structure to the code. Instead of using comments, we should either split this into two tests, or, use assertAll.
  • The "all backoffs should be 0" comment adds no information, as it repeats both the straightforward assertion in the code, as well as the assertion message.

Let's remove the comments and use more proper tools to structure the code.

}

This file was deleted.

Loading