Skip to content

Commit a485378

Browse files
authored
Prevent CachedSupplier jitter from cycling through low values (#6811)
* Prevent CachedSupplier.maxStaleFailureJitter() from cycling through low values * Update CachedSupplierTest
1 parent 0b028a6 commit a485378

File tree

4 files changed

+44
-0
lines changed

4 files changed

+44
-0
lines changed

.changes/2.42.18.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,12 @@
3131
"category": "AWS SDK for Java v2",
3232
"contributor": "",
3333
"description": "Updated endpoint and partition metadata."
34+
},
35+
{
36+
"type": "bugfix",
37+
"category": "AWS SDK for Java v2",
38+
"contributor": "",
39+
"description": "Fix bug in CachedSupplier that disabled InstanceProfileCredentialsProvider credential refreshing after 58 consecutive failures."
3440
}
3541
]
3642
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"type": "bugfix",
3+
"category": "AWS SDK for Java v2",
4+
"contributor": "",
5+
"description": "Fix bug in CachedSupplier that retries aggressively after many consecutive failures"
6+
}

utils/src/main/java/software/amazon/awssdk/utils/cache/CachedSupplier.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,13 +334,22 @@ private Duration maxPrefetchJitter(RefreshResult<T> result) {
334334
}
335335

336336
private Duration maxStaleFailureJitter(int numFailures) {
337+
// prevent cycling back through low values
338+
if (numFailures > 63) {
339+
return Duration.ofSeconds(10);
340+
}
337341
long exponentialBackoffMillis = (1L << numFailures - 1) * 100;
338342
if (exponentialBackoffMillis <= 0) {
339343
exponentialBackoffMillis = Long.MAX_VALUE;
340344
}
341345
return ComparableUtils.minimum(Duration.ofMillis(exponentialBackoffMillis), Duration.ofSeconds(10));
342346
}
343347

348+
@SdkTestInternalApi
349+
protected Duration maxStaleFailureJitterTest(int numFailures) {
350+
return maxStaleFailureJitter(numFailures);
351+
}
352+
344353
private Instant jitterTime(Instant time, Duration jitterStart, Duration jitterEnd) {
345354
long jitterRange = jitterEnd.minus(jitterStart).toMillis();
346355
long jitterAmount = Math.abs(jitterRandom.nextLong() % jitterRange);

utils/src/test/java/software/amazon/awssdk/utils/cache/CachedSupplierTest.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
import java.io.Closeable;
2929
import java.time.Clock;
30+
import java.time.Duration;
3031
import java.time.Instant;
3132
import java.time.ZoneId;
3233
import java.time.ZoneOffset;
@@ -362,6 +363,28 @@ public void throwIsHiddenIfValueIsStaleInAllowMode() throws InterruptedException
362363
}
363364
}
364365

366+
@Test
367+
public void maxStaleFailureJitter_shouldNotReturnNegativeOrCycleLowValues() {
368+
CachedSupplier<String> supplier = CachedSupplier.builder(() -> RefreshResult.builder("v")
369+
.staleTime(Instant.MAX)
370+
.build())
371+
.build();
372+
373+
for (int i = 1; i <= 70; i++) {
374+
Duration jitter = supplier.maxStaleFailureJitterTest(i);
375+
assertThat(jitter)
376+
.as("numFailures=%d: jitter must be positive", i)
377+
.isPositive();
378+
379+
if (i > 64) {
380+
assertThat(jitter)
381+
.isEqualTo(Duration.ofSeconds(10));
382+
}
383+
}
384+
385+
supplier.close();
386+
}
387+
365388
@Test
366389
public void basicCachingWorks() {
367390
try (WaitingSupplier waitingSupplier = new WaitingSupplier(future(), future())) {

0 commit comments

Comments
 (0)