Skip to content

Commit 7432584

Browse files
authored
fix: reset refreshRunning on TerminalException in RefreshAheadStrategy (#2253)
1 parent 7151a94 commit 7432584

File tree

2 files changed

+48
-0
lines changed

2 files changed

+48
-0
lines changed

core/src/main/java/com/google/cloud/sql/core/RefreshAheadStrategy.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,9 @@ private ListenableFuture<ConnectionInfo> handleRefreshResult(
276276
final Throwable cause = e.getCause();
277277
if (cause instanceof TerminalException) {
278278
logger.debug(String.format("[%s] Refresh Operation: Failed! No retry.", name), e);
279+
synchronized (connectionInfoGuard) {
280+
refreshRunning = false;
281+
}
279282
throw (TerminalException) cause;
280283
}
281284

core/src/test/java/com/google/cloud/sql/core/RefreshAheadStrategyTest.java

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import java.time.temporal.ChronoUnit;
2727
import java.util.concurrent.Executors;
2828
import java.util.concurrent.ScheduledThreadPoolExecutor;
29+
import java.util.concurrent.TimeoutException;
2930
import java.util.concurrent.atomic.AtomicInteger;
3031
import org.junit.After;
3132
import org.junit.Before;
@@ -514,6 +515,50 @@ public void testGetConnectionInfo_throwsTerminalException_refreshOperationNotSch
514515
assertThat(refreshCount.get()).isEqualTo(1);
515516
}
516517

518+
@Test
519+
public void testGetConnectionInfo_throwsTerminalException_forceRefreshResumes()
520+
throws InterruptedException, TimeoutException {
521+
ExampleData data = new ExampleData(Instant.now().plus(1, ChronoUnit.HOURS));
522+
AtomicInteger refreshCount = new AtomicInteger();
523+
524+
RefreshAheadStrategy r =
525+
new RefreshAheadStrategy(
526+
"RefresherTest.testGetConnectionInfo_throwsTerminalException_forceRefreshResumes",
527+
executorService,
528+
() -> {
529+
int c = refreshCount.get();
530+
ExampleData refreshResult = data;
531+
if (c == 0) { // refresh 0 should throw an exception
532+
refreshCount.incrementAndGet();
533+
throw new TerminalException("Not authorized");
534+
}
535+
// refresh 2 and on should return data immediately
536+
refreshCount.incrementAndGet();
537+
return Futures.immediateFuture(refreshResult);
538+
},
539+
rateLimiter);
540+
541+
// Raising TerminalException stops the refresher's executor from running the next task.
542+
assertThrows(TerminalException.class, () -> r.getConnectionInfo(TEST_TIMEOUT_MS));
543+
assertThat(refreshCount.get()).isEqualTo(1);
544+
545+
// Manually force a refresh
546+
r.forceRefresh();
547+
548+
// getConnectionInfo again, and assert the refresh operation completed.
549+
new PauseCondition()
550+
.waitForCondition(
551+
() -> {
552+
try {
553+
return r.getConnectionInfo(TEST_TIMEOUT_MS) == data;
554+
} catch (TerminalException e) {
555+
return false;
556+
}
557+
},
558+
1000L);
559+
assertThat(refreshCount.get()).isEqualTo(2);
560+
}
561+
517562
@Test
518563
public void testGetConnectionInfo_throwsRuntimeException_refreshOperationScheduled()
519564
throws Exception {

0 commit comments

Comments
 (0)