Skip to content

Commit d976ed4

Browse files
mderkamderka
committed
RetryHelper can be configured to not retry on socket timeouts.
This fixes #816 and #808.
1 parent b29df66 commit d976ed4

4 files changed

Lines changed: 56 additions & 17 deletions

File tree

‎gcloud-java-core/src/main/java/com/google/gcloud/RetryHelper.java‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import com.google.common.base.Stopwatch;
3030

3131
import java.io.InterruptedIOException;
32+
import java.net.SocketTimeoutException;
3233
import java.nio.channels.ClosedByInterruptException;
3334
import java.util.concurrent.Callable;
3435
import java.util.logging.Level;
@@ -44,14 +45,14 @@
4445
public class RetryHelper<V> {
4546

4647
private static final Logger log = Logger.getLogger(RetryHelper.class.getName());
48+
private final static boolean DEFAULT_RETRY_ON_TIMEOUTS = true;
4749

4850
private final Stopwatch stopwatch;
4951
private final Callable<V> callable;
5052
private final RetryParams params;
5153
private final ExceptionHandler exceptionHandler;
5254
private int attemptNumber;
5355

54-
5556
private static final ThreadLocal<Context> context = new ThreadLocal<>();
5657

5758
public static class RetryHelperException extends RuntimeException {
@@ -172,7 +173,7 @@ public String toString() {
172173
return toStringHelper.toString();
173174
}
174175

175-
private V doRetry() throws RetryHelperException {
176+
private V doRetry(boolean retryOnTimeout) throws RetryHelperException {
176177
stopwatch.start();
177178
while (true) {
178179
attemptNumber++;
@@ -189,7 +190,8 @@ private V doRetry() throws RetryHelperException {
189190
}
190191
exception = e;
191192
} catch (Exception e) {
192-
if (!exceptionHandler.shouldRetry(e)) {
193+
if (!exceptionHandler.shouldRetry(e)
194+
|| (!retryOnTimeout && e.getCause() instanceof SocketTimeoutException)) {
193195
throw new NonRetriableException(e);
194196
}
195197
exception = e;
@@ -229,22 +231,30 @@ private static long getExponentialValue(long initialDelay, double backoffFactor,
229231

230232
public static <V> V runWithRetries(Callable<V> callable) throws RetryHelperException {
231233
return runWithRetries(callable, RetryParams.defaultInstance(),
232-
ExceptionHandler.defaultInstance());
234+
ExceptionHandler.defaultInstance(), DEFAULT_RETRY_ON_TIMEOUTS);
233235
}
234236

235237
public static <V> V runWithRetries(Callable<V> callable, RetryParams params,
236238
ExceptionHandler exceptionHandler) throws RetryHelperException {
237-
return runWithRetries(callable, params, exceptionHandler, Stopwatch.createUnstarted());
239+
return runWithRetries(callable, params, exceptionHandler, Stopwatch.createUnstarted(),
240+
DEFAULT_RETRY_ON_TIMEOUTS);
241+
}
242+
243+
public static <V> V runWithRetries(Callable<V> callable, RetryParams params,
244+
ExceptionHandler exceptionHandler, boolean retryOnTimeout) throws RetryHelperException {
245+
return runWithRetries(callable, params, exceptionHandler, Stopwatch.createUnstarted(),
246+
retryOnTimeout);
238247
}
239248

240249
@VisibleForTesting
241250
static <V> V runWithRetries(Callable<V> callable, RetryParams params,
242-
ExceptionHandler exceptionHandler, Stopwatch stopwatch) throws RetryHelperException {
251+
ExceptionHandler exceptionHandler, Stopwatch stopwatch, boolean retryOnTimeout)
252+
throws RetryHelperException {
243253
RetryHelper<V> retryHelper = new RetryHelper<>(callable, params, exceptionHandler, stopwatch);
244254
Context previousContext = getContext();
245255
setContext(new Context(retryHelper));
246256
try {
247-
return retryHelper.doRetry();
257+
return retryHelper.doRetry(retryOnTimeout);
248258
} finally {
249259
setContext(previousContext);
250260
}

‎gcloud-java-core/src/test/java/com/google/gcloud/RetryHelperTest.java‎

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package com.google.gcloud;
1818

19+
import static com.google.gcloud.RetryHelper.runWithRetries;
1920
import static java.util.concurrent.Executors.callable;
2021
import static org.junit.Assert.assertEquals;
2122
import static org.junit.Assert.assertNull;
@@ -30,6 +31,7 @@
3031
import org.junit.Test;
3132

3233
import java.io.IOException;
34+
import java.net.SocketTimeoutException;
3335
import java.util.Arrays;
3436
import java.util.Iterator;
3537
import java.util.concurrent.Callable;
@@ -67,7 +69,7 @@ public void testTriesWithExceptionHandling() {
6769
.retryOn(IOException.class).abortOn(RuntimeException.class).build();
6870
final AtomicInteger count = new AtomicInteger(3);
6971
try {
70-
RetryHelper.runWithRetries(new Callable<Void>() {
72+
runWithRetries(new Callable<Void>() {
7173
@Override public Void call() throws IOException, NullPointerException {
7274
if (count.decrementAndGet() == 2) {
7375
assertEquals(1, RetryHelper.getContext().getAttemptNumber());
@@ -91,7 +93,7 @@ public void testTriesWithExceptionHandling() {
9193
final Iterator<? extends E1Exception> exceptions = Arrays.asList(
9294
new E1Exception(), new E2Exception(), new E4Exception(), new E3Exception()).iterator();
9395
try {
94-
RetryHelper.runWithRetries(new Callable<Void>() {
96+
runWithRetries(new Callable<Void>() {
9597
@Override public Void call() throws E1Exception {
9698
throw exceptions.next();
9799
}
@@ -113,7 +115,7 @@ public void testTriesAtLeastMinTimes() {
113115
.build();
114116
final int timesToFail = 7;
115117
assertNull(RetryHelper.getContext());
116-
int attempted = RetryHelper.runWithRetries(new Callable<Integer>() {
118+
int attempted = runWithRetries(new Callable<Integer>() {
117119
int timesCalled;
118120
@Override public Integer call() throws IOException {
119121
timesCalled++;
@@ -140,7 +142,7 @@ public void testTriesNoMoreThanMaxTimes() {
140142
.build();
141143
final AtomicInteger timesCalled = new AtomicInteger(0);
142144
try {
143-
RetryHelper.runWithRetries(callable(new Runnable() {
145+
runWithRetries(callable(new Runnable() {
144146
@Override public void run() {
145147
// Throw an exception up to maxAttempts times, should never be called beyond that
146148
if (timesCalled.incrementAndGet() <= maxAttempts) {
@@ -186,22 +188,47 @@ public void testTriesNoMoreLongerThanTotalRetryPeriod() {
186188
final int sleepOnAttempt = 8;
187189
final AtomicInteger timesCalled = new AtomicInteger(0);
188190
try {
189-
RetryHelper.runWithRetries(callable(new Runnable() {
191+
runWithRetries(callable(new Runnable() {
190192
@Override public void run() {
191193
timesCalled.incrementAndGet();
192194
if (timesCalled.get() == sleepOnAttempt) {
193195
ticker.advance(1000, TimeUnit.MILLISECONDS);
194196
}
195197
throw new RuntimeException();
196198
}
197-
}), params, handler, stopwatch);
199+
}), params, handler, stopwatch, true);
198200
fail();
199201
} catch (RetriesExhaustedException expected) {
200202
// verify timesCalled
201203
assertEquals(sleepOnAttempt, timesCalled.get());
202204
}
203205
}
204206

207+
@Test
208+
public void testNoRetriesOnSocketTimeout() {
209+
final FakeTicker ticker = new FakeTicker();
210+
Stopwatch stopwatch = Stopwatch.createUnstarted(ticker);
211+
RetryParams params = RetryParams.builder().initialRetryDelayMillis(0)
212+
.totalRetryPeriodMillis(999)
213+
.retryMinAttempts(5)
214+
.retryMaxAttempts(10)
215+
.build();
216+
ExceptionHandler handler = ExceptionHandler.builder().retryOn(RuntimeException.class).build();
217+
final AtomicInteger timesCalled = new AtomicInteger(0);
218+
try {
219+
RetryHelper.runWithRetries(callable(new Runnable() {
220+
@Override public void run() {
221+
timesCalled.incrementAndGet();
222+
throw new RuntimeException(new SocketTimeoutException("Simulated socket timeout"));
223+
}
224+
}), params, handler, stopwatch, false);
225+
fail();
226+
} catch (RuntimeException expected) {
227+
// verify timesCalled
228+
assertEquals(1, timesCalled.get());
229+
}
230+
}
231+
205232
@Test
206233
public void testBackoffIsExponential() {
207234
// Total retry period set to 60 seconds so as to not factor into test
@@ -248,7 +275,7 @@ private int invokeNested(final int level, final int retries) {
248275
if (level < 0) {
249276
return 0;
250277
}
251-
return RetryHelper.runWithRetries(new Callable<Integer>() {
278+
return runWithRetries(new Callable<Integer>() {
252279
@Override
253280
public Integer call() throws IOException {
254281
if (RetryHelper.getContext().getAttemptNumber() < retries) {

‎gcloud-java-dns/src/main/java/com/google/gcloud/dns/DnsImpl.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ public Zone create(final ZoneInfo zoneInfo, Dns.ZoneOption... options) {
215215
public ManagedZone call() {
216216
return dnsRpc.create(zoneInfo.toPb(), optionsMap);
217217
}
218-
}, options().retryParams(), EXCEPTION_HANDLER);
218+
}, options().retryParams(), EXCEPTION_HANDLER, false);
219219
return answer == null ? null : Zone.fromPb(this, answer);
220220
} catch (RetryHelper.RetryHelperException ex) {
221221
throw DnsException.translateAndThrow(ex);
@@ -247,7 +247,7 @@ public boolean delete(final String zoneName) {
247247
public Boolean call() {
248248
return dnsRpc.deleteZone(zoneName);
249249
}
250-
}, options().retryParams(), EXCEPTION_HANDLER);
250+
}, options().retryParams(), EXCEPTION_HANDLER, false);
251251
} catch (RetryHelper.RetryHelperException ex) {
252252
throw DnsException.translateAndThrow(ex);
253253
}
@@ -281,7 +281,7 @@ public ChangeRequest applyChangeRequest(final String zoneName,
281281
public Change call() {
282282
return dnsRpc.applyChangeRequest(zoneName, changeRequest.toPb(), optionsMap);
283283
}
284-
}, options().retryParams(), EXCEPTION_HANDLER);
284+
}, options().retryParams(), EXCEPTION_HANDLER, false);
285285
return answer == null ? null : ChangeRequest.fromPb(this, zoneName, answer); // not null
286286
} catch (RetryHelper.RetryHelperException ex) {
287287
throw DnsException.translateAndThrow(ex);

‎gcloud-java-dns/src/test/java/com/google/gcloud/dns/DnsImplTest.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import com.google.common.collect.ImmutableMap;
2727
import com.google.common.collect.Lists;
2828
import com.google.gcloud.Page;
29+
import com.google.gcloud.RetryHelper;
2930
import com.google.gcloud.RetryParams;
3031
import com.google.gcloud.ServiceOptions;
3132
import com.google.gcloud.dns.spi.DnsRpc;
@@ -37,6 +38,7 @@
3738
import org.junit.Before;
3839
import org.junit.Test;
3940

41+
import java.net.SocketTimeoutException;
4042
import java.util.Map;
4143

4244
public class DnsImplTest {

0 commit comments

Comments
 (0)