From bd982291416107ee2b504458ebe17696fffa8dd4 Mon Sep 17 00:00:00 2001 From: Nabil Hachicha Date: Wed, 3 Dec 2025 17:39:43 +0000 Subject: [PATCH 01/16] JAVA-5950 - Update Transactions Convenient API with exponential backoff on retries --- .../mongodb/internal/ExponentialBackoff.java | 174 +++++++++++++++ .../internal/ExponentialBackoffTest.java | 205 ++++++++++++++++++ .../client/internal/ClientSessionImpl.java | 38 +++- .../client/WithTransactionProseTest.java | 39 ++++ 4 files changed, 454 insertions(+), 2 deletions(-) create mode 100644 driver-core/src/main/com/mongodb/internal/ExponentialBackoff.java create mode 100644 driver-core/src/test/unit/com/mongodb/internal/ExponentialBackoffTest.java diff --git a/driver-core/src/main/com/mongodb/internal/ExponentialBackoff.java b/driver-core/src/main/com/mongodb/internal/ExponentialBackoff.java new file mode 100644 index 00000000000..518286319ad --- /dev/null +++ b/driver-core/src/main/com/mongodb/internal/ExponentialBackoff.java @@ -0,0 +1,174 @@ +/* + * 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; + +import com.mongodb.annotations.NotThreadSafe; + +import java.util.concurrent.ThreadLocalRandom; + +/** + * Implements exponential backoff with jitter for retry scenarios. + * Formula: delayMS = jitter * min(maxBackoffMs, baseBackoffMs * growthFactor^retryCount) + * where jitter is random value [0, 1). + * + *

This class provides factory methods for common use cases: + *

+ */ +@NotThreadSafe +public final class ExponentialBackoff { + // Transaction retry constants (per spec) + private static final double TRANSACTION_BASE_BACKOFF_MS = 5.0; + private static final double TRANSACTION_MAX_BACKOFF_MS = 500.0; + private static final double TRANSACTION_BACKOFF_GROWTH = 1.5; + + // Command retry constants (per spec) + private static final double COMMAND_BASE_BACKOFF_MS = 100.0; + private static final double COMMAND_MAX_BACKOFF_MS = 10000.0; + private static final double COMMAND_BACKOFF_GROWTH = 2.0; + + private final double baseBackoffMs; + private final double maxBackoffMs; + private final double growthFactor; + private int retryCount = 0; + + /** + * Creates an exponential backoff instance with specified parameters. + * + * @param baseBackoffMs Initial backoff in milliseconds + * @param maxBackoffMs Maximum backoff cap in milliseconds + * @param growthFactor Exponential growth factor (e.g., 1.5 or 2.0) + */ + public ExponentialBackoff(final double baseBackoffMs, final double maxBackoffMs, final double growthFactor) { + this.baseBackoffMs = baseBackoffMs; + this.maxBackoffMs = maxBackoffMs; + this.growthFactor = growthFactor; + } + + /** + * Creates a backoff instance configured for withTransaction retries. + * Uses: 5ms base, 500ms max, 1.5 growth factor. + * + * @return ExponentialBackoff configured for transaction retries + */ + public static ExponentialBackoff forTransactionRetry() { + return new ExponentialBackoff( + TRANSACTION_BASE_BACKOFF_MS, + TRANSACTION_MAX_BACKOFF_MS, + TRANSACTION_BACKOFF_GROWTH + ); + } + + /** + * Creates a backoff instance configured for command retries during overload. + * Uses: 100ms base, 10000ms max, 2.0 growth factor. + * + * @return ExponentialBackoff configured for command retries + */ + public static ExponentialBackoff forCommandRetry() { + return new ExponentialBackoff( + COMMAND_BASE_BACKOFF_MS, + COMMAND_MAX_BACKOFF_MS, + COMMAND_BACKOFF_GROWTH + ); + } + + /** + * Calculate next backoff delay with jitter. + * + * @return delay in milliseconds + */ + public long calculateDelayMs() { + double jitter = ThreadLocalRandom.current().nextDouble(); + double exponentialBackoff = baseBackoffMs * Math.pow(growthFactor, retryCount); + double cappedBackoff = Math.min(exponentialBackoff, maxBackoffMs); + retryCount++; + return Math.round(jitter * cappedBackoff); + } + + /** + * Apply backoff delay by sleeping current thread. + * + * @throws InterruptedException if thread is interrupted during sleep + */ + public void applyBackoff() throws InterruptedException { + long delayMs = calculateDelayMs(); + if (delayMs > 0) { + Thread.sleep(delayMs); + } + } + + /** + * Check if applying backoff would exceed the retry time limit. + * @param startTimeMs start time of retry attempts + * @param maxRetryTimeMs maximum retry time allowed + * @return true if backoff would exceed limit, false otherwise + */ +// public boolean wouldExceedTimeLimit(final long startTimeMs, final long maxRetryTimeMs) { +// long elapsedMs = ClientSessionClock.INSTANCE.now() - startTimeMs; +// // Peek at next delay without incrementing counter +// double exponentialBackoff = baseBackoffMs * Math.pow(growthFactor, retryCount); +// double cappedBackoff = Math.min(exponentialBackoff, maxBackoffMs); +// long maxPossibleDelay = Math.round(cappedBackoff); // worst case with jitter=1 +// return elapsedMs + maxPossibleDelay > maxRetryTimeMs; +// } + + /** + * Reset retry counter for new sequence of retries. + */ + public void reset() { + retryCount = 0; + } + + /** + * Get current retry count for testing. + * + * @return current retry count + */ + public int getRetryCount() { + return retryCount; + } + + /** + * Get the base backoff in milliseconds. + * + * @return base backoff + */ + public double getBaseBackoffMs() { + return baseBackoffMs; + } + + /** + * Get the maximum backoff in milliseconds. + * + * @return maximum backoff + */ + public double getMaxBackoffMs() { + return maxBackoffMs; + } + + /** + * Get the growth factor. + * + * @return growth factor + */ + public double getGrowthFactor() { + return growthFactor; + } +} diff --git a/driver-core/src/test/unit/com/mongodb/internal/ExponentialBackoffTest.java b/driver-core/src/test/unit/com/mongodb/internal/ExponentialBackoffTest.java new file mode 100644 index 00000000000..bfee96e67fb --- /dev/null +++ b/driver-core/src/test/unit/com/mongodb/internal/ExponentialBackoffTest.java @@ -0,0 +1,205 @@ +/* + * 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; + +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 { + + @Test + void testTransactionRetryBackoff() { + ExponentialBackoff backoff = ExponentialBackoff.forTransactionRetry(); + + // Verify configuration + assertEquals(5.0, backoff.getBaseBackoffMs()); + assertEquals(500.0, backoff.getMaxBackoffMs()); + assertEquals(1.5, backoff.getGrowthFactor()); + + // First retry (i=0): delay = jitter * min(5 * 1.5^0, 500) = jitter * 5 + // Since jitter is random [0,1), the delay should be between 0 and 5ms + long delay1 = backoff.calculateDelayMs(); + assertTrue(delay1 >= 0 && delay1 <= 5, "First delay should be 0-5ms, got: " + delay1); + + // Second retry (i=1): delay = jitter * min(5 * 1.5^1, 500) = jitter * 7.5 + long delay2 = backoff.calculateDelayMs(); + assertTrue(delay2 >= 0 && delay2 <= 8, "Second delay should be 0-8ms, got: " + delay2); + + // Third retry (i=2): delay = jitter * min(5 * 1.5^2, 500) = jitter * 11.25 + long delay3 = backoff.calculateDelayMs(); + assertTrue(delay3 >= 0 && delay3 <= 12, "Third delay should be 0-12ms, got: " + delay3); + + // Verify the retry count is incrementing properly + assertEquals(3, backoff.getRetryCount()); + } + + @Test + void testTransactionRetryBackoffRespectsMaximum() { + ExponentialBackoff backoff = ExponentialBackoff.forTransactionRetry(); + + // Advance to a high retry count where backoff would exceed 500ms without capping + for (int i = 0; i < 20; i++) { + backoff.calculateDelayMs(); + } + + // Even at high retry counts, delay should never exceed 500ms + for (int i = 0; i < 5; i++) { + long delay = backoff.calculateDelayMs(); + assertTrue(delay >= 0 && delay <= 500, "Delay should be capped at 500ms, got: " + delay); + } + } + + @Test + void testTransactionRetryBackoffSequenceWithExpectedValues() { + // Test that the backoff sequence follows the expected pattern with growth factor 1.5 + // Expected sequence (without jitter): 5, 7.5, 11.25, 16.875, 25.3125, 37.96875, 56.953125, ... + // With jitter, actual values will be between 0 and these maxima + + ExponentialBackoff backoff = ExponentialBackoff.forTransactionRetry(); + + double[] expectedMaxValues = {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}; + + for (int i = 0; i < expectedMaxValues.length; i++) { + long delay = backoff.calculateDelayMs(); + assertTrue(delay >= 0 && delay <= Math.round(expectedMaxValues[i]), + String.format("Retry %d: delay should be 0-%d ms, got: %d", i, Math.round(expectedMaxValues[i]), delay)); + } + } + + @Test + void testCommandRetryBackoff() { + ExponentialBackoff backoff = ExponentialBackoff.forCommandRetry(); + + // Verify configuration + assertEquals(100.0, backoff.getBaseBackoffMs()); + assertEquals(10000.0, backoff.getMaxBackoffMs()); + assertEquals(2.0, backoff.getGrowthFactor()); + + // Test sequence with growth factor 2.0 + // Expected max delays: 100, 200, 400, 800, 1600, 3200, 6400, 10000 (capped) + long delay1 = backoff.calculateDelayMs(); + assertTrue(delay1 >= 0 && delay1 <= 100, "First delay should be 0-100ms, got: " + delay1); + + long delay2 = backoff.calculateDelayMs(); + assertTrue(delay2 >= 0 && delay2 <= 200, "Second delay should be 0-200ms, got: " + delay2); + + long delay3 = backoff.calculateDelayMs(); + assertTrue(delay3 >= 0 && delay3 <= 400, "Third delay should be 0-400ms, got: " + delay3); + + long delay4 = backoff.calculateDelayMs(); + assertTrue(delay4 >= 0 && delay4 <= 800, "Fourth delay should be 0-800ms, got: " + delay4); + + long delay5 = backoff.calculateDelayMs(); + assertTrue(delay5 >= 0 && delay5 <= 1600, "Fifth delay should be 0-1600ms, got: " + delay5); + } + + @Test + void testCommandRetryBackoffRespectsMaximum() { + ExponentialBackoff backoff = ExponentialBackoff.forCommandRetry(); + + // Advance to where exponential would exceed 10000ms + for (int i = 0; i < 10; i++) { + backoff.calculateDelayMs(); + } + + // Even at high retry counts, delay should never exceed 10000ms + for (int i = 0; i < 5; i++) { + long delay = backoff.calculateDelayMs(); + assertTrue(delay >= 0 && delay <= 10000, "Delay should be capped at 10000ms, got: " + delay); + } + } + + @Test + void testCustomBackoff() { + // Test with custom parameters + ExponentialBackoff backoff = new ExponentialBackoff(50.0, 2000.0, 1.8); + + assertEquals(50.0, backoff.getBaseBackoffMs()); + assertEquals(2000.0, backoff.getMaxBackoffMs()); + assertEquals(1.8, backoff.getGrowthFactor()); + + // First delay: 0-50ms + long delay1 = backoff.calculateDelayMs(); + assertTrue(delay1 >= 0 && delay1 <= 50, "First delay should be 0-50ms, got: " + delay1); + + // Second delay: 0-90ms (50 * 1.8) + long delay2 = backoff.calculateDelayMs(); + assertTrue(delay2 >= 0 && delay2 <= 90, "Second delay should be 0-90ms, got: " + delay2); + } + + @Test + void testReset() { + ExponentialBackoff backoff = ExponentialBackoff.forTransactionRetry(); + + // Perform some retries + backoff.calculateDelayMs(); + backoff.calculateDelayMs(); + assertEquals(2, backoff.getRetryCount()); + + // Reset and verify counter is back to 0 + backoff.reset(); + assertEquals(0, backoff.getRetryCount()); + + // First delay after reset should be in the initial range again + long delay = backoff.calculateDelayMs(); + assertTrue(delay >= 0 && delay <= 5, "First delay after reset should be 0-5ms, got: " + delay); + } + +// @Test +// void testWouldExceedTimeLimitTransactionRetry() { +// ExponentialBackoff backoff = ExponentialBackoff.forTransactionRetry(); +// long startTime = ClientSessionClock.INSTANCE.now(); +// +// // Initially, should not exceed time limit +// assertFalse(backoff.wouldExceedTimeLimit(startTime, 120000)); +// +// // With very little time remaining (4ms), first backoff (up to 5ms) would exceed +// long nearLimitTime = startTime - 119996; // 4ms remaining +// assertTrue(backoff.wouldExceedTimeLimit(nearLimitTime, 120000)); +// } + +// @Test +// void testWouldExceedTimeLimitCommandRetry() { +// ExponentialBackoff backoff = ExponentialBackoff.forCommandRetry(); +// long startTime = ClientSessionClock.INSTANCE.now(); +// +// // Initially, should not exceed time limit +// assertFalse(backoff.wouldExceedTimeLimit(startTime, 10000)); +// +// // With 99ms remaining, first backoff (up to 100ms) would exceed +// long nearLimitTime = startTime - 9901; // 99ms remaining +// assertTrue(backoff.wouldExceedTimeLimit(nearLimitTime, 10000)); +// } + + @Test + void testCommandRetrySequenceMatchesSpec() { + // Test that command retry follows spec: 100ms * 2^i capped at 10000ms + ExponentialBackoff backoff = ExponentialBackoff.forCommandRetry(); + + double[] expectedMaxValues = {100.0, 200.0, 400.0, 800.0, 1600.0, 3200.0, 6400.0, 10000.0, 10000.0}; + + for (int i = 0; i < expectedMaxValues.length; i++) { + long delay = backoff.calculateDelayMs(); + double expectedMax = expectedMaxValues[i]; + assertTrue(delay >= 0 && delay <= Math.round(expectedMax), + String.format("Retry %d: delay should be 0-%d ms, got: %d", i, Math.round(expectedMax), delay)); + } + } +} diff --git a/driver-sync/src/main/com/mongodb/client/internal/ClientSessionImpl.java b/driver-sync/src/main/com/mongodb/client/internal/ClientSessionImpl.java index aa1414dce5d..63e1d68165b 100644 --- a/driver-sync/src/main/com/mongodb/client/internal/ClientSessionImpl.java +++ b/driver-sync/src/main/com/mongodb/client/internal/ClientSessionImpl.java @@ -22,12 +22,14 @@ import com.mongodb.MongoExecutionTimeoutException; import com.mongodb.MongoInternalException; import com.mongodb.MongoOperationTimeoutException; +import com.mongodb.MongoTimeoutException; import com.mongodb.ReadConcern; import com.mongodb.TransactionOptions; import com.mongodb.WriteConcern; import com.mongodb.client.ClientSession; import com.mongodb.client.TransactionBody; import com.mongodb.internal.TimeoutContext; +import com.mongodb.internal.ExponentialBackoff; import com.mongodb.internal.operation.AbortTransactionOperation; import com.mongodb.internal.operation.CommitTransactionOperation; import com.mongodb.internal.operation.OperationHelper; @@ -251,10 +253,35 @@ public T withTransaction(final TransactionBody transactionBody, final Tra notNull("transactionBody", transactionBody); long startTime = ClientSessionClock.INSTANCE.now(); TimeoutContext withTransactionTimeoutContext = createTimeoutContext(options); + // Use CSOT timeout if set, otherwise default to MAX_RETRY_TIME_LIMIT_MS + Long timeoutMS = withTransactionTimeoutContext.getTimeoutSettings().getTimeoutMS(); + long maxRetryTimeMS = timeoutMS != null ? timeoutMS : MAX_RETRY_TIME_LIMIT_MS; + ExponentialBackoff transactionBackoff = null; + boolean isRetry = false; try { outer: while (true) { + // Apply exponential backoff before retrying transaction + if (isRetry) { + // Check if we've exceeded the retry time limit BEFORE applying backoff + if (ClientSessionClock.INSTANCE.now() - startTime >= maxRetryTimeMS) { + throw withTransactionTimeoutContext.hasTimeoutMS() + ? new MongoOperationTimeoutException("Transaction retry exceeded the timeout limit") + : new MongoTimeoutException("Transaction retry time limit exceeded"); + + } + if (transactionBackoff == null) { + transactionBackoff = ExponentialBackoff.forTransactionRetry(); + } + try { + transactionBackoff.applyBackoff(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new MongoClientException("Transaction retry interrupted", e); + } + } + isRetry = true; T retVal; try { startTransaction(options, withTransactionTimeoutContext.copyTimeoutContext()); @@ -269,7 +296,7 @@ public T withTransaction(final TransactionBody transactionBody, final Tra if (e instanceof MongoException && !(e instanceof MongoOperationTimeoutException)) { MongoException exceptionToHandle = OperationHelper.unwrap((MongoException) e); if (exceptionToHandle.hasErrorLabel(TRANSIENT_TRANSACTION_ERROR_LABEL) - && ClientSessionClock.INSTANCE.now() - startTime < MAX_RETRY_TIME_LIMIT_MS) { + && ClientSessionClock.INSTANCE.now() - startTime < maxRetryTimeMS) { if (transactionSpan != null) { transactionSpan.spanFinalizing(false); } @@ -280,13 +307,20 @@ public T withTransaction(final TransactionBody transactionBody, final Tra } if (transactionState == TransactionState.IN) { while (true) { + // Check if we've exceeded the retry time limit + if (ClientSessionClock.INSTANCE.now() - startTime >= maxRetryTimeMS) { + throw hasTimeoutMS(withTransactionTimeoutContext) + ? new MongoOperationTimeoutException("Transaction commit retry time limit exceeded") + : new MongoTimeoutException("Transaction commit retry time limit exceeded"); + } + try { commitTransaction(false); break; } catch (MongoException e) { clearTransactionContextOnError(e); if (!(e instanceof MongoOperationTimeoutException) - && ClientSessionClock.INSTANCE.now() - startTime < MAX_RETRY_TIME_LIMIT_MS) { + && ClientSessionClock.INSTANCE.now() - startTime < maxRetryTimeMS) { applyMajorityWriteConcernToTransactionOptions(); if (!(e instanceof MongoExecutionTimeoutException) diff --git a/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java b/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java index 1afbf61565e..406cee6c9a6 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java +++ b/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java @@ -27,6 +27,7 @@ import org.junit.jupiter.api.Test; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import static com.mongodb.ClusterFixture.TIMEOUT; import static com.mongodb.ClusterFixture.isDiscoverableReplicaSet; @@ -203,6 +204,44 @@ public void testTimeoutMSAndLegacySettings() { } } + // + // Test that exponential backoff is applied when retrying transactions + // Backoff uses growth factor of 1.5 as per spec + // + @Test + public void testExponentialBackoffOnTransientError() { + // Configure failpoint to simulate transient errors + MongoDatabase failPointAdminDb = client.getDatabase("admin"); + failPointAdminDb.runCommand( + Document.parse("{'configureFailPoint': 'failCommand', 'mode': {'times': 3}, " + + "'data': {'failCommands': ['insert'], 'errorCode': 112, " + + "'errorLabels': ['TransientTransactionError']}}")); + + try (ClientSession session = client.startSession()) { + long startTime = System.currentTimeMillis(); + + // Track retry count + AtomicInteger retryCount = new AtomicInteger(0); + + session.withTransaction(() -> { + retryCount.incrementAndGet(); // Count the attempt before the operation that might fail + collection.insertOne(session, Document.parse("{ _id : 'backoff-test' }")); + return retryCount; + }); + + long elapsedTime = System.currentTimeMillis() - startTime; + + // With backoff (growth factor 1.5), we expect at least some delay between retries + // 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"); + assertEquals(4, retryCount.get(), "Expected 1 initial attempt + 3 retries"); + } finally { + failPointAdminDb.runCommand(Document.parse("{'configureFailPoint': 'failCommand', 'mode': 'off'}")); + } + } + private boolean canRunTests() { return isSharded() || isDiscoverableReplicaSet(); } From 72cd714b2851c1401cf20a59db8d25b4e94944e6 Mon Sep 17 00:00:00 2001 From: Nabil Hachicha Date: Tue, 9 Dec 2025 13:27:38 +0000 Subject: [PATCH 02/16] Simplifying test, clean up. --- .../com/mongodb/client/internal/ClientSessionImpl.java | 1 - .../com/mongodb/client/WithTransactionProseTest.java | 7 ------- 2 files changed, 8 deletions(-) diff --git a/driver-sync/src/main/com/mongodb/client/internal/ClientSessionImpl.java b/driver-sync/src/main/com/mongodb/client/internal/ClientSessionImpl.java index 63e1d68165b..855c21e275e 100644 --- a/driver-sync/src/main/com/mongodb/client/internal/ClientSessionImpl.java +++ b/driver-sync/src/main/com/mongodb/client/internal/ClientSessionImpl.java @@ -269,7 +269,6 @@ public T withTransaction(final TransactionBody transactionBody, final Tra throw withTransactionTimeoutContext.hasTimeoutMS() ? new MongoOperationTimeoutException("Transaction retry exceeded the timeout limit") : new MongoTimeoutException("Transaction retry time limit exceeded"); - } if (transactionBackoff == null) { transactionBackoff = ExponentialBackoff.forTransactionRetry(); diff --git a/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java b/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java index 406cee6c9a6..01768a43671 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java +++ b/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java @@ -229,13 +229,6 @@ public void testExponentialBackoffOnTransientError() { return retryCount; }); - long elapsedTime = System.currentTimeMillis() - startTime; - - // With backoff (growth factor 1.5), we expect at least some delay between retries - // 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"); assertEquals(4, retryCount.get(), "Expected 1 initial attempt + 3 retries"); } finally { failPointAdminDb.runCommand(Document.parse("{'configureFailPoint': 'failCommand', 'mode': 'off'}")); From 573c86c5cd60b52cfeee81ef4d35e0e3b5600f0c Mon Sep 17 00:00:00 2001 From: Nabil Hachicha Date: Tue, 9 Dec 2025 14:22:57 +0000 Subject: [PATCH 03/16] Fixing test --- .../com/mongodb/client/internal/ClientSessionImpl.java | 7 ------- .../com/mongodb/client/WithTransactionProseTest.java | 2 +- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/driver-sync/src/main/com/mongodb/client/internal/ClientSessionImpl.java b/driver-sync/src/main/com/mongodb/client/internal/ClientSessionImpl.java index 855c21e275e..b38b1f9027d 100644 --- a/driver-sync/src/main/com/mongodb/client/internal/ClientSessionImpl.java +++ b/driver-sync/src/main/com/mongodb/client/internal/ClientSessionImpl.java @@ -306,13 +306,6 @@ public T withTransaction(final TransactionBody transactionBody, final Tra } if (transactionState == TransactionState.IN) { while (true) { - // Check if we've exceeded the retry time limit - if (ClientSessionClock.INSTANCE.now() - startTime >= maxRetryTimeMS) { - throw hasTimeoutMS(withTransactionTimeoutContext) - ? new MongoOperationTimeoutException("Transaction commit retry time limit exceeded") - : new MongoTimeoutException("Transaction commit retry time limit exceeded"); - } - try { commitTransaction(false); break; diff --git a/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java b/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java index 01768a43671..c8a27cce9cc 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java +++ b/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java @@ -147,7 +147,7 @@ public void testRetryTimeoutEnforcedUnknownTransactionCommit() { public void testRetryTimeoutEnforcedTransientTransactionErrorOnCommit() { MongoDatabase failPointAdminDb = client.getDatabase("admin"); failPointAdminDb.runCommand( - Document.parse("{'configureFailPoint': 'failCommand', 'mode': {'times': 2}, " + Document.parse("{'configureFailPoint': 'failCommand', 'mode': {'times': 1}, " + "'data': {'failCommands': ['commitTransaction'], 'errorCode': 251, 'codeName': 'NoSuchTransaction', " + "'errmsg': 'Transaction 0 has been aborted', 'closeConnection': false}}")); From ecf9b4d5edbe6e2297d7c263fa5041500d157e31 Mon Sep 17 00:00:00 2001 From: Nabil Hachicha Date: Tue, 9 Dec 2025 15:32:31 +0000 Subject: [PATCH 04/16] Update driver-sync/src/main/com/mongodb/client/internal/ClientSessionImpl.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../main/com/mongodb/client/internal/ClientSessionImpl.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/driver-sync/src/main/com/mongodb/client/internal/ClientSessionImpl.java b/driver-sync/src/main/com/mongodb/client/internal/ClientSessionImpl.java index b38b1f9027d..a65d3340886 100644 --- a/driver-sync/src/main/com/mongodb/client/internal/ClientSessionImpl.java +++ b/driver-sync/src/main/com/mongodb/client/internal/ClientSessionImpl.java @@ -267,8 +267,8 @@ public T withTransaction(final TransactionBody transactionBody, final Tra // Check if we've exceeded the retry time limit BEFORE applying backoff if (ClientSessionClock.INSTANCE.now() - startTime >= maxRetryTimeMS) { throw withTransactionTimeoutContext.hasTimeoutMS() - ? new MongoOperationTimeoutException("Transaction retry exceeded the timeout limit") - : new MongoTimeoutException("Transaction retry time limit exceeded"); + ? new MongoOperationTimeoutException("Transaction retry time limit of " + maxRetryTimeMS + "ms exceeded") + : new MongoTimeoutException("Transaction retry time limit of " + maxRetryTimeMS + "ms exceeded"); } if (transactionBackoff == null) { transactionBackoff = ExponentialBackoff.forTransactionRetry(); From 68dbece86a29156a897e4c4074fcbefbf90e4143 Mon Sep 17 00:00:00 2001 From: Nabil Hachicha Date: Tue, 9 Dec 2025 15:43:23 +0000 Subject: [PATCH 05/16] retrigger checks From 7cfc4c4297b1ef5332635ead3ae8b91f9e240cc6 Mon Sep 17 00:00:00 2001 From: Nabil Hachicha Date: Tue, 9 Dec 2025 17:01:23 +0000 Subject: [PATCH 06/16] retrigger checks From cd96a49e9f5f02598932ffc331c35bfe5ac0585b Mon Sep 17 00:00:00 2001 From: Nabil Hachicha Date: Tue, 9 Dec 2025 17:16:33 +0000 Subject: [PATCH 07/16] retrigger checks From 82dc8b186fc5b80e3a0faea4301c293435e7d5e4 Mon Sep 17 00:00:00 2001 From: Nabil Hachicha Date: Tue, 9 Dec 2025 17:20:32 +0000 Subject: [PATCH 08/16] retrigger checks From 558bfadbe8514534108a4b76cd47b4764ba69bd5 Mon Sep 17 00:00:00 2001 From: Nabil Hachicha Date: Tue, 9 Dec 2025 17:25:53 +0000 Subject: [PATCH 09/16] test cleanup --- .../functional/com/mongodb/client/WithTransactionProseTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java b/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java index c8a27cce9cc..78d884ca14c 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java +++ b/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java @@ -124,7 +124,7 @@ public void testRetryTimeoutEnforcedUnknownTransactionCommit() { try (ClientSession session = client.startSession()) { ClientSessionClock.INSTANCE.setTime(START_TIME_MS); - session.withTransaction((TransactionBody) () -> { + session.withTransaction(() -> { ClientSessionClock.INSTANCE.setTime(ERROR_GENERATING_INTERVAL); collection.insertOne(session, new Document("_id", 2)); return null; From fbe881ef5c6cbfbcfac2b6c39ca39a48f3f3a602 Mon Sep 17 00:00:00 2001 From: Nabil Hachicha Date: Tue, 9 Dec 2025 22:53:30 +0000 Subject: [PATCH 10/16] retrigger checks From d89c7140541f907b1250004fa8e25c118c757df9 Mon Sep 17 00:00:00 2001 From: Nabil Hachicha Date: Tue, 9 Dec 2025 23:16:34 +0000 Subject: [PATCH 11/16] Test cleanup --- .../com/mongodb/client/WithTransactionProseTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java b/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java index 78d884ca14c..e06f3c82908 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java +++ b/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java @@ -119,7 +119,7 @@ public void testRetryTimeoutEnforcedTransientTransactionError() { public void testRetryTimeoutEnforcedUnknownTransactionCommit() { MongoDatabase failPointAdminDb = client.getDatabase("admin"); failPointAdminDb.runCommand( - Document.parse("{'configureFailPoint': 'failCommand', 'mode': {'times': 2}, " + Document.parse("{'configureFailPoint': 'failCommand', 'mode': {'times': 1}, " + "'data': {'failCommands': ['commitTransaction'], 'errorCode': 91, 'closeConnection': false}}")); try (ClientSession session = client.startSession()) { @@ -153,7 +153,7 @@ public void testRetryTimeoutEnforcedTransientTransactionErrorOnCommit() { try (ClientSession session = client.startSession()) { ClientSessionClock.INSTANCE.setTime(START_TIME_MS); - session.withTransaction((TransactionBody) () -> { + session.withTransaction(() -> { ClientSessionClock.INSTANCE.setTime(ERROR_GENERATING_INTERVAL); collection.insertOne(session, Document.parse("{ _id : 1 }")); return null; From 2eccff0bd88056e2a0a729e5f799a1324a0a7649 Mon Sep 17 00:00:00 2001 From: Nabil Hachicha Date: Wed, 10 Dec 2025 00:06:08 +0000 Subject: [PATCH 12/16] retrigger checks From c406ae712d125b44d57766d92d2568546a360a06 Mon Sep 17 00:00:00 2001 From: Nabil Hachicha Date: Wed, 10 Dec 2025 14:31:11 +0000 Subject: [PATCH 13/16] Update the implementation according to the spec --- .../client/internal/ClientSessionImpl.java | 21 ++++++++++++------- .../client/WithTransactionProseTest.java | 10 ++++----- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/driver-sync/src/main/com/mongodb/client/internal/ClientSessionImpl.java b/driver-sync/src/main/com/mongodb/client/internal/ClientSessionImpl.java index a65d3340886..59ef120a08b 100644 --- a/driver-sync/src/main/com/mongodb/client/internal/ClientSessionImpl.java +++ b/driver-sync/src/main/com/mongodb/client/internal/ClientSessionImpl.java @@ -22,7 +22,6 @@ import com.mongodb.MongoExecutionTimeoutException; import com.mongodb.MongoInternalException; import com.mongodb.MongoOperationTimeoutException; -import com.mongodb.MongoTimeoutException; import com.mongodb.ReadConcern; import com.mongodb.TransactionOptions; import com.mongodb.WriteConcern; @@ -258,23 +257,27 @@ public T withTransaction(final TransactionBody transactionBody, final Tra long maxRetryTimeMS = timeoutMS != null ? timeoutMS : MAX_RETRY_TIME_LIMIT_MS; ExponentialBackoff transactionBackoff = null; boolean isRetry = false; + MongoException lastError = null; try { outer: while (true) { // Apply exponential backoff before retrying transaction if (isRetry) { - // Check if we've exceeded the retry time limit BEFORE applying backoff - if (ClientSessionClock.INSTANCE.now() - startTime >= maxRetryTimeMS) { - throw withTransactionTimeoutContext.hasTimeoutMS() - ? new MongoOperationTimeoutException("Transaction retry time limit of " + maxRetryTimeMS + "ms exceeded") - : new MongoTimeoutException("Transaction retry time limit of " + maxRetryTimeMS + "ms exceeded"); - } if (transactionBackoff == null) { transactionBackoff = ExponentialBackoff.forTransactionRetry(); } + // Calculate backoff delay and check if it would exceed timeout + long backoffMs = transactionBackoff.calculateDelayMs(); + if (ClientSessionClock.INSTANCE.now() - startTime + backoffMs >= maxRetryTimeMS) { + // Throw the last error as per spec + // lastError is always set here since we only retry on MongoException + throw lastError; + } try { - transactionBackoff.applyBackoff(); + if (backoffMs > 0) { + Thread.sleep(backoffMs); + } } catch (InterruptedException e) { Thread.currentThread().interrupt(); throw new MongoClientException("Transaction retry interrupted", e); @@ -296,6 +299,7 @@ public T withTransaction(final TransactionBody transactionBody, final Tra MongoException exceptionToHandle = OperationHelper.unwrap((MongoException) e); if (exceptionToHandle.hasErrorLabel(TRANSIENT_TRANSACTION_ERROR_LABEL) && ClientSessionClock.INSTANCE.now() - startTime < maxRetryTimeMS) { + lastError = exceptionToHandle; // Track the last error for timeout scenarios if (transactionSpan != null) { transactionSpan.spanFinalizing(false); } @@ -310,6 +314,7 @@ public T withTransaction(final TransactionBody transactionBody, final Tra commitTransaction(false); break; } catch (MongoException e) { + lastError = e; // Track the last error for timeout scenarios clearTransactionContextOnError(e); if (!(e instanceof MongoOperationTimeoutException) && ClientSessionClock.INSTANCE.now() - startTime < maxRetryTimeMS) { diff --git a/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java b/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java index e06f3c82908..45f28e586fe 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java +++ b/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java @@ -119,12 +119,12 @@ public void testRetryTimeoutEnforcedTransientTransactionError() { public void testRetryTimeoutEnforcedUnknownTransactionCommit() { MongoDatabase failPointAdminDb = client.getDatabase("admin"); failPointAdminDb.runCommand( - Document.parse("{'configureFailPoint': 'failCommand', 'mode': {'times': 1}, " + Document.parse("{'configureFailPoint': 'failCommand', 'mode': {'times': 2}, " + "'data': {'failCommands': ['commitTransaction'], 'errorCode': 91, 'closeConnection': false}}")); try (ClientSession session = client.startSession()) { ClientSessionClock.INSTANCE.setTime(START_TIME_MS); - session.withTransaction(() -> { + session.withTransaction((TransactionBody) () -> { ClientSessionClock.INSTANCE.setTime(ERROR_GENERATING_INTERVAL); collection.insertOne(session, new Document("_id", 2)); return null; @@ -147,13 +147,13 @@ public void testRetryTimeoutEnforcedUnknownTransactionCommit() { public void testRetryTimeoutEnforcedTransientTransactionErrorOnCommit() { MongoDatabase failPointAdminDb = client.getDatabase("admin"); failPointAdminDb.runCommand( - Document.parse("{'configureFailPoint': 'failCommand', 'mode': {'times': 1}, " + Document.parse("{'configureFailPoint': 'failCommand', 'mode': {'times': 2}, " + "'data': {'failCommands': ['commitTransaction'], 'errorCode': 251, 'codeName': 'NoSuchTransaction', " + "'errmsg': 'Transaction 0 has been aborted', 'closeConnection': false}}")); try (ClientSession session = client.startSession()) { ClientSessionClock.INSTANCE.setTime(START_TIME_MS); - session.withTransaction(() -> { + session.withTransaction((TransactionBody) () -> { ClientSessionClock.INSTANCE.setTime(ERROR_GENERATING_INTERVAL); collection.insertOne(session, Document.parse("{ _id : 1 }")); return null; @@ -218,8 +218,6 @@ public void testExponentialBackoffOnTransientError() { + "'errorLabels': ['TransientTransactionError']}}")); try (ClientSession session = client.startSession()) { - long startTime = System.currentTimeMillis(); - // Track retry count AtomicInteger retryCount = new AtomicInteger(0); From a28ce984d5c72d75d6a399483e9054622d33abc4 Mon Sep 17 00:00:00 2001 From: Nabil Hachicha Date: Wed, 10 Dec 2025 16:28:47 +0000 Subject: [PATCH 14/16] Added prose test --- .../mongodb/internal/ExponentialBackoff.java | 41 ++--- .../internal/ExponentialBackoffTest.java | 152 ++++++++++++++---- .../client/WithTransactionProseTest.java | 55 +++++++ 3 files changed, 196 insertions(+), 52 deletions(-) diff --git a/driver-core/src/main/com/mongodb/internal/ExponentialBackoff.java b/driver-core/src/main/com/mongodb/internal/ExponentialBackoff.java index 518286319ad..0a01d38e271 100644 --- a/driver-core/src/main/com/mongodb/internal/ExponentialBackoff.java +++ b/driver-core/src/main/com/mongodb/internal/ExponentialBackoff.java @@ -19,6 +19,9 @@ import com.mongodb.annotations.NotThreadSafe; import java.util.concurrent.ThreadLocalRandom; +import java.util.function.DoubleSupplier; + +import static com.mongodb.internal.VisibleForTesting.AccessModifier.PRIVATE; /** * Implements exponential backoff with jitter for retry scenarios. @@ -48,6 +51,9 @@ public final class ExponentialBackoff { private final double growthFactor; private int retryCount = 0; + // Test-only jitter supplier - when set, overrides ThreadLocalRandom + private static volatile DoubleSupplier testJitterSupplier = null; + /** * Creates an exponential backoff instance with specified parameters. * @@ -95,7 +101,10 @@ public static ExponentialBackoff forCommandRetry() { * @return delay in milliseconds */ public long calculateDelayMs() { - double jitter = ThreadLocalRandom.current().nextDouble(); + // Use test jitter supplier if set, otherwise use ThreadLocalRandom + double jitter = testJitterSupplier != null + ? testJitterSupplier.getAsDouble() + : ThreadLocalRandom.current().nextDouble(); double exponentialBackoff = baseBackoffMs * Math.pow(growthFactor, retryCount); double cappedBackoff = Math.min(exponentialBackoff, maxBackoffMs); retryCount++; @@ -103,31 +112,23 @@ public long calculateDelayMs() { } /** - * Apply backoff delay by sleeping current thread. + * Set a custom jitter supplier for testing purposes. + * This overrides the default ThreadLocalRandom jitter generation. * - * @throws InterruptedException if thread is interrupted during sleep + * @param supplier A DoubleSupplier that returns values in [0, 1) range, or null to use default */ - public void applyBackoff() throws InterruptedException { - long delayMs = calculateDelayMs(); - if (delayMs > 0) { - Thread.sleep(delayMs); - } + @VisibleForTesting(otherwise = PRIVATE) + public static void setTestJitterSupplier(final DoubleSupplier supplier) { + testJitterSupplier = supplier; } /** - * Check if applying backoff would exceed the retry time limit. - * @param startTimeMs start time of retry attempts - * @param maxRetryTimeMs maximum retry time allowed - * @return true if backoff would exceed limit, false otherwise + * Clear the test jitter supplier, reverting to default ThreadLocalRandom behavior. */ -// public boolean wouldExceedTimeLimit(final long startTimeMs, final long maxRetryTimeMs) { -// long elapsedMs = ClientSessionClock.INSTANCE.now() - startTimeMs; -// // Peek at next delay without incrementing counter -// double exponentialBackoff = baseBackoffMs * Math.pow(growthFactor, retryCount); -// double cappedBackoff = Math.min(exponentialBackoff, maxBackoffMs); -// long maxPossibleDelay = Math.round(cappedBackoff); // worst case with jitter=1 -// return elapsedMs + maxPossibleDelay > maxRetryTimeMs; -// } + @VisibleForTesting(otherwise = PRIVATE) + public static void clearTestJitterSupplier() { + testJitterSupplier = null; + } /** * Reset retry counter for new sequence of retries. diff --git a/driver-core/src/test/unit/com/mongodb/internal/ExponentialBackoffTest.java b/driver-core/src/test/unit/com/mongodb/internal/ExponentialBackoffTest.java index bfee96e67fb..84ab56a0e47 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/ExponentialBackoffTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/ExponentialBackoffTest.java @@ -16,6 +16,7 @@ package com.mongodb.internal; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -23,6 +24,12 @@ public class ExponentialBackoffTest { + @AfterEach + void cleanup() { + // Always clear the test jitter supplier after each test to avoid test pollution + ExponentialBackoff.clearTestJitterSupplier(); + } + @Test void testTransactionRetryBackoff() { ExponentialBackoff backoff = ExponentialBackoff.forTransactionRetry(); @@ -73,13 +80,11 @@ void testTransactionRetryBackoffSequenceWithExpectedValues() { ExponentialBackoff backoff = ExponentialBackoff.forTransactionRetry(); - double[] expectedMaxValues = {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}; + double[] expectedMaxValues = {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}; for (int i = 0; i < expectedMaxValues.length; i++) { long delay = backoff.calculateDelayMs(); - assertTrue(delay >= 0 && delay <= Math.round(expectedMaxValues[i]), - String.format("Retry %d: delay should be 0-%d ms, got: %d", i, Math.round(expectedMaxValues[i]), delay)); + assertTrue(delay >= 0 && delay <= Math.round(expectedMaxValues[i]), String.format("Retry %d: delay should be 0-%d ms, got: %d", i, Math.round(expectedMaxValues[i]), delay)); } } @@ -162,32 +167,6 @@ void testReset() { assertTrue(delay >= 0 && delay <= 5, "First delay after reset should be 0-5ms, got: " + delay); } -// @Test -// void testWouldExceedTimeLimitTransactionRetry() { -// ExponentialBackoff backoff = ExponentialBackoff.forTransactionRetry(); -// long startTime = ClientSessionClock.INSTANCE.now(); -// -// // Initially, should not exceed time limit -// assertFalse(backoff.wouldExceedTimeLimit(startTime, 120000)); -// -// // With very little time remaining (4ms), first backoff (up to 5ms) would exceed -// long nearLimitTime = startTime - 119996; // 4ms remaining -// assertTrue(backoff.wouldExceedTimeLimit(nearLimitTime, 120000)); -// } - -// @Test -// void testWouldExceedTimeLimitCommandRetry() { -// ExponentialBackoff backoff = ExponentialBackoff.forCommandRetry(); -// long startTime = ClientSessionClock.INSTANCE.now(); -// -// // Initially, should not exceed time limit -// assertFalse(backoff.wouldExceedTimeLimit(startTime, 10000)); -// -// // With 99ms remaining, first backoff (up to 100ms) would exceed -// long nearLimitTime = startTime - 9901; // 99ms remaining -// assertTrue(backoff.wouldExceedTimeLimit(nearLimitTime, 10000)); -// } - @Test void testCommandRetrySequenceMatchesSpec() { // Test that command retry follows spec: 100ms * 2^i capped at 10000ms @@ -198,8 +177,117 @@ void testCommandRetrySequenceMatchesSpec() { for (int i = 0; i < expectedMaxValues.length; i++) { long delay = backoff.calculateDelayMs(); double expectedMax = expectedMaxValues[i]; - assertTrue(delay >= 0 && delay <= Math.round(expectedMax), - String.format("Retry %d: delay should be 0-%d ms, got: %d", i, Math.round(expectedMax), delay)); + assertTrue(delay >= 0 && delay <= Math.round(expectedMax), String.format("Retry %d: delay should be 0-%d ms, got: %d", i, Math.round(expectedMax), delay)); + } + } + + // Tests for the test jitter supplier functionality + + @Test + void testJitterSupplierWithZeroJitter() { + // Set jitter to always return 0 (no backoff) + ExponentialBackoff.setTestJitterSupplier(() -> 0.0); + + ExponentialBackoff backoff = ExponentialBackoff.forTransactionRetry(); + + // With jitter = 0, all delays should be 0 + for (int i = 0; i < 10; i++) { + long delay = backoff.calculateDelayMs(); + assertEquals(0, delay, "With jitter=0, delay should always be 0ms"); + } + } + + @Test + void testJitterSupplierWithFullJitter() { + // Set jitter to always return 1.0 (full backoff) + ExponentialBackoff.setTestJitterSupplier(() -> 1.0); + + ExponentialBackoff backoff = ExponentialBackoff.forTransactionRetry(); + + // Expected delays with jitter=1.0 and growth factor 1.5 + double[] expectedDelays = {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}; + + for (int i = 0; i < expectedDelays.length; i++) { + long delay = backoff.calculateDelayMs(); + long expected = Math.round(expectedDelays[i]); + assertEquals(expected, delay, String.format("Retry %d: with jitter=1.0, delay should be %dms", i, expected)); + } + } + + @Test + void testJitterSupplierWithHalfJitter() { + // Set jitter to always return 0.5 (half backoff) + ExponentialBackoff.setTestJitterSupplier(() -> 0.5); + + ExponentialBackoff backoff = ExponentialBackoff.forTransactionRetry(); + + // Expected delays with jitter=0.5 and growth factor 1.5 + double[] expectedMaxDelays = {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}; + + for (int i = 0; i < expectedMaxDelays.length; i++) { + long delay = backoff.calculateDelayMs(); + long expected = Math.round(0.5 * expectedMaxDelays[i]); + assertEquals(expected, delay, String.format("Retry %d: with jitter=0.5, delay should be %dms", i, expected)); + } + } + + @Test + void testJitterSupplierForCommandRetry() { + // Test that custom jitter also works with command retry configuration + ExponentialBackoff.setTestJitterSupplier(() -> 1.0); + + ExponentialBackoff backoff = ExponentialBackoff.forCommandRetry(); + + // Expected first few delays with jitter=1.0 and growth factor 2.0 + long[] expectedDelays = {100, 200, 400, 800, 1600, 3200, 6400, 10000}; + + for (int i = 0; i < expectedDelays.length; i++) { + long delay = backoff.calculateDelayMs(); + assertEquals(expectedDelays[i], delay, String.format("Command retry %d: with jitter=1.0, delay should be %dms", i, expectedDelays[i])); + } + } + + @Test + void testClearingJitterSupplierReturnsToRandom() { + // First set a fixed jitter + ExponentialBackoff.setTestJitterSupplier(() -> 0.0); + + ExponentialBackoff backoff1 = ExponentialBackoff.forTransactionRetry(); + long delay1 = backoff1.calculateDelayMs(); + assertEquals(0, delay1, "With jitter=0, delay should be 0ms"); + + // Clear the test jitter supplier + ExponentialBackoff.clearTestJitterSupplier(); + + // Now delays should be random again + ExponentialBackoff backoff2 = ExponentialBackoff.forTransactionRetry(); + + // Run multiple times to verify randomness (statistically very unlikely to get all zeros) + boolean foundNonZero = false; + for (int i = 0; i < 20; i++) { + long delay = backoff2.calculateDelayMs(); + assertTrue(delay >= 0 && delay <= Math.round(5.0 * Math.pow(1.5, i)), "Delay should be within expected range"); + if (delay > 0) { + foundNonZero = true; + } } + assertTrue(foundNonZero, "After clearing test jitter, should get some non-zero delays (random behavior)"); + } + + @Test + void testJitterSupplierWithCustomBackoff() { + // Test that custom jitter works with custom backoff parameters + ExponentialBackoff.setTestJitterSupplier(() -> 0.75); + + ExponentialBackoff backoff = new ExponentialBackoff(100.0, 1000.0, 2.5); + + // First delay: 0.75 * 100 = 75 + assertEquals(75, backoff.calculateDelayMs()); + + // Second delay: 0.75 * 100 * 2.5 = 0.75 * 250 = 188 (rounded) + assertEquals(188, backoff.calculateDelayMs()); + + // Third delay: 0.75 * 100 * 2.5^2 = 0.75 * 625 = 469 (rounded) + assertEquals(469, backoff.calculateDelayMs()); } } diff --git a/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java b/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java index 45f28e586fe..43292321ead 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java +++ b/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java @@ -22,6 +22,7 @@ import com.mongodb.TransactionOptions; import com.mongodb.client.internal.ClientSessionClock; import com.mongodb.client.model.Sorts; +import com.mongodb.internal.ExponentialBackoff; import org.bson.Document; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -233,6 +234,60 @@ public void testExponentialBackoffOnTransientError() { } } + // + // Test that retries within withTransaction do not occur immediately + // This test verifies that exponential backoff is enforced during commit retries + // See: https://github.com/mongodb/specifications/blob/master/source/transactions-convenient-api/tests/README.md#retry-backoff-is-enforced + // + @Test + public void testRetryBackoffIsEnforced() { + MongoDatabase failPointAdminDb = client.getDatabase("admin"); + + // Test 1: Run with jitter = 0 (no backoff) + ExponentialBackoff.setTestJitterSupplier(() -> 0.0); + + failPointAdminDb.runCommand(Document.parse("{'configureFailPoint': 'failCommand', 'mode': {'times': 13}, " + "'data': {'failCommands': ['commitTransaction'], 'errorCode': 251}}")); + + long noBackoffTime; + try (ClientSession session = client.startSession()) { + long startNanos = System.nanoTime(); + session.withTransaction(() -> { + collection.insertOne(session, Document.parse("{ _id : 'backoff-test-no-jitter' }")); + return null; + }); + long endNanos = System.nanoTime(); + noBackoffTime = TimeUnit.NANOSECONDS.toMillis(endNanos - startNanos); + } finally { + // Clear the test jitter supplier to avoid affecting other tests + ExponentialBackoff.clearTestJitterSupplier(); + failPointAdminDb.runCommand(Document.parse("{'configureFailPoint': 'failCommand', 'mode': 'off'}")); + } + + // Test 2: Run with jitter = 1 (full backoff) + ExponentialBackoff.setTestJitterSupplier(() -> 1.0); + + failPointAdminDb.runCommand(Document.parse("{'configureFailPoint': 'failCommand', 'mode': {'times': 13}, " + "'data': {'failCommands': ['commitTransaction'], 'errorCode': 251}}")); + + long withBackoffTime; + try (ClientSession session = client.startSession()) { + long startNanos = System.nanoTime(); + session.withTransaction(() -> { + collection.insertOne(session, Document.parse("{ _id : 'backoff-test-full-jitter' }")); + return null; + }); + long endNanos = System.nanoTime(); + withBackoffTime = TimeUnit.NANOSECONDS.toMillis(endNanos - startNanos); + } finally { + ExponentialBackoff.clearTestJitterSupplier(); + failPointAdminDb.runCommand(Document.parse("{'configureFailPoint': 'failCommand', 'mode': 'off'}")); + } + + long expectedWithBackoffTime = noBackoffTime + 2200; // 2.2 seconds as per spec + long actualDifference = Math.abs(withBackoffTime - expectedWithBackoffTime); + + assertTrue(actualDifference < 1000, String.format("Expected withBackoffTime to be ~%dms (noBackoffTime %dms + 2200ms), " + "but got %dms. Difference: %dms (tolerance: 1000ms per spec)", expectedWithBackoffTime, noBackoffTime, withBackoffTime, actualDifference)); + } + private boolean canRunTests() { return isSharded() || isDiscoverableReplicaSet(); } From 27ddef90473bedb83310a740bf81971ce5782af4 Mon Sep 17 00:00:00 2001 From: Nabil Hachicha Date: Wed, 10 Dec 2025 18:16:26 +0000 Subject: [PATCH 15/16] Flaky test --- .../client/AbstractClientSideOperationsTimeoutProseTest.java | 3 +++ .../com/mongodb/client/WithTransactionProseTest.java | 2 ++ 2 files changed, 5 insertions(+) diff --git a/driver-sync/src/test/functional/com/mongodb/client/AbstractClientSideOperationsTimeoutProseTest.java b/driver-sync/src/test/functional/com/mongodb/client/AbstractClientSideOperationsTimeoutProseTest.java index 9ce58b1654f..094ae51f5d2 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/AbstractClientSideOperationsTimeoutProseTest.java +++ b/driver-sync/src/test/functional/com/mongodb/client/AbstractClientSideOperationsTimeoutProseTest.java @@ -675,6 +675,9 @@ public void test10CustomTestWithTransactionUsesASingleTimeout() { } @DisplayName("10. Convenient Transactions - Custom Test: with transaction uses a single timeout - lock") + // The timing of when the timeout check occurred relative to the retry attempts and backoff delays could vary based on system load and jitter values, sometimes allowing + // the LockTimeout error to surface before the timeout was detected. + @FlakyTest(maxAttempts = 3) @Test public void test10CustomTestWithTransactionUsesASingleTimeoutWithLock() { assumeTrue(serverVersionAtLeast(4, 4)); diff --git a/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java b/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java index 43292321ead..bcd52025ac2 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java +++ b/driver-sync/src/test/functional/com/mongodb/client/WithTransactionProseTest.java @@ -25,6 +25,7 @@ import com.mongodb.internal.ExponentialBackoff; import org.bson.Document; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import java.util.concurrent.TimeUnit; @@ -239,6 +240,7 @@ public void testExponentialBackoffOnTransientError() { // This test verifies that exponential backoff is enforced during commit retries // See: https://github.com/mongodb/specifications/blob/master/source/transactions-convenient-api/tests/README.md#retry-backoff-is-enforced // + @DisplayName("Retry Backoff is Enforced") @Test public void testRetryBackoffIsEnforced() { MongoDatabase failPointAdminDb = client.getDatabase("admin"); From 3bef60a2bba3c1fe4363ef53b570459d087ae614 Mon Sep 17 00:00:00 2001 From: Nabil Hachicha Date: Thu, 11 Dec 2025 11:51:59 +0000 Subject: [PATCH 16/16] Remove extra Test annotation --- .../client/AbstractClientSideOperationsTimeoutProseTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/driver-sync/src/test/functional/com/mongodb/client/AbstractClientSideOperationsTimeoutProseTest.java b/driver-sync/src/test/functional/com/mongodb/client/AbstractClientSideOperationsTimeoutProseTest.java index 094ae51f5d2..b4694e143a0 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/AbstractClientSideOperationsTimeoutProseTest.java +++ b/driver-sync/src/test/functional/com/mongodb/client/AbstractClientSideOperationsTimeoutProseTest.java @@ -678,7 +678,6 @@ public void test10CustomTestWithTransactionUsesASingleTimeout() { // The timing of when the timeout check occurred relative to the retry attempts and backoff delays could vary based on system load and jitter values, sometimes allowing // the LockTimeout error to surface before the timeout was detected. @FlakyTest(maxAttempts = 3) - @Test public void test10CustomTestWithTransactionUsesASingleTimeoutWithLock() { assumeTrue(serverVersionAtLeast(4, 4)); assumeFalse(isStandalone());