Skip to content

Commit e59c4cb

Browse files
nhachichanhachicha
committed
PR feedback
1 parent 88071e0 commit e59c4cb

4 files changed

Lines changed: 50 additions & 54 deletions

File tree

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

Lines changed: 15 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -24,46 +24,34 @@
2424
import static com.mongodb.internal.VisibleForTesting.AccessModifier.PRIVATE;
2525

2626
/**
27-
* Implements exponential backoff with jitter for retry scenarios.
27+
* Provides exponential backoff calculations with jitter for retry scenarios.
2828
*/
29-
public enum ExponentialBackoff {
30-
TRANSACTION(5.0, 500.0, 1.5);
29+
public final class ExponentialBackoff {
3130

32-
private final double baseMs, maxMs, growth;
31+
// Constants for transaction retry backoff
32+
private static final double TRANSACTION_BASE_MS = 5.0;
33+
private static final double TRANSACTION_MAX_MS = 500.0;
34+
private static final double TRANSACTION_GROWTH = 1.5;
3335

34-
// TODO remove this global state once https://jira.mongodb.org/browse/JAVA-6060 is done
36+
// TODO-JAVA-6079
3537
private static DoubleSupplier testJitterSupplier = null;
3638

37-
ExponentialBackoff(final double baseMs, final double maxMs, final double growth) {
38-
this.baseMs = baseMs;
39-
this.maxMs = maxMs;
40-
this.growth = growth;
39+
private ExponentialBackoff() {
4140
}
4241

4342
/**
44-
* Calculate the next delay in milliseconds based on the retry count.
43+
* Calculate the backoff in milliseconds for transaction retries.
4544
*
46-
* @param retryCount The number of retries that have occurred.
47-
* @return The calculated delay in milliseconds.
45+
* @param attemptNumber The 0-based attempt number
46+
* @return The calculated backoff in milliseconds.
4847
*/
49-
public long calculateDelayBeforeNextRetryMs(final int retryCount) {
48+
public static long calculateTransactionBackoffMs(final int attemptNumber) {
5049
double jitter = testJitterSupplier != null
5150
? testJitterSupplier.getAsDouble()
5251
: ThreadLocalRandom.current().nextDouble();
53-
double backoff = Math.min(baseMs * Math.pow(growth, retryCount), maxMs);
54-
return Math.round(jitter * backoff);
55-
}
56-
57-
/**
58-
* Calculate the next delay in milliseconds based on the retry count and a provided jitter.
59-
*
60-
* @param retryCount The number of retries that have occurred.
61-
* @param jitter A double in the range [0, 1) to apply as jitter.
62-
* @return The calculated delay in milliseconds.
63-
*/
64-
public long calculateDelayBeforeNextRetryMs(final int retryCount, final double jitter) {
65-
double backoff = Math.min(baseMs * Math.pow(growth, retryCount), maxMs);
66-
return Math.round(jitter * backoff);
52+
return Math.round(jitter * Math.min(
53+
TRANSACTION_BASE_MS * Math.pow(TRANSACTION_GROWTH, attemptNumber),
54+
TRANSACTION_MAX_MS));
6755
}
6856

6957
/**

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

Lines changed: 31 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -31,43 +31,50 @@ void testTransactionRetryBackoff() {
3131
// With jitter, actual values will be between 0 and these maxima
3232
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};
3333

34-
ExponentialBackoff backoff = ExponentialBackoff.TRANSACTION;
35-
for (int retry = 0; retry < expectedMaxValues.length; retry++) {
36-
long delay = backoff.calculateDelayBeforeNextRetryMs(retry);
37-
assertTrue(delay >= 0 && delay <= Math.round(expectedMaxValues[retry]), String.format("Retry %d: delay should be 0-%d ms, got: %d", retry, Math.round(expectedMaxValues[retry]), delay));
34+
for (int attemptNumber = 0; attemptNumber < expectedMaxValues.length; attemptNumber++) {
35+
long backoff = ExponentialBackoff.calculateTransactionBackoffMs(attemptNumber);
36+
assertTrue(backoff >= 0 && backoff <= Math.round(expectedMaxValues[attemptNumber]),
37+
String.format("Attempt %d: backoff should be 0-%d ms, got: %d", attemptNumber, Math.round(expectedMaxValues[attemptNumber]), backoff));
3838
}
3939
}
4040

4141
@Test
4242
void testTransactionRetryBackoffRespectsMaximum() {
43-
ExponentialBackoff backoff = ExponentialBackoff.TRANSACTION;
44-
45-
// Even at high retry counts, delay should never exceed 500ms
46-
for (int retry = 0; retry < 25; retry++) {
47-
long delay = backoff.calculateDelayBeforeNextRetryMs(retry);
48-
assertTrue(delay >= 0 && delay <= 500, String.format("Retry %d: delay should be capped at 500 ms, got: %d ms", retry, delay));
43+
// Even at high attempt numbers, backoff should never exceed 500ms
44+
for (int attemptNumber = 0; attemptNumber < 25; attemptNumber++) {
45+
long backoff = ExponentialBackoff.calculateTransactionBackoffMs(attemptNumber);
46+
assertTrue(backoff >= 0 && backoff <= 500,
47+
String.format("Attempt %d: backoff should be capped at 500 ms, got: %d ms", attemptNumber, backoff));
4948
}
5049
}
5150

5251
@Test
5352
void testCustomJitter() {
54-
ExponentialBackoff backoff = ExponentialBackoff.TRANSACTION;
55-
56-
// Expected delays with jitter=1.0 and growth factor 1.5
57-
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};
58-
double jitter = 1.0;
53+
// Expected backoffs with jitter=1.0 and growth factor 1.5
54+
double[] expectedBackoffs = {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};
5955

60-
for (int retry = 0; retry < expectedDelays.length; retry++) {
61-
long delay = backoff.calculateDelayBeforeNextRetryMs(retry, jitter);
62-
long expected = Math.round(expectedDelays[retry]);
63-
assertEquals(expected, delay, String.format("Retry %d: with jitter=1.0, delay should be %d ms", retry, expected));
56+
// Test with jitter = 1.0
57+
ExponentialBackoff.setTestJitterSupplier(() -> 1.0);
58+
try {
59+
for (int attemptNumber = 0; attemptNumber < expectedBackoffs.length; attemptNumber++) {
60+
long backoff = ExponentialBackoff.calculateTransactionBackoffMs(attemptNumber);
61+
long expected = Math.round(expectedBackoffs[attemptNumber]);
62+
assertEquals(expected, backoff,
63+
String.format("Attempt %d: with jitter=1.0, backoff should be %d ms", attemptNumber, expected));
64+
}
65+
} finally {
66+
ExponentialBackoff.clearTestJitterSupplier();
6467
}
6568

66-
// With jitter = 0, all delays should be 0
67-
jitter = 0;
68-
for (int retry = 0; retry < 10; retry++) {
69-
long delay = backoff.calculateDelayBeforeNextRetryMs(retry, jitter);
70-
assertEquals(0, delay, "With jitter=0, delay should always be 0 ms");
69+
// Test with jitter = 0, all backoffs should be 0
70+
ExponentialBackoff.setTestJitterSupplier(() -> 0.0);
71+
try {
72+
for (int attemptNumber = 0; attemptNumber < 10; attemptNumber++) {
73+
long backoff = ExponentialBackoff.calculateTransactionBackoffMs(attemptNumber);
74+
assertEquals(0, backoff, "With jitter=0, backoff should always be 0 ms");
75+
}
76+
} finally {
77+
ExponentialBackoff.clearTestJitterSupplier();
7178
}
7279
}
7380
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ private TimeoutContext createTimeoutContext(final TransactionOptions transaction
382382

383383
private static void backoff(final int transactionAttempt,
384384
final Timeout withTransactionTimeout, final MongoException lastError) {
385-
long backoffMs = ExponentialBackoff.TRANSACTION.calculateDelayBeforeNextRetryMs(transactionAttempt - 1);
385+
long backoffMs = ExponentialBackoff.calculateTransactionBackoffMs(transactionAttempt - 1);
386386
withTransactionTimeout.shortenBy(backoffMs, TimeUnit.MILLISECONDS).onExpired(() -> {
387387
throw lastError;
388388
});

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,8 +288,9 @@ private static boolean canRunTests() {
288288

289289
private static void doWithSystemNanoTimeHandle(final Consumer<SystemNanoTimeHandle> action) {
290290
long startNanos = SystemNanoTime.get();
291-
try (MockedStatic<SystemNanoTime> mockedStaticSystem = Mockito.mockStatic(SystemNanoTime.class)) {
292-
action.accept(change -> mockedStaticSystem.when(SystemNanoTime::get).thenReturn(startNanos + change.toNanos()));
291+
try (MockedStatic<SystemNanoTime> mockedStaticSystemNanoTime = Mockito.mockStatic(SystemNanoTime.class)) {
292+
mockedStaticSystemNanoTime.when(SystemNanoTime::get).thenReturn(startNanos);
293+
action.accept(change -> mockedStaticSystemNanoTime.when(SystemNanoTime::get).thenReturn(startNanos + change.toNanos()));
293294
}
294295
}
295296

0 commit comments

Comments
 (0)