Skip to content

Commit 03e1eaf

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

3 files changed

Lines changed: 48 additions & 11 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: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import org.junit.Test;
3131

3232
import java.io.IOException;
33+
import java.net.SocketTimeoutException;
3334
import java.util.Arrays;
3435
import java.util.Iterator;
3536
import java.util.concurrent.Callable;
@@ -194,14 +195,40 @@ public void testTriesNoMoreLongerThanTotalRetryPeriod() {
194195
}
195196
throw new RuntimeException();
196197
}
197-
}), params, handler, stopwatch);
198+
}), params, handler, stopwatch, true);
198199
fail();
199200
} catch (RetriesExhaustedException expected) {
200201
// verify timesCalled
201202
assertEquals(sleepOnAttempt, timesCalled.get());
202203
}
203204
}
204205

206+
@Test
207+
public void testNoRetriesOnSocketTimeout() {
208+
final FakeTicker ticker = new FakeTicker();
209+
Stopwatch stopwatch = Stopwatch.createUnstarted(ticker);
210+
RetryParams params = RetryParams.builder().initialRetryDelayMillis(0)
211+
.totalRetryPeriodMillis(999)
212+
.retryMinAttempts(5)
213+
.retryMaxAttempts(10)
214+
.build();
215+
ExceptionHandler handler = ExceptionHandler.builder().retryOn(RuntimeException.class).build();
216+
final AtomicInteger timesCalled = new AtomicInteger(0);
217+
try {
218+
RetryHelper.runWithRetries(callable(new Runnable() {
219+
@Override public void run() {
220+
timesCalled.incrementAndGet();
221+
throw new RetryHelper.RetryHelperException(
222+
new SocketTimeoutException("Simulated socket timeout"));
223+
}
224+
}), params, handler, stopwatch, false);
225+
fail();
226+
} catch (RetryHelper.RetryHelperException 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

‎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);

0 commit comments

Comments
 (0)