From fc30507024be6a9e2e4722e8668f7c2d7625c75b Mon Sep 17 00:00:00 2001 From: vam Date: Fri, 10 Mar 2017 17:09:00 -0800 Subject: [PATCH 01/15] Reconcile RetrySettings & Clock Reconcile RetrySettings in GAX with RetryParams in google-cloud-java https://github.com/GoogleCloudPlatform/google-cloud-java/issues/1574 Reconcile NanoClock in GAX with Clock in google-cloud-java https://github.com/GoogleCloudPlatform/google-cloud-java/issues/1575 --- .../google/api/gax/core/DefaultNanoClock.java | 61 +++++ .../api/gax/{grpc => core}/NanoClock.java | 7 +- .../google/api/gax/core/RetrySettings.java | 32 ++- .../com/google/api/gax/core/SystemClock.java | 58 +++++ .../google/api/gax/grpc/RetryingCallable.java | 240 ++++++------------ .../google/api/gax/grpc/UnaryCallable.java | 6 +- .../gax/retrying/AbstractRetryHandler.java | 137 ++++++++++ .../api/gax/retrying/DirectRetryHandler.java | 81 ++++++ .../gax/retrying/RetryAttemptSettings.java | 106 ++++++++ .../google/api/gax/retrying/RetryFuture.java | 57 +++++ .../api/gax/retrying/RetryFutureImpl.java | 175 +++++++++++++ .../google/api/gax/retrying/RetryHandler.java | 101 ++++++++ .../gax/retrying/ScheduledRetryHandler.java | 87 +++++++ .../google/api/gax/grpc/CancellationTest.java | 85 ++++--- .../google/api/gax/grpc/FakeNanoClock.java | 8 +- .../api/gax/grpc/RecordingScheduler.java | 68 +++-- .../api/gax/grpc/UnaryCallableTest.java | 129 +++++++--- .../retrying/AbstractRetryHandlerTest.java | 140 ++++++++++ .../gax/retrying/DirectRetryHandlerTest.java} | 21 +- .../api/gax/retrying/FailingCallable.java | 72 ++++++ .../retrying/ScheduledRetryHandlerTest.java | 118 +++++++++ version.txt | 2 +- 22 files changed, 1504 insertions(+), 287 deletions(-) create mode 100644 src/main/java/com/google/api/gax/core/DefaultNanoClock.java rename src/main/java/com/google/api/gax/{grpc => core}/NanoClock.java (92%) create mode 100644 src/main/java/com/google/api/gax/core/SystemClock.java create mode 100644 src/main/java/com/google/api/gax/retrying/AbstractRetryHandler.java create mode 100644 src/main/java/com/google/api/gax/retrying/DirectRetryHandler.java create mode 100644 src/main/java/com/google/api/gax/retrying/RetryAttemptSettings.java create mode 100644 src/main/java/com/google/api/gax/retrying/RetryFuture.java create mode 100644 src/main/java/com/google/api/gax/retrying/RetryFutureImpl.java create mode 100644 src/main/java/com/google/api/gax/retrying/RetryHandler.java create mode 100644 src/main/java/com/google/api/gax/retrying/ScheduledRetryHandler.java create mode 100644 src/test/java/com/google/api/gax/retrying/AbstractRetryHandlerTest.java rename src/{main/java/com/google/api/gax/grpc/DefaultNanoClock.java => test/java/com/google/api/gax/retrying/DirectRetryHandlerTest.java} (78%) create mode 100644 src/test/java/com/google/api/gax/retrying/FailingCallable.java create mode 100644 src/test/java/com/google/api/gax/retrying/ScheduledRetryHandlerTest.java diff --git a/src/main/java/com/google/api/gax/core/DefaultNanoClock.java b/src/main/java/com/google/api/gax/core/DefaultNanoClock.java new file mode 100644 index 000000000..38adf29b2 --- /dev/null +++ b/src/main/java/com/google/api/gax/core/DefaultNanoClock.java @@ -0,0 +1,61 @@ +/* + * Copyright 2016, Google Inc. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package com.google.api.gax.core; + +import java.io.ObjectStreamException; +import java.io.Serializable; +import java.util.concurrent.TimeUnit; + +/** Default implementation of the NanoClock interface, using call to System.nanoTime(). */ +public final class DefaultNanoClock implements NanoClock, Serializable { + + private static final NanoClock DEFAULT_CLOCK = new DefaultNanoClock(); + private static final long serialVersionUID = 5541462688633944865L; + + public static NanoClock getDefaultClock() { + return DEFAULT_CLOCK; + } + + private DefaultNanoClock() {} + + @Override + public final long nanoTime() { + return System.nanoTime(); + } + + @Override + public final long millisTime() { + return TimeUnit.MILLISECONDS.convert(nanoTime(), TimeUnit.NANOSECONDS); + } + + private Object readResolve() throws ObjectStreamException { + return DEFAULT_CLOCK; + } +} diff --git a/src/main/java/com/google/api/gax/grpc/NanoClock.java b/src/main/java/com/google/api/gax/core/NanoClock.java similarity index 92% rename from src/main/java/com/google/api/gax/grpc/NanoClock.java rename to src/main/java/com/google/api/gax/core/NanoClock.java index 6f62b42de..e7f01fbaf 100644 --- a/src/main/java/com/google/api/gax/grpc/NanoClock.java +++ b/src/main/java/com/google/api/gax/core/NanoClock.java @@ -27,7 +27,7 @@ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -package com.google.api.gax.grpc; +package com.google.api.gax.core; /** * An interface for getting the current value of a high-resolution time source, in nanoseconds. @@ -43,4 +43,9 @@ public interface NanoClock { * Returns the current value of this clock's high-resolution time source, in nanoseconds. */ long nanoTime(); + + /** + * Returns the current value of this clock's high-resolution time source, in milliseconds. + */ + long millisTime(); } diff --git a/src/main/java/com/google/api/gax/core/RetrySettings.java b/src/main/java/com/google/api/gax/core/RetrySettings.java index d9406b88e..09415e0ce 100644 --- a/src/main/java/com/google/api/gax/core/RetrySettings.java +++ b/src/main/java/com/google/api/gax/core/RetrySettings.java @@ -30,6 +30,7 @@ package com.google.api.gax.core; import com.google.auto.value.AutoValue; +import java.io.Serializable; import org.joda.time.Duration; /** @@ -64,7 +65,9 @@ * is computed which will terminate the call when the total time is up. */ @AutoValue -public abstract class RetrySettings { +public abstract class RetrySettings implements Serializable { + + private static final long serialVersionUID = 8258475264439710899L; /** * The TotalTimeout parameter has ultimate control over how long the logic should keep trying the @@ -91,6 +94,13 @@ public abstract class RetrySettings { */ public abstract Duration getMaxRetryDelay(); + /** + * MaxAttempts defines the maximum number of attempts to perform. If number of attempts reaches + * this limit the logic will give up retrying even if the total retry time is still lower than + * TotalTimeout. + */ + public abstract int getMaxAttempts(); + /** * The InitialRpcTimeout parameter controls the timeout for the initial RPC. Subsequent calls will * use this value adjusted according to the RpcTimeoutMultiplier. @@ -110,7 +120,7 @@ public abstract class RetrySettings { public abstract Duration getMaxRpcTimeout(); public static Builder newBuilder() { - return new AutoValue_RetrySettings.Builder(); + return new AutoValue_RetrySettings.Builder().setMaxAttempts(0); } public Builder toBuilder() { @@ -150,6 +160,13 @@ public abstract static class Builder { */ public abstract Builder setMaxRetryDelay(Duration maxDelay); + /** + * MaxAttempts defines the maximum number of attempts to perform. If number of attempts reaches + * this limit the logic will give up retrying even if the total retry time is still lower than + * TotalTimeout. + */ + public abstract Builder setMaxAttempts(int maxAttempts); + /** * The InitialRpcTimeout parameter controls the timeout for the initial RPC. Subsequent calls * will use this value adjusted according to the RpcTimeoutMultiplier. @@ -188,6 +205,13 @@ public abstract static class Builder { */ public abstract double getRetryDelayMultiplier(); + /** + * MaxAttempts defines the maximum number of attempts to perform. If number of attempts reaches + * this limit the logic will give up retrying even if the total retry time is still lower than + * TotalTimeout. + */ + public abstract int getMaxAttempts(); + /** * The MaxRetryDelay puts a limit on the value of the retry delay, so that the * RetryDelayMultiplier can't increase the retry delay higher than this amount. @@ -228,6 +252,9 @@ public RetrySettings build() { if (params.getMaxRetryDelay().compareTo(params.getInitialRetryDelay()) < 0) { throw new IllegalStateException("max retry delay must not be shorter than initial delay"); } + if (params.getMaxAttempts() < 0) { + throw new IllegalStateException("max attempts must be non-negative"); + } if (params.getInitialRpcTimeout().getMillis() < 0) { throw new IllegalStateException("initial rpc timeout must not be negative"); } @@ -253,6 +280,7 @@ public RetrySettings.Builder merge(RetrySettings.Builder newSettings) { if (newSettings.getMaxRetryDelay() != null) { setMaxRetryDelay(newSettings.getMaxRetryDelay()); } + setMaxAttempts(newSettings.getMaxAttempts()); if (newSettings.getInitialRpcTimeout() != null) { setInitialRpcTimeout(newSettings.getInitialRpcTimeout()); } diff --git a/src/main/java/com/google/api/gax/core/SystemClock.java b/src/main/java/com/google/api/gax/core/SystemClock.java new file mode 100644 index 000000000..22a4e87c9 --- /dev/null +++ b/src/main/java/com/google/api/gax/core/SystemClock.java @@ -0,0 +1,58 @@ +/* + * Copyright 2017, Google Inc. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package com.google.api.gax.core; + +import java.io.ObjectStreamException; +import java.io.Serializable; +import java.util.concurrent.TimeUnit; + +public final class SystemClock implements NanoClock, Serializable { + private static final long serialVersionUID = -6019259882852183285L; + private static final NanoClock DEFAULT_CLOCK = new SystemClock(); + + public static NanoClock getDefaultClock() { + return DEFAULT_CLOCK; + } + + @Override + public long nanoTime() { + return TimeUnit.NANOSECONDS.convert(millisTime(), TimeUnit.MILLISECONDS); + } + + @Override + public long millisTime() { + return System.currentTimeMillis(); + } + + private Object readResolve() throws ObjectStreamException { + return DEFAULT_CLOCK; + } +} diff --git a/src/main/java/com/google/api/gax/grpc/RetryingCallable.java b/src/main/java/com/google/api/gax/grpc/RetryingCallable.java index 811f0abd1..72fe3e3c7 100644 --- a/src/main/java/com/google/api/gax/grpc/RetryingCallable.java +++ b/src/main/java/com/google/api/gax/grpc/RetryingCallable.java @@ -29,60 +29,55 @@ */ package com.google.api.gax.grpc; -import com.google.api.gax.core.AbstractApiFuture; import com.google.api.gax.core.ApiFuture; -import com.google.api.gax.core.ApiFutureCallback; -import com.google.api.gax.core.ApiFutures; +import com.google.api.gax.core.NanoClock; import com.google.api.gax.core.RetrySettings; +import com.google.api.gax.core.internal.ApiFutureToListenableFuture; +import com.google.api.gax.retrying.RetryAttemptSettings; +import com.google.api.gax.retrying.RetryFuture; +import com.google.api.gax.retrying.ScheduledRetryHandler; import com.google.common.base.Preconditions; import io.grpc.CallOptions; -import io.grpc.Status; -import java.util.concurrent.CancellationException; -import java.util.concurrent.Future; -import java.util.concurrent.ThreadLocalRandom; +import io.grpc.Status.Code; +import java.util.concurrent.Callable; +import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; -import javax.annotation.Nullable; import org.joda.time.Duration; /** - * Implements the retry and timeout functionality used in {@link UnaryCallable}. The behavior is - * controlled by the given {@link RetrySettings}. + * Implements the retry and timeout functionality used in {@link UnaryCallable}. + * + * The behavior is controlled by the given {@link RetrySettings}. */ class RetryingCallable implements FutureCallable { - // Duration to sleep on if the error is DEADLINE_EXCEEDED. static final Duration DEADLINE_SLEEP_DURATION = Duration.millis(1); private final FutureCallable callable; private final RetrySettings retryParams; - private final UnaryCallable.Scheduler executor; + private final ScheduledExecutorService scheduler; private final NanoClock clock; RetryingCallable( FutureCallable callable, RetrySettings retrySettings, - UnaryCallable.Scheduler executor, + ScheduledExecutorService scheduler, NanoClock clock) { this.callable = Preconditions.checkNotNull(callable); this.retryParams = Preconditions.checkNotNull(retrySettings); - this.executor = executor; + this.scheduler = scheduler; this.clock = clock; } @Override public ApiFuture futureCall(RequestT request, CallContext context) { - RetryingResultFuture resultFuture = new RetryingResultFuture(); - context = getCallContextWithDeadlineAfter(context, retryParams.getTotalTimeout()); - Retryer retryer = - new Retryer( - request, - context, - resultFuture, - retryParams.getInitialRetryDelay(), - retryParams.getInitialRpcTimeout(), - null); - resultFuture.issueCall(request, context, retryer); - return resultFuture; + GrpcRetryCallable retryCallable = + new GrpcRetryCallable<>(callable, request, context); + GrpcRetryHandler retryHelper = new GrpcRetryHandler<>(clock, scheduler); + RetryFuture rv = retryHelper.createFirstAttempt(retryCallable, retryParams); + retryCallable.setExternalFuture(rv); + retryCallable.call(); + return rv; } @Override @@ -90,137 +85,6 @@ public String toString() { return String.format("retrying(%s)", callable); } - private class Retryer implements Runnable, ApiFutureCallback { - private final RequestT request; - private final CallContext context; - private final RetryingResultFuture resultFuture; - private final Duration retryDelay; - private final Duration rpcTimeout; - private final Throwable savedThrowable; - - private Retryer( - RequestT request, - CallContext context, - RetryingResultFuture resultFuture, - Duration retryDelay, - Duration rpcTimeout, - Throwable savedThrowable) { - this.request = request; - this.context = context; - this.resultFuture = resultFuture; - this.retryDelay = retryDelay; - this.rpcTimeout = rpcTimeout; - this.savedThrowable = savedThrowable; - } - - @Override - public void run() { - if (context.getCallOptions().getDeadlineNanoTime() < clock.nanoTime()) { - if (savedThrowable == null) { - resultFuture.setException( - Status.DEADLINE_EXCEEDED - .withDescription("Total deadline exceeded without completing any call") - .asException()); - } else { - resultFuture.setException(savedThrowable); - } - return; - } - CallContext deadlineContext = getCallContextWithDeadlineAfter(context, rpcTimeout); - resultFuture.issueCall(request, deadlineContext, this); - } - - @Override - public void onSuccess(ResponseT r) { - resultFuture.set(r); - } - - @Override - public void onFailure(Throwable throwable) { - if (!canRetry(throwable)) { - resultFuture.setException(throwable); - return; - } - if (isDeadlineExceeded(throwable)) { - Retryer retryer = - new Retryer(request, context, resultFuture, retryDelay, rpcTimeout, throwable); - resultFuture.scheduleNext( - executor, retryer, DEADLINE_SLEEP_DURATION.getMillis(), TimeUnit.MILLISECONDS); - return; - } - - long newRetryDelay = (long) (retryDelay.getMillis() * retryParams.getRetryDelayMultiplier()); - newRetryDelay = Math.min(newRetryDelay, retryParams.getMaxRetryDelay().getMillis()); - - long newRpcTimeout = (long) (rpcTimeout.getMillis() * retryParams.getRpcTimeoutMultiplier()); - newRpcTimeout = Math.min(newRpcTimeout, retryParams.getMaxRpcTimeout().getMillis()); - - long randomRetryDelay = ThreadLocalRandom.current().nextLong(retryDelay.getMillis()); - Retryer retryer = - new Retryer( - request, - context, - resultFuture, - Duration.millis(newRetryDelay), - Duration.millis(newRpcTimeout), - throwable); - resultFuture.scheduleNext(executor, retryer, randomRetryDelay, TimeUnit.MILLISECONDS); - } - } - - private class RetryingResultFuture extends AbstractApiFuture { - private volatile Future activeFuture = null; - private final Object syncObject = new Object(); - - @Override - protected void interruptTask() { - synchronized (syncObject) { - activeFuture.cancel(true); - } - } - - @Override - public boolean set(@Nullable ResponseT value) { - synchronized (syncObject) { - return super.set(value); - } - } - - @Override - public boolean setException(Throwable throwable) { - synchronized (syncObject) { - if (throwable instanceof CancellationException) { - super.cancel(false); - return true; - } else { - return super.setException(throwable); - } - } - } - - private void scheduleNext( - UnaryCallable.Scheduler executor, Runnable retryer, long delay, TimeUnit unit) { - synchronized (syncObject) { - if (!isCancelled()) { - activeFuture = executor.schedule(retryer, delay, TimeUnit.MILLISECONDS); - } - } - } - - public void issueCall( - RequestT request, - CallContext deadlineContext, - RetryingCallable.Retryer retryer) { - synchronized (syncObject) { - if (!isCancelled()) { - ApiFuture callFuture = callable.futureCall(request, deadlineContext); - ApiFutures.addCallback(callFuture, retryer); - activeFuture = callFuture; - } - } - } - } - private static CallContext getCallContextWithDeadlineAfter( CallContext oldCtx, Duration rpcTimeout) { CallOptions oldOpt = oldCtx.getCallOptions(); @@ -236,19 +100,61 @@ private static CallContext getCallContextWithDeadlineAfter( return newCtx; } - private static boolean canRetry(Throwable throwable) { - if (!(throwable instanceof ApiException)) { - return false; + private static class GrpcRetryCallable implements Callable { + private final FutureCallable callable; + private final RequestT request; + + private RetryFuture externalFuture; + private CallContext callContext; + + private GrpcRetryCallable( + FutureCallable callable, RequestT request, CallContext callContext) { + this.callable = callable; + this.request = request; + this.callContext = callContext; + } + + private void setExternalFuture(RetryFuture externalFuture) { + this.externalFuture = externalFuture; + } + + @Override + public ResponseT call() { + callContext = + getCallContextWithDeadlineAfter( + callContext, externalFuture.getAttemptSettings().getRpcTimeout()); + ApiFuture internalFuture = callable.futureCall(request, callContext); + externalFuture.setAttemptFuture(new ApiFutureToListenableFuture<>(internalFuture)); + return null; } - ApiException apiException = (ApiException) throwable; - return apiException.isRetryable(); } - private static boolean isDeadlineExceeded(Throwable throwable) { - if (!(throwable instanceof ApiException)) { - return false; + private static class GrpcRetryHandler extends ScheduledRetryHandler { + private GrpcRetryHandler(NanoClock clock, ScheduledExecutorService scheduler) { + super(clock, scheduler); + } + + @Override + public boolean accept(Throwable e, RetryAttemptSettings nextAttemptSettings) { + return super.accept(e, nextAttemptSettings) + && (e instanceof ApiException) + && ((ApiException) e).isRetryable(); + } + + @Override + public RetryAttemptSettings createNextAttemptSettings( + Throwable e, RetryAttemptSettings prevSettings) { + if (((ApiException) e).getStatusCode() == Code.DEADLINE_EXCEEDED) { + return new RetryAttemptSettings( + prevSettings.getGlobalSettings(), + prevSettings.getRetryDelay(), + prevSettings.getRpcTimeout(), + DEADLINE_SLEEP_DURATION, + prevSettings.getAttemptCount() + 1, + prevSettings.getFirstAttemptStartTime()); + } + + return super.createNextAttemptSettings(e, prevSettings); } - ApiException apiException = (ApiException) throwable; - return apiException.getStatusCode() == Status.Code.DEADLINE_EXCEEDED; } } diff --git a/src/main/java/com/google/api/gax/grpc/UnaryCallable.java b/src/main/java/com/google/api/gax/grpc/UnaryCallable.java index e579d64ed..604553314 100644 --- a/src/main/java/com/google/api/gax/grpc/UnaryCallable.java +++ b/src/main/java/com/google/api/gax/grpc/UnaryCallable.java @@ -30,6 +30,8 @@ package com.google.api.gax.grpc; import com.google.api.gax.core.ApiFuture; +import com.google.api.gax.core.DefaultNanoClock; +import com.google.api.gax.core.NanoClock; import com.google.api.gax.core.RetrySettings; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; @@ -323,7 +325,7 @@ UnaryCallable retryableOn(ImmutableSet retryab */ UnaryCallable retrying( RetrySettings retrySettings, ScheduledExecutorService executor) { - return retrying(retrySettings, new DelegatingScheduler(executor), DefaultNanoClock.create()); + return retrying(retrySettings, executor, DefaultNanoClock.getDefaultClock()); } /** @@ -336,7 +338,7 @@ UnaryCallable retrying( */ @VisibleForTesting UnaryCallable retrying( - RetrySettings retrySettings, Scheduler executor, NanoClock clock) { + RetrySettings retrySettings, ScheduledExecutorService executor, NanoClock clock) { return new UnaryCallable<>( new RetryingCallable<>(callable, retrySettings, executor, clock), channel, settings); } diff --git a/src/main/java/com/google/api/gax/retrying/AbstractRetryHandler.java b/src/main/java/com/google/api/gax/retrying/AbstractRetryHandler.java new file mode 100644 index 000000000..053d360a6 --- /dev/null +++ b/src/main/java/com/google/api/gax/retrying/AbstractRetryHandler.java @@ -0,0 +1,137 @@ +/* + * Copyright 2017, Google Inc. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package com.google.api.gax.retrying; + +import com.google.api.gax.core.NanoClock; +import com.google.api.gax.core.RetrySettings; +import java.util.concurrent.Callable; +import java.util.concurrent.ThreadLocalRandom; +import java.util.concurrent.TimeUnit; +import org.joda.time.Duration; + +/** + * Basic implementation of the retry {@link RetryHandler} interface. Responsible for defining and + * checking attempt's basic properties (execution time and count limits). + * + * This class is thread-safe, and all inheriting classes are required to be thread-safe. + * + * @param response type + */ +public abstract class AbstractRetryHandler implements RetryHandler { + + private final NanoClock clock; + + protected AbstractRetryHandler(NanoClock clock) { + this.clock = clock; + } + + /** + * Ensures that the retry logic hasn't exceeded neither maximum number of retries nor the total + * execution timeout. + * + * @param e exception thrown by the previous attempt + * @param nextAttemptSettings attempt settings, which will be used for the next attempt, if + * accepted + * @return {@code true} if none of the retry limits are exceeded + */ + @Override + public boolean accept(Throwable e, RetryAttemptSettings nextAttemptSettings) { + RetrySettings globalSettings = nextAttemptSettings.getGlobalSettings(); + long randRetryDelayMillis = nextAttemptSettings.getRandomizedRetryDelay().getMillis(); + long totalTimeSpentNanos = + clock.nanoTime() + - nextAttemptSettings.getFirstAttemptStartTime() + + TimeUnit.NANOSECONDS.convert(randRetryDelayMillis, TimeUnit.MILLISECONDS); + + long totalTimeoutMillis = globalSettings.getTotalTimeout().getMillis(); + long totalTimeoutNanos = + TimeUnit.NANOSECONDS.convert(totalTimeoutMillis, TimeUnit.MILLISECONDS); + + return totalTimeSpentNanos <= totalTimeoutNanos + && (globalSettings.getMaxAttempts() <= 0 + || nextAttemptSettings.getAttemptCount() < globalSettings.getMaxAttempts()); + } + + /** + * Crates next attempt settings. It increments the current attempt count and uses randomized + * exponential backoff factor for calculating next attempt execution time. + * + * @param e exception thrown by the previous attempt + * @param prevSettings previous attempt settings + * @return next attempt settings + */ + @Override + public RetryAttemptSettings createNextAttemptSettings( + Throwable e, RetryAttemptSettings prevSettings) { + RetrySettings settings = prevSettings.getGlobalSettings(); + + long newRetryDelay = settings.getInitialRetryDelay().getMillis(); + long newRpcTimeout = settings.getInitialRpcTimeout().getMillis(); + + if (prevSettings.getAttemptCount() > 0) { + newRetryDelay = + (long) (settings.getRetryDelayMultiplier() * prevSettings.getRetryDelay().getMillis()); + newRetryDelay = Math.min(newRetryDelay, settings.getMaxRetryDelay().getMillis()); + newRpcTimeout = + (long) (settings.getRpcTimeoutMultiplier() * prevSettings.getRpcTimeout().getMillis()); + newRpcTimeout = Math.min(newRpcTimeout, settings.getMaxRpcTimeout().getMillis()); + } + + return new RetryAttemptSettings( + prevSettings.getGlobalSettings(), + Duration.millis(newRetryDelay), + Duration.millis(newRpcTimeout), + Duration.millis(ThreadLocalRandom.current().nextLong(newRetryDelay)), + prevSettings.getAttemptCount() + 1, + prevSettings.getFirstAttemptStartTime()); + } + + /** + * Creates first attempt future. By default the first attempt is configured to be executed + * immediately. + * + * @param callable the actual callable, which should be executed in a retriable context + * @param globalSettings global retry settings (attempt independent) + */ + @Override + public RetryFuture createFirstAttempt( + Callable callable, RetrySettings globalSettings) { + RetryAttemptSettings firstAttemptSettings = + new RetryAttemptSettings( + globalSettings, + Duration.ZERO, + globalSettings.getTotalTimeout(), + Duration.ZERO, + 0, + clock.nanoTime()); + + return new RetryFutureImpl<>(callable, firstAttemptSettings, this); + } +} diff --git a/src/main/java/com/google/api/gax/retrying/DirectRetryHandler.java b/src/main/java/com/google/api/gax/retrying/DirectRetryHandler.java new file mode 100644 index 000000000..093a4ac3c --- /dev/null +++ b/src/main/java/com/google/api/gax/retrying/DirectRetryHandler.java @@ -0,0 +1,81 @@ +/* + * Copyright 2017, Google Inc. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package com.google.api.gax.retrying; + +import com.google.api.gax.core.NanoClock; +import com.google.common.util.concurrent.Futures; +import java.io.InterruptedIOException; +import java.nio.channels.ClosedByInterruptException; +import java.util.concurrent.Callable; +import java.util.concurrent.Future; +import org.joda.time.Duration; + +/** + * The retry handler, which executes attempts in the current thread, potentially causing the current + * thread to sleep for a specified amount of type before execution. + * + * This class is thread-safe. + * + * @param response type + */ +public class DirectRetryHandler extends AbstractRetryHandler { + + /** + * Creates a new default direct retry handler + * + * @param clock clock to use during execution scheduling + */ + public DirectRetryHandler(NanoClock clock) { + super(clock); + } + + /** + * Executes attempt in the current thread. Causes the current thread to sleep, if it is not the + * first attempt. + * + * @param callable the actual callable to execute + * @param attemptSettings current attempt settings + */ + @Override + public Future executeAttempt( + Callable callable, RetryAttemptSettings attemptSettings) { + try { + if (Duration.ZERO.compareTo(attemptSettings.getRandomizedRetryDelay()) < 0) { + Thread.sleep(attemptSettings.getRandomizedRetryDelay().getMillis()); + } + return Futures.immediateFuture(callable.call()); + } catch (InterruptedException | InterruptedIOException | ClosedByInterruptException e) { + Thread.currentThread().interrupt(); + return Futures.immediateFailedFuture(e); + } catch (Throwable e) { + return Futures.immediateFailedFuture(e); + } + } +} diff --git a/src/main/java/com/google/api/gax/retrying/RetryAttemptSettings.java b/src/main/java/com/google/api/gax/retrying/RetryAttemptSettings.java new file mode 100644 index 000000000..0916e4c94 --- /dev/null +++ b/src/main/java/com/google/api/gax/retrying/RetryAttemptSettings.java @@ -0,0 +1,106 @@ +/* + * Copyright 2017, Google Inc. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package com.google.api.gax.retrying; + +import com.google.api.gax.core.RetrySettings; +import org.joda.time.Duration; + +/** + * Execution attempt settings. Defines attempt-specific properties of a retry process. + */ +public class RetryAttemptSettings { + + private final RetrySettings globalSettings; + private final Duration retryDelay; + private final Duration rpcTimeout; + private final Duration randomizedRetryDelay; + private final int attemptCount; + private final long firstAttemptStartTime; + + public RetryAttemptSettings( + RetrySettings globalSettings, + Duration retryDelay, + Duration rpcTimeout, + Duration randomizedRetryDelay, + int attemptCount, + long firstAttemptStartTime) { + this.globalSettings = globalSettings; + this.retryDelay = retryDelay; + this.rpcTimeout = rpcTimeout; + this.randomizedRetryDelay = randomizedRetryDelay; + this.attemptCount = attemptCount; + this.firstAttemptStartTime = firstAttemptStartTime; + } + + /** + * Returns global (attempt-independent) retry settings. + */ + public RetrySettings getGlobalSettings() { + return globalSettings; + } + + /** + * Returns the calculated retry delay. Note that the actual delay used for retry scheduling may be + * different (randomized, based on this value). + */ + public Duration getRetryDelay() { + return retryDelay; + } + + /** + * Returns rpc timeout used for this attempt. + */ + public Duration getRpcTimeout() { + return rpcTimeout; + } + + /** + * Returns randomized attempt delay. By default this value is calculated based on the + * {@code retryDelay} value, and is used as the actual attempt execution delay. + */ + public Duration getRandomizedRetryDelay() { + return randomizedRetryDelay; + } + + /** + * The attempt count. It is a zero-based value (first attempt will have this value set to 0). + */ + public int getAttemptCount() { + return attemptCount; + } + + /** + * The start time of the first attempt. Note that this value is dependent on the actual + * {@link com.google.api.gax.core.NanoClock} used during the process. + */ + public long getFirstAttemptStartTime() { + return firstAttemptStartTime; + } +} diff --git a/src/main/java/com/google/api/gax/retrying/RetryFuture.java b/src/main/java/com/google/api/gax/retrying/RetryFuture.java new file mode 100644 index 000000000..80672edc4 --- /dev/null +++ b/src/main/java/com/google/api/gax/retrying/RetryFuture.java @@ -0,0 +1,57 @@ +/* + * Copyright 2017, Google Inc. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package com.google.api.gax.retrying; + +import com.google.api.gax.core.ApiFuture; +import java.util.concurrent.Future; + +/** + * Represents retriable future. This is a facade hiding all the complications of a an asynchronous + * execution of a retriable task. + * + * This interface is for advanced/internal use only. + * + * @param response type + */ +public interface RetryFuture extends ApiFuture { + + /** + * Set the attempt future. This future represents a concrete retry attempt, potentially scheduled + * for execution in some form of {@link java.util.concurrent.ScheduledExecutorService}. + * + * @param attemptFuture the attempt future + */ + void setAttemptFuture(Future attemptFuture); + + /** + * Returns current (active) attempt settings. + */ + RetryAttemptSettings getAttemptSettings(); +} diff --git a/src/main/java/com/google/api/gax/retrying/RetryFutureImpl.java b/src/main/java/com/google/api/gax/retrying/RetryFutureImpl.java new file mode 100644 index 000000000..9c44b0bc1 --- /dev/null +++ b/src/main/java/com/google/api/gax/retrying/RetryFutureImpl.java @@ -0,0 +1,175 @@ +/* + * Copyright 2017, Google Inc. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package com.google.api.gax.retrying; + +import com.google.api.gax.core.ApiFutureCallback; +import com.google.common.util.concurrent.AbstractFuture; +import com.google.common.util.concurrent.FutureCallback; +import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.ListenableFuture; +import java.util.concurrent.Callable; +import java.util.concurrent.Future; + +/** + * For internal use only. + * + * This class is the key component of the retry logic. It implements {@link RetryFuture} facade + * interface, and is doing the following: + * + *
    + *
  • Schedules next attempt in case of failure using callback chaining technique.
    • + *
  • Terminates retrying process if more retries are not accepted.
    • + *
  • Propagates future cancellation in both directions (from this to the attempt and from the + * attempt to this)
    • + *
+ * + * This class is thread-safe. + */ +class RetryFutureImpl extends AbstractFuture + implements RetryFuture { + + private final Object lock = new Object(); + private final Callable callable; + private final RetryHandler retryHandler; + + private volatile RetryAttemptSettings attemptSettings; + private volatile AttemptFutureCallback callbackFutureCallback; + + RetryFutureImpl( + Callable callable, + RetryAttemptSettings attemptSettings, + RetryHandler retryHandler) { + this.callable = callable; + this.attemptSettings = attemptSettings; + this.retryHandler = retryHandler; + } + + @Override + protected boolean set(ResponseT value) { + return super.set(value); + } + + @Override + protected boolean setException(Throwable throwable) { + return super.setException(throwable); + } + + @Override + public boolean cancel(boolean mayInterruptIfRunning) { + synchronized (lock) { + boolean rv = super.cancel(mayInterruptIfRunning); + if (callbackFutureCallback != null) { + callbackFutureCallback.attemptFuture.cancel(mayInterruptIfRunning); + } + return rv; + } + } + + @Override + public RetryAttemptSettings getAttemptSettings() { + synchronized (lock) { + return attemptSettings; + } + } + + @Override + public void setAttemptFuture(Future attemptFuture) { + synchronized (lock) { + if (attemptFuture != null) { + callbackFutureCallback = new AttemptFutureCallback(attemptFuture); + Futures.addCallback((ListenableFuture) attemptFuture, callbackFutureCallback); + if (isCancelled()) { + attemptFuture.cancel(false); + } + } else { + callbackFutureCallback = null; + } + } + } + + private void scheduleRetry(Throwable delegateThrowable, Future prevProxiedFuture) { + try { + if (prevProxiedFuture.isCancelled()) { + cancel(false); + } + if (isDone()) { + return; + } + + RetryAttemptSettings nextAttemptSettings = + retryHandler.createNextAttemptSettings(delegateThrowable, attemptSettings); + if (retryHandler.accept(delegateThrowable, nextAttemptSettings)) { + attemptSettings = nextAttemptSettings; + Future nextInternalFuture = + retryHandler.executeAttempt(callable, attemptSettings); + setAttemptFuture(nextInternalFuture); + } else { + setException(delegateThrowable); + } + } catch (Throwable e) { + setException(delegateThrowable); + } + } + + private class AttemptFutureCallback + implements FutureCallback, ApiFutureCallback { + + private Future attemptFuture; + + private AttemptFutureCallback(Future attemptFuture) { + this.attemptFuture = attemptFuture; + } + + @Override + public void onSuccess(ResponseT result) { + if (this == callbackFutureCallback && !isDone()) { + synchronized (lock) { + if (this == callbackFutureCallback && !isDone()) { + setAttemptFuture(null); + set(result); + } + } + } + } + + @Override + public void onFailure(Throwable t) { + if (this == callbackFutureCallback && !isDone()) { + synchronized (lock) { + if (this == callbackFutureCallback && !isDone()) { + setAttemptFuture(null); + scheduleRetry(t, this.attemptFuture); + } + } + } + } + } +} diff --git a/src/main/java/com/google/api/gax/retrying/RetryHandler.java b/src/main/java/com/google/api/gax/retrying/RetryHandler.java new file mode 100644 index 000000000..8ea6037f2 --- /dev/null +++ b/src/main/java/com/google/api/gax/retrying/RetryHandler.java @@ -0,0 +1,101 @@ +/* + * Copyright 2017, Google Inc. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package com.google.api.gax.retrying; + +import com.google.api.gax.core.RetrySettings; +import java.util.concurrent.Callable; +import java.util.concurrent.Future; + +/** + * A retry handler is responsible for the following operations: + *
    + *
  1. Accepting or rejecting a task for retry, depending on the previous attempt result (exception) + * and/or other attempt properties (like number of already executed attempts or total time spent + * retrying).
    2. + * + *
  2. Creating first attempt {@link RetryFuture}, which acts as a facade, hiding from client code + * the actual scheduled retry task.
    3. + * + *
  3. Creating {@link RetryAttemptSettings} for each subsequnt retry attempt.
    4. + * + *
  4. Executing the actual {@link Callable} in a retriable context.
    5. + * + *
+ * + * This interface is for internal/advanced use only. + * + * @param response type + */ +public interface RetryHandler { + + /** + * Returns {@code true} if another attempt should be made, or {@code false} otherwise + * + * @param e exception thrown by the previous attempt + * @param nextAttemptSettings attempt settings, which will be used for the next attempt, if + * accepted + * @return {@code true} if another attempt should be made, or {@code false} otherwise + */ + boolean accept(Throwable e, RetryAttemptSettings nextAttemptSettings); + + /** + * Creates a first try {@link RetryFuture}, which is a facade, returned to the client code to wait + * for any retriable operation to complete. + * + * @param callable the actual callable, which should be executed in a retriable context + * @param globalSettings global retry settings (attempt independent) + * @return retriable future facade + */ + RetryFuture createFirstAttempt( + Callable callable, RetrySettings globalSettings); + + /** + * Creates next attempt {@link RetryAttemptSettings}, which defines properties of the next + * attempt. Eventually this object will be passed to + * {@link RetryHandler#accept(Throwable, RetryAttemptSettings)} and + * {@link RetryHandler#executeAttempt(Callable, RetryAttemptSettings)}. + * + * @param e exception thrown by the previous attempt + * @param prevSettings previous attempt settings + * @return next attempt settings + */ + RetryAttemptSettings createNextAttemptSettings(Throwable e, RetryAttemptSettings prevSettings); + + /** + * Executes an attempt. A typical implementation will either try to execute in the current thread + * or schedule it for an execution, using some sort of async execution service. + * + * @param callable the actual callable to execute + * @param attemptSettings current attempt settings + * @return the {@link Future}, representing the scheduled execution + */ + Future executeAttempt( + Callable callable, RetryAttemptSettings attemptSettings); +} diff --git a/src/main/java/com/google/api/gax/retrying/ScheduledRetryHandler.java b/src/main/java/com/google/api/gax/retrying/ScheduledRetryHandler.java new file mode 100644 index 000000000..5be4d741d --- /dev/null +++ b/src/main/java/com/google/api/gax/retrying/ScheduledRetryHandler.java @@ -0,0 +1,87 @@ +/* + * Copyright 2017, Google Inc. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package com.google.api.gax.retrying; + +import com.google.api.gax.core.NanoClock; +import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.ListeningScheduledExecutorService; +import com.google.common.util.concurrent.MoreExecutors; +import java.util.concurrent.Callable; +import java.util.concurrent.Future; +import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; + +/** + * A retry handler which uses {@link ScheduledExecutorService} to schedule attempt tasks. Unless a + * direct executor service is used, this handler will schedule attempts for an execution in another + * thread. + * + * This class is thread-safe. + * + * @param + */ +public class ScheduledRetryHandler extends AbstractRetryHandler { + private final ListeningScheduledExecutorService scheduler; + + /** + * Creates new scheduled retry handler, which will be using {@link ScheduledExecutorService} for + * acutal attempts scheduling. + * + * @param clock clock to use for scheduling operations + * @param scheduler scheduler + */ + public ScheduledRetryHandler(NanoClock clock, ScheduledExecutorService scheduler) { + super(clock); + this.scheduler = MoreExecutors.listeningDecorator(scheduler); + } + + /** + * Executes attempt using previously provided shceduller. + * + * @param callable the actual callable to execute + * @param attemptSettings current attempt settings + * @return actual attempt future + */ + @Override + public Future executeAttempt( + Callable callable, RetryAttemptSettings attemptSettings) { + try { + System.out.println( + "Scheduling with delay = " + attemptSettings.getRandomizedRetryDelay().getMillis()); + + return scheduler.schedule( + callable, attemptSettings.getRandomizedRetryDelay().getMillis(), TimeUnit.MILLISECONDS); + } catch (RejectedExecutionException e) { + System.out.println("Rejected"); + return Futures.immediateCancelledFuture(); + } + } +} diff --git a/src/test/java/com/google/api/gax/grpc/CancellationTest.java b/src/test/java/com/google/api/gax/grpc/CancellationTest.java index 7b4f92cff..d951805e6 100644 --- a/src/test/java/com/google/api/gax/grpc/CancellationTest.java +++ b/src/test/java/com/google/api/gax/grpc/CancellationTest.java @@ -29,11 +29,14 @@ */ package com.google.api.gax.grpc; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyLong; +import static org.mockito.Mockito.when; + import com.google.api.gax.core.AbstractApiFuture; import com.google.api.gax.core.ApiFuture; import com.google.api.gax.core.RetrySettings; import com.google.api.gax.core.SettableApiFuture; -import com.google.api.gax.grpc.UnaryCallable.Scheduler; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Lists; import com.google.common.truth.Truth; @@ -54,6 +57,8 @@ import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import org.mockito.Mockito; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; @RunWith(JUnit4.class) public class CancellationTest { @@ -90,7 +95,7 @@ public class CancellationTest { @Before public void resetClock() { fakeClock = new FakeNanoClock(System.nanoTime()); - executor = new RecordingScheduler(fakeClock); + executor = RecordingScheduler.get(fakeClock); } @After @@ -115,24 +120,36 @@ public void cancellationBeforeGetOnRetryingCallable() throws Exception { resultFuture.get(); } - private static class LatchCountDownScheduler implements UnaryCallable.Scheduler { - private final ScheduledExecutorService executor; - private final CountDownLatch latch; - - LatchCountDownScheduler(CountDownLatch latch) { - this.executor = new ScheduledThreadPoolExecutor(1); - this.latch = latch; - } - - @Override - public ScheduledFuture schedule(Runnable runnable, long delay, TimeUnit unit) { - latch.countDown(); - return executor.schedule(runnable, delay, unit); - } - - @Override - public List shutdownNow() { - return executor.shutdownNow(); + private abstract static class LatchCountDownScheduler implements ScheduledExecutorService { + private static LatchCountDownScheduler get(final CountDownLatch latch) { + LatchCountDownScheduler mock = Mockito.mock(LatchCountDownScheduler.class); + + // mock class fields: + final ScheduledExecutorService executor = new ScheduledThreadPoolExecutor(1); + + // mock class methods: + // ScheduledFuture schedule(Runnable command, long delay, TimeUnit unit); + when(mock.schedule(any(Runnable.class), anyLong(), any(TimeUnit.class))) + .then( + new Answer>() { + @Override + public ScheduledFuture answer(InvocationOnMock invocation) throws Throwable { + Object[] args = invocation.getArguments(); + latch.countDown(); + return executor.schedule((Runnable) args[0], (Long) args[1], (TimeUnit) args[2]); + } + }); + // List shutdownNow() + when(mock.shutdownNow()) + .then( + new Answer>() { + @Override + public List answer(InvocationOnMock invocation) throws Throwable { + return executor.shutdownNow(); + } + }); + + return mock; } } @@ -190,10 +207,7 @@ public void cancellationDuringFirstCall() throws Exception { UnaryCallable callable = UnaryCallable.create(innerCallable) .retryableOn(retryable) - .retrying( - FAST_RETRY_SETTINGS, - new UnaryCallable.DelegatingScheduler(new ScheduledThreadPoolExecutor(1)), - fakeClock); + .retrying(FAST_RETRY_SETTINGS, new ScheduledThreadPoolExecutor(1), fakeClock); ApiFuture resultFuture = callable.futureCall(0); CancellationHelpers.cancelInThreadAfterLatchCountDown(resultFuture, callIssuedLatch); @@ -210,16 +224,16 @@ public void cancellationDuringFirstCall() throws Exception { @Test public void cancellationDuringRetryDelay() throws Exception { Throwable throwable = Status.UNAVAILABLE.asException(); - CancellationTrackingFuture innerFuture = CancellationTrackingFuture.create(); + CancellationTrackingFuture innerFuture = CancellationTrackingFuture.create(); Mockito.when(callInt.futureCall((Integer) Mockito.any(), (CallContext) Mockito.any())) .thenReturn(UnaryCallableTest.immediateFailedFuture(throwable)) .thenReturn(innerFuture); CountDownLatch retryScheduledLatch = new CountDownLatch(1); - Scheduler scheduler = new LatchCountDownScheduler(retryScheduledLatch); - ImmutableSet retryable = ImmutableSet.of(Status.Code.UNAVAILABLE); + LatchCountDownScheduler scheduler = LatchCountDownScheduler.get(retryScheduledLatch); + ImmutableSet retryable = ImmutableSet.of(Status.Code.UNAVAILABLE); UnaryCallable callable = - UnaryCallable.create(callInt) + UnaryCallable.create(callInt) .retryableOn(retryable) .retrying(SLOW_RETRY_SETTINGS, scheduler, fakeClock); @@ -239,21 +253,18 @@ public void cancellationDuringRetryDelay() throws Exception { public void cancellationDuringSecondCall() throws Exception { Throwable throwable = Status.UNAVAILABLE.asException(); ApiFuture failingFuture = UnaryCallableTest.immediateFailedFuture(throwable); - CancellationTrackingFuture innerFuture = CancellationTrackingFuture.create(); + CancellationTrackingFuture innerFuture = CancellationTrackingFuture.create(); CountDownLatch callIssuedLatch = new CountDownLatch(2); @SuppressWarnings("unchecked") FutureCallable innerCallable = - new LatchCountDownFutureCallable( - callIssuedLatch, Lists.>newArrayList(failingFuture, innerFuture)); + new LatchCountDownFutureCallable<>( + callIssuedLatch, Lists.newArrayList(failingFuture, innerFuture)); - ImmutableSet retryable = ImmutableSet.of(Status.Code.UNAVAILABLE); + ImmutableSet retryable = ImmutableSet.of(Status.Code.UNAVAILABLE); UnaryCallable callable = - UnaryCallable.create(innerCallable) + UnaryCallable.create(innerCallable) .retryableOn(retryable) - .retrying( - FAST_RETRY_SETTINGS, - new UnaryCallable.DelegatingScheduler(new ScheduledThreadPoolExecutor(1)), - fakeClock); + .retrying(FAST_RETRY_SETTINGS, new ScheduledThreadPoolExecutor(1), fakeClock); ApiFuture resultFuture = callable.futureCall(0); CancellationHelpers.cancelInThreadAfterLatchCountDown(resultFuture, callIssuedLatch); diff --git a/src/test/java/com/google/api/gax/grpc/FakeNanoClock.java b/src/test/java/com/google/api/gax/grpc/FakeNanoClock.java index 1fb7695cc..a735a9af3 100644 --- a/src/test/java/com/google/api/gax/grpc/FakeNanoClock.java +++ b/src/test/java/com/google/api/gax/grpc/FakeNanoClock.java @@ -29,8 +29,9 @@ */ package com.google.api.gax.grpc; -class FakeNanoClock implements NanoClock { +import com.google.api.gax.core.NanoClock; +class FakeNanoClock implements NanoClock { private volatile long currentNanoTime; public FakeNanoClock(long initialNanoTime) { @@ -42,6 +43,11 @@ public long nanoTime() { return currentNanoTime; } + @Override + public long millisTime() { + return nanoTime() / 1000_000L; + } + public void setCurrentNanoTime(long nanoTime) { currentNanoTime = nanoTime; } diff --git a/src/test/java/com/google/api/gax/grpc/RecordingScheduler.java b/src/test/java/com/google/api/gax/grpc/RecordingScheduler.java index 66f9c4c11..01a3f6e16 100644 --- a/src/test/java/com/google/api/gax/grpc/RecordingScheduler.java +++ b/src/test/java/com/google/api/gax/grpc/RecordingScheduler.java @@ -29,6 +29,10 @@ */ package com.google.api.gax.grpc; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyLong; +import static org.mockito.Mockito.when; + import java.util.ArrayList; import java.util.List; import java.util.concurrent.ScheduledExecutorService; @@ -36,34 +40,52 @@ import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; import org.joda.time.Duration; +import org.mockito.Mockito; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; -class RecordingScheduler implements UnaryCallable.Scheduler { - private final ScheduledExecutorService executor; - private final List sleepDurations = new ArrayList<>(); - private final FakeNanoClock clock; +abstract class RecordingScheduler implements ScheduledExecutorService { - public RecordingScheduler(FakeNanoClock clock) { - this.executor = new ScheduledThreadPoolExecutor(1); - this.clock = clock; - } + abstract List getSleepDurations(); - @Override - public ScheduledFuture schedule(Runnable runnable, long delay, TimeUnit unit) { - sleepDurations.add(new Duration(TimeUnit.MILLISECONDS.convert(delay, unit))); - clock.setCurrentNanoTime(clock.nanoTime() + TimeUnit.NANOSECONDS.convert(delay, unit)); - return executor.schedule(runnable, 0, TimeUnit.NANOSECONDS); - } + static RecordingScheduler get(final FakeNanoClock clock) { + RecordingScheduler mock = Mockito.mock(RecordingScheduler.class); - @Override - public List shutdownNow() { - return executor.shutdownNow(); - } + // mock class fields: + final ScheduledExecutorService executor = new ScheduledThreadPoolExecutor(1); + final List sleepDurations = new ArrayList<>(); - public List getSleepDurations() { - return sleepDurations; - } + // mock class methods: + // ScheduledFuture schedule(Runnable command, long delay, TimeUnit unit); + when(mock.schedule(any(Runnable.class), anyLong(), any(TimeUnit.class))) + .then( + new Answer>() { + @Override + public ScheduledFuture answer(InvocationOnMock invocation) throws Throwable { + Object[] args = invocation.getArguments(); + Runnable runnable = (Runnable) args[0]; + Long delay = (Long) args[1]; + TimeUnit unit = (TimeUnit) args[2]; + sleepDurations.add(new Duration(TimeUnit.MILLISECONDS.convert(delay, unit))); + clock.setCurrentNanoTime( + clock.nanoTime() + TimeUnit.NANOSECONDS.convert(delay, unit)); + return executor.schedule(runnable, 0, TimeUnit.NANOSECONDS); + } + }); + + // List shutdownNow() + when(mock.shutdownNow()) + .then( + new Answer>() { + @Override + public List answer(InvocationOnMock invocation) throws Throwable { + return executor.shutdownNow(); + } + }); + + // List getSleepDurations() + when(mock.getSleepDurations()).thenReturn(sleepDurations); - public FakeNanoClock getClock() { - return clock; + return mock; } } diff --git a/src/test/java/com/google/api/gax/grpc/UnaryCallableTest.java b/src/test/java/com/google/api/gax/grpc/UnaryCallableTest.java index 619f7002b..2b0dffe5d 100644 --- a/src/test/java/com/google/api/gax/grpc/UnaryCallableTest.java +++ b/src/test/java/com/google/api/gax/grpc/UnaryCallableTest.java @@ -92,7 +92,7 @@ static ApiFuture immediateFailedFuture(Throwable t) { @Before public void resetClock() { fakeClock = new FakeNanoClock(System.nanoTime()); - executor = new RecordingScheduler(fakeClock); + executor = RecordingScheduler.get(fakeClock); } @After @@ -124,7 +124,7 @@ public ApiFuture futureCall(RequestT request, CallContext context) { public void simpleCall() throws Exception { StashCallable stash = new StashCallable<>(1); - UnaryCallable callable = UnaryCallable.create(stash); + UnaryCallable callable = UnaryCallable.create(stash); Integer response = callable.call(2, CallContext.createDefault()); Truth.assertThat(response).isEqualTo(Integer.valueOf(1)); Truth.assertThat(stash.context.getChannel()).isNull(); @@ -137,7 +137,7 @@ public void simpleCall() throws Exception { public void bind() { Channel channel = Mockito.mock(Channel.class); StashCallable stash = new StashCallable<>(0); - UnaryCallable.create(stash).bind(channel).futureCall(0); + UnaryCallable.create(stash).bind(channel).futureCall(0); Truth.assertThat(stash.context.getChannel()).isSameAs(channel); } @@ -146,9 +146,9 @@ public void retryableBind() throws Exception { Channel channel = Mockito.mock(Channel.class); StashCallable stash = new StashCallable<>(0); - ImmutableSet retryable = ImmutableSet.of(Status.Code.UNAVAILABLE); + ImmutableSet retryable = ImmutableSet.of(Status.Code.UNAVAILABLE); UnaryCallable callable = - UnaryCallable.create(stash) + UnaryCallable.create(stash) .bind(channel) .retryableOn(retryable) .retrying(FAST_RETRY_SETTINGS, executor, fakeClock); @@ -162,10 +162,7 @@ public void pagedBind() { StashCallable> stash = new StashCallable>(new ArrayList()); - UnaryCallable.>create(stash) - .bind(channel) - .paged(new PagedFactory()) - .call(0); + UnaryCallable.create(stash).bind(channel).paged(new PagedFactory()).call(0); Truth.assertThat(stash.context.getChannel()).isSameAs(channel); } @@ -253,7 +250,7 @@ public void bundlingBind() throws Exception { @Test public void retry() { - ImmutableSet retryable = ImmutableSet.of(Status.Code.UNAVAILABLE); + ImmutableSet retryable = ImmutableSet.of(Status.Code.UNAVAILABLE); Throwable throwable = Status.UNAVAILABLE.asException(); Mockito.when(callInt.futureCall((Integer) Mockito.any(), (CallContext) Mockito.any())) .thenReturn(UnaryCallableTest.immediateFailedFuture(throwable)) @@ -261,15 +258,71 @@ public void retry() { .thenReturn(UnaryCallableTest.immediateFailedFuture(throwable)) .thenReturn(immediateFuture(2)); UnaryCallable callable = - UnaryCallable.create(callInt) + UnaryCallable.create(callInt) .retryableOn(retryable) .retrying(FAST_RETRY_SETTINGS, executor, fakeClock); Truth.assertThat(callable.call(1)).isEqualTo(2); } + @Test(expected = ApiException.class) + public void retryTotalTimeoutExceeded() { + ImmutableSet retryable = ImmutableSet.of(Status.Code.UNAVAILABLE); + Throwable throwable = Status.UNAVAILABLE.asException(); + Mockito.when(callInt.futureCall((Integer) Mockito.any(), (CallContext) Mockito.any())) + .thenReturn(UnaryCallableTest.immediateFailedFuture(throwable)) + .thenReturn(immediateFuture(2)); + + RetrySettings retrySettings = + FAST_RETRY_SETTINGS + .toBuilder() + .setInitialRetryDelay(Duration.millis(Integer.MAX_VALUE)) + .setMaxRetryDelay(Duration.millis(Integer.MAX_VALUE)) + .build(); + UnaryCallable callable = + UnaryCallable.create(callInt) + .retryableOn(retryable) + .retrying(retrySettings, executor, fakeClock); + callable.call(1); + } + + @Test(expected = ApiException.class) + public void retryMaxAttemptsExeeded() { + ImmutableSet retryable = ImmutableSet.of(Status.Code.UNAVAILABLE); + Throwable throwable = Status.UNAVAILABLE.asException(); + Mockito.when(callInt.futureCall((Integer) Mockito.any(), (CallContext) Mockito.any())) + .thenReturn(UnaryCallableTest.immediateFailedFuture(throwable)) + .thenReturn(UnaryCallableTest.immediateFailedFuture(throwable)) + .thenReturn(immediateFuture(2)); + + RetrySettings retrySettings = FAST_RETRY_SETTINGS.toBuilder().setMaxAttempts(2).build(); + UnaryCallable callable = + UnaryCallable.create(callInt) + .retryableOn(retryable) + .retrying(retrySettings, executor, fakeClock); + callable.call(1); + } + + @Test + public void retryWithinMaxAttempts() { + ImmutableSet retryable = ImmutableSet.of(Status.Code.UNAVAILABLE); + Throwable throwable = Status.UNAVAILABLE.asException(); + Mockito.when(callInt.futureCall((Integer) Mockito.any(), (CallContext) Mockito.any())) + .thenReturn(UnaryCallableTest.immediateFailedFuture(throwable)) + .thenReturn(UnaryCallableTest.immediateFailedFuture(throwable)) + .thenReturn(immediateFuture(2)); + + RetrySettings retrySettings = FAST_RETRY_SETTINGS.toBuilder().setMaxAttempts(3).build(); + UnaryCallable callable = + UnaryCallable.create(callInt) + .retryableOn(retryable) + .retrying(retrySettings, executor, fakeClock); + callable.call(1); + Truth.assertThat(callable.call(1)).isEqualTo(2); + } + @Test public void retryOnStatusUnknown() { - ImmutableSet retryable = ImmutableSet.of(Status.Code.UNKNOWN); + ImmutableSet retryable = ImmutableSet.of(Status.Code.UNKNOWN); Throwable throwable = Status.UNKNOWN.asException(); Mockito.when(callInt.futureCall((Integer) Mockito.any(), (CallContext) Mockito.any())) .thenReturn(UnaryCallableTest.immediateFailedFuture(throwable)) @@ -277,7 +330,7 @@ public void retryOnStatusUnknown() { .thenReturn(UnaryCallableTest.immediateFailedFuture(throwable)) .thenReturn(immediateFuture(2)); UnaryCallable callable = - UnaryCallable.create(callInt) + UnaryCallable.create(callInt) .retryableOn(retryable) .retrying(FAST_RETRY_SETTINGS, executor, fakeClock); Truth.assertThat(callable.call(1)).isEqualTo(2); @@ -287,12 +340,12 @@ public void retryOnStatusUnknown() { public void retryOnUnexpectedException() { thrown.expect(ApiException.class); thrown.expectMessage("foobar"); - ImmutableSet retryable = ImmutableSet.of(Status.Code.UNKNOWN); + ImmutableSet retryable = ImmutableSet.of(Status.Code.UNKNOWN); Throwable throwable = new RuntimeException("foobar"); Mockito.when(callInt.futureCall((Integer) Mockito.any(), (CallContext) Mockito.any())) .thenReturn(UnaryCallableTest.immediateFailedFuture(throwable)); UnaryCallable callable = - UnaryCallable.create(callInt) + UnaryCallable.create(callInt) .retryableOn(retryable) .retrying(FAST_RETRY_SETTINGS, executor, fakeClock); callable.call(1); @@ -302,14 +355,14 @@ public void retryOnUnexpectedException() { public void retryNoRecover() { thrown.expect(ApiException.class); thrown.expectMessage("foobar"); - ImmutableSet retryable = ImmutableSet.of(Status.Code.UNAVAILABLE); + ImmutableSet retryable = ImmutableSet.of(Status.Code.UNAVAILABLE); Mockito.when(callInt.futureCall((Integer) Mockito.any(), (CallContext) Mockito.any())) .thenReturn( UnaryCallableTest.immediateFailedFuture( Status.FAILED_PRECONDITION.withDescription("foobar").asException())) .thenReturn(immediateFuture(2)); UnaryCallable callable = - UnaryCallable.create(callInt) + UnaryCallable.create(callInt) .retryableOn(retryable) .retrying(FAST_RETRY_SETTINGS, executor, fakeClock); callable.call(1); @@ -319,13 +372,13 @@ public void retryNoRecover() { public void retryKeepFailing() { thrown.expect(UncheckedExecutionException.class); thrown.expectMessage("foobar"); - ImmutableSet retryable = ImmutableSet.of(Status.Code.UNAVAILABLE); + ImmutableSet retryable = ImmutableSet.of(Status.Code.UNAVAILABLE); Mockito.when(callInt.futureCall((Integer) Mockito.any(), (CallContext) Mockito.any())) .thenReturn( UnaryCallableTest.immediateFailedFuture( Status.UNAVAILABLE.withDescription("foobar").asException())); UnaryCallable callable = - UnaryCallable.create(callInt) + UnaryCallable.create(callInt) .retryableOn(retryable) .retrying(FAST_RETRY_SETTINGS, executor, fakeClock); // Need to advance time inside the call. @@ -336,7 +389,7 @@ public void retryKeepFailing() { @Test public void noSleepOnRetryTimeout() { ImmutableSet retryable = - ImmutableSet.of(Status.Code.UNAVAILABLE, Status.Code.DEADLINE_EXCEEDED); + ImmutableSet.of(Status.Code.UNAVAILABLE, Status.Code.DEADLINE_EXCEEDED); Mockito.when(callInt.futureCall((Integer) Mockito.any(), (CallContext) Mockito.any())) .thenReturn( UnaryCallableTest.immediateFailedFuture( @@ -344,7 +397,7 @@ public void noSleepOnRetryTimeout() { .thenReturn(immediateFuture(2)); UnaryCallable callable = - UnaryCallable.create(callInt) + UnaryCallable.create(callInt) .retryableOn(retryable) .retrying(FAST_RETRY_SETTINGS, executor, fakeClock); callable.call(1); @@ -421,7 +474,7 @@ public void paged() { .thenReturn(immediateFuture(Arrays.asList(3, 4))) .thenReturn(immediateFuture(Collections.emptyList())); Truth.assertThat( - UnaryCallable.>create(callIntList) + UnaryCallable.create(callIntList) .paged(new PagedFactory()) .call(0) .iterateAllElements()) @@ -436,10 +489,7 @@ public void pagedByPage() { .thenReturn(immediateFuture(Arrays.asList(3, 4))) .thenReturn(immediateFuture(Collections.emptyList())); Page, Integer> page = - UnaryCallable.>create(callIntList) - .paged(new PagedFactory()) - .call(0) - .getPage(); + UnaryCallable.create(callIntList).paged(new PagedFactory()).call(0).getPage(); Truth.assertThat(page).containsExactly(0, 1, 2).inOrder(); Truth.assertThat(page.getNextPage()).containsExactly(3, 4).inOrder(); @@ -453,7 +503,7 @@ public void pagedByFixedSizeCollection() { .thenReturn(immediateFuture(Arrays.asList(5, 6, 7))) .thenReturn(immediateFuture(Collections.emptyList())); FixedSizeCollection fixedSizeCollection = - UnaryCallable.>create(callIntList) + UnaryCallable.create(callIntList) .paged(new PagedFactory()) .call(0) .expandToFixedSizeCollection(5); @@ -469,7 +519,7 @@ public void pagedFixedSizeCollectionTooManyElements() { .thenReturn(immediateFuture(Arrays.asList(3, 4))) .thenReturn(immediateFuture(Collections.emptyList())); - UnaryCallable.>create(callIntList) + UnaryCallable.create(callIntList) .paged(new PagedFactory()) .call(0) .expandToFixedSizeCollection(4); @@ -481,7 +531,7 @@ public void pagedFixedSizeCollectionTooSmallCollectionSize() { .thenReturn(immediateFuture(Arrays.asList(0, 1))) .thenReturn(immediateFuture(Collections.emptyList())); - UnaryCallable.>create(callIntList) + UnaryCallable.create(callIntList) .paged(new PagedFactory()) .call(0) .expandToFixedSizeCollection(2); @@ -591,7 +641,7 @@ public void bundling() throws Exception { new BundlerFactory<>(SQUARER_BUNDLING_DESC, bundlingSettings); try { UnaryCallable> callable = - UnaryCallable.>create(callLabeledIntSquarer) + UnaryCallable.create(callLabeledIntSquarer) .bundling(SQUARER_BUNDLING_DESC, bundlerFactory); ApiFuture> f1 = callable.futureCall(new LabeledIntList("one", 1, 2)); ApiFuture> f2 = callable.futureCall(new LabeledIntList("one", 3, 4)); @@ -650,7 +700,7 @@ public void bundlingDisabled() throws Exception { new BundlerFactory<>(DISABLED_BUNDLING_DESC, bundlingSettings); try { UnaryCallable> callable = - UnaryCallable.>create(callLabeledIntSquarer) + UnaryCallable.create(callLabeledIntSquarer) .bundling(DISABLED_BUNDLING_DESC, bundlerFactory); ApiFuture> f1 = callable.futureCall(new LabeledIntList("one", 1, 2)); ApiFuture> f2 = callable.futureCall(new LabeledIntList("one", 3, 4)); @@ -671,7 +721,7 @@ public void bundlingWithBlockingCallThreshold() throws Exception { new BundlerFactory<>(SQUARER_BUNDLING_DESC, bundlingSettings); try { UnaryCallable> callable = - UnaryCallable.>create(callLabeledIntSquarer) + UnaryCallable.create(callLabeledIntSquarer) .bundling(SQUARER_BUNDLING_DESC, bundlerFactory); ApiFuture> f1 = callable.futureCall(new LabeledIntList("one", 1)); ApiFuture> f2 = callable.futureCall(new LabeledIntList("one", 3)); @@ -686,8 +736,7 @@ public void bundlingWithBlockingCallThreshold() throws Exception { new FutureCallable>() { @Override public ApiFuture> futureCall(LabeledIntList request, CallContext context) { - return UnaryCallableTest.>immediateFailedFuture( - new IllegalArgumentException("I FAIL!!")); + return UnaryCallableTest.immediateFailedFuture(new IllegalArgumentException("I FAIL!!")); } }; @@ -702,7 +751,7 @@ public void bundlingException() throws Exception { new BundlerFactory<>(SQUARER_BUNDLING_DESC, bundlingSettings); try { UnaryCallable> callable = - UnaryCallable.>create(callLabeledIntExceptionThrower) + UnaryCallable.create(callLabeledIntExceptionThrower) .bundling(SQUARER_BUNDLING_DESC, bundlerFactory); ApiFuture> f1 = callable.futureCall(new LabeledIntList("one", 1, 2)); ApiFuture> f2 = callable.futureCall(new LabeledIntList("one", 3, 4)); @@ -728,13 +777,12 @@ public void bundlingException() throws Exception { @Test public void testKnownStatusCode() { - ImmutableSet retryable = ImmutableSet.of(Status.Code.UNAVAILABLE); + ImmutableSet retryable = ImmutableSet.of(Status.Code.UNAVAILABLE); Mockito.when(callInt.futureCall((Integer) Mockito.any(), (CallContext) Mockito.any())) .thenReturn( UnaryCallableTest.immediateFailedFuture( Status.FAILED_PRECONDITION.withDescription("known").asException())); - UnaryCallable callable = - UnaryCallable.create(callInt).retryableOn(retryable); + UnaryCallable callable = UnaryCallable.create(callInt).retryableOn(retryable); try { callable.call(1); } catch (ApiException exception) { @@ -746,12 +794,11 @@ public void testKnownStatusCode() { @Test public void testUnknownStatusCode() { - ImmutableSet retryable = ImmutableSet.of(); + ImmutableSet retryable = ImmutableSet.of(); Mockito.when(callInt.futureCall((Integer) Mockito.any(), (CallContext) Mockito.any())) .thenReturn( UnaryCallableTest.immediateFailedFuture(new RuntimeException("unknown"))); - UnaryCallable callable = - UnaryCallable.create(callInt).retryableOn(retryable); + UnaryCallable callable = UnaryCallable.create(callInt).retryableOn(retryable); try { callable.call(1); } catch (ApiException exception) { diff --git a/src/test/java/com/google/api/gax/retrying/AbstractRetryHandlerTest.java b/src/test/java/com/google/api/gax/retrying/AbstractRetryHandlerTest.java new file mode 100644 index 000000000..85bf40d65 --- /dev/null +++ b/src/test/java/com/google/api/gax/retrying/AbstractRetryHandlerTest.java @@ -0,0 +1,140 @@ +/* + * Copyright 2017, Google Inc. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package com.google.api.gax.retrying; + +import static com.google.api.gax.retrying.FailingCallable.FAST_RETRY_SETTINGS; +import static junit.framework.TestCase.assertFalse; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import com.google.api.gax.core.RetrySettings; +import com.google.api.gax.retrying.FailingCallable.CustomException; +import java.util.concurrent.CancellationException; +import java.util.concurrent.ExecutionException; +import org.joda.time.Duration; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public abstract class AbstractRetryHandlerTest { + + protected abstract RetryHandler getRetryHandler(); + + @Test + public void testNoFailures() throws ExecutionException, InterruptedException { + RetryHandler handler = getRetryHandler(); + FailingCallable callable = new FailingCallable(0, "SUCCESS"); + RetryFuture future = handler.createFirstAttempt(callable, FAST_RETRY_SETTINGS); + future.setAttemptFuture(handler.executeAttempt(callable, future.getAttemptSettings())); + assertEquals("SUCCESS", future.get()); + assertTrue(future.isDone()); + assertFalse(future.isCancelled()); + assertEquals(0, future.getAttemptSettings().getAttemptCount()); + } + + @Test + public void testSuccessWithFailures() throws ExecutionException, InterruptedException { + RetryHandler handler = getRetryHandler(); + FailingCallable callable = new FailingCallable(5, "SUCCESS"); + RetryFuture future = handler.createFirstAttempt(callable, FAST_RETRY_SETTINGS); + future.setAttemptFuture(handler.executeAttempt(callable, future.getAttemptSettings())); + assertEquals("SUCCESS", future.get()); + assertTrue(future.isDone()); + assertFalse(future.isCancelled()); + assertEquals(5, future.getAttemptSettings().getAttemptCount()); + } + + @Test + public void testMaxRetriesExcceeded() { + RetryHandler handler = getRetryHandler(); + FailingCallable callable = new FailingCallable(6, "FAILURE"); + RetryFuture future = handler.createFirstAttempt(callable, FAST_RETRY_SETTINGS); + future.setAttemptFuture(handler.executeAttempt(callable, future.getAttemptSettings())); + + CustomException exception = null; + try { + future.get(); + } catch (Exception e) { + exception = (CustomException) e.getCause(); + } + assertEquals(CustomException.class, exception.getClass()); + + assertEquals(5, future.getAttemptSettings().getAttemptCount()); + assertTrue(future.isDone()); + assertFalse(future.isCancelled()); + } + + @Test + public void testTotalTimeoutExcceeded() throws Exception { + RetryHandler handler = getRetryHandler(); + FailingCallable callable = new FailingCallable(6, "FAILURE"); + RetrySettings retrySettings = + FAST_RETRY_SETTINGS + .toBuilder() + .setInitialRetryDelay(Duration.millis(Integer.MAX_VALUE)) + .setMaxRetryDelay(Duration.millis(Integer.MAX_VALUE)) + .build(); + RetryFuture future = handler.createFirstAttempt(callable, retrySettings); + future.setAttemptFuture(handler.executeAttempt(callable, future.getAttemptSettings())); + + CustomException exception = null; + try { + future.get(); + } catch (Exception e) { + exception = (CustomException) e.getCause(); + } + assertEquals(CustomException.class, exception.getClass()); + assertTrue(future.getAttemptSettings().getAttemptCount() < 4); + assertTrue(future.isDone()); + assertFalse(future.isCancelled()); + } + + @Test(expected = CancellationException.class) + public void testCancelOuterFuture() throws ExecutionException, InterruptedException { + RetryHandler handler = getRetryHandler(); + FailingCallable callable = new FailingCallable(4, "SUCCESS"); + RetrySettings retrySettings = + FAST_RETRY_SETTINGS + .toBuilder() + .setInitialRetryDelay(Duration.millis(1_000L)) + .setMaxRetryDelay(Duration.millis(1_000L)) + .setTotalTimeout(Duration.millis(10_0000L)) + .build(); + + RetryFuture future = handler.createFirstAttempt(callable, retrySettings); + future.cancel(false); + future.setAttemptFuture(handler.executeAttempt(callable, future.getAttemptSettings())); + assertTrue(future.isDone()); + assertTrue(future.isCancelled()); + assertTrue(future.getAttemptSettings().getAttemptCount() < 4); + future.get(); + } +} diff --git a/src/main/java/com/google/api/gax/grpc/DefaultNanoClock.java b/src/test/java/com/google/api/gax/retrying/DirectRetryHandlerTest.java similarity index 78% rename from src/main/java/com/google/api/gax/grpc/DefaultNanoClock.java rename to src/test/java/com/google/api/gax/retrying/DirectRetryHandlerTest.java index d0d308333..9d6070529 100644 --- a/src/main/java/com/google/api/gax/grpc/DefaultNanoClock.java +++ b/src/test/java/com/google/api/gax/retrying/DirectRetryHandlerTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2016, Google Inc. All rights reserved. + * Copyright 2017, Google Inc. All rights reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -27,20 +27,17 @@ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -package com.google.api.gax.grpc; +package com.google.api.gax.retrying; -/** - * Default implementation of the NanoClock interface, using call to System.nanoTime(). - */ -public final class DefaultNanoClock implements NanoClock { - public static NanoClock create() { - return new DefaultNanoClock(); - } +import com.google.api.gax.core.DefaultNanoClock; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; - private DefaultNanoClock() {} +@RunWith(JUnit4.class) +public class DirectRetryHandlerTest extends AbstractRetryHandlerTest { @Override - public final long nanoTime() { - return System.nanoTime(); + protected RetryHandler getRetryHandler() { + return new DirectRetryHandler<>(DefaultNanoClock.getDefaultClock()); } } diff --git a/src/test/java/com/google/api/gax/retrying/FailingCallable.java b/src/test/java/com/google/api/gax/retrying/FailingCallable.java new file mode 100644 index 000000000..3b2f0aeac --- /dev/null +++ b/src/test/java/com/google/api/gax/retrying/FailingCallable.java @@ -0,0 +1,72 @@ +/* + * Copyright 2017, Google Inc. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package com.google.api.gax.retrying; + +import com.google.api.gax.core.RetrySettings; +import java.util.concurrent.Callable; +import java.util.concurrent.atomic.AtomicInteger; +import org.joda.time.Duration; + +class FailingCallable implements Callable { + protected static final RetrySettings FAST_RETRY_SETTINGS = + RetrySettings.newBuilder() + .setMaxAttempts(6) + .setInitialRetryDelay(Duration.millis(2L)) + .setRetryDelayMultiplier(1) + .setMaxRetryDelay(Duration.millis(2L)) + .setInitialRpcTimeout(Duration.millis(2L)) + .setRpcTimeoutMultiplier(1) + .setMaxRpcTimeout(Duration.millis(2L)) + .setTotalTimeout(Duration.millis(10L)) + .build(); + + private AtomicInteger attemptsCount = new AtomicInteger(0); + private final int expectedFailuresCount; + private final String result; + + protected FailingCallable(int expectedFailuresCount, String result) { + this.expectedFailuresCount = expectedFailuresCount; + this.result = result; + } + + @Override + public String call() throws Exception { + if (attemptsCount.getAndIncrement() < expectedFailuresCount) { + System.out.println("Throw: " + attemptsCount.get()); + throw new CustomException(); + } + return result; + } + + protected static class CustomException extends RuntimeException { + + private static final long serialVersionUID = -1543459008653697004L; + } +} diff --git a/src/test/java/com/google/api/gax/retrying/ScheduledRetryHandlerTest.java b/src/test/java/com/google/api/gax/retrying/ScheduledRetryHandlerTest.java new file mode 100644 index 000000000..fc30861bb --- /dev/null +++ b/src/test/java/com/google/api/gax/retrying/ScheduledRetryHandlerTest.java @@ -0,0 +1,118 @@ +/* + * Copyright 2017, Google Inc. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package com.google.api.gax.retrying; + +import static com.google.api.gax.retrying.FailingCallable.FAST_RETRY_SETTINGS; +import static org.junit.Assert.assertTrue; + +import com.google.api.gax.core.DefaultNanoClock; +import com.google.api.gax.core.RetrySettings; +import java.util.concurrent.CancellationException; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import org.joda.time.Duration; +import org.junit.After; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class ScheduledRetryHandlerTest extends AbstractRetryHandlerTest { + + private ScheduledExecutorService executorService = Executors.newSingleThreadScheduledExecutor(); + private ScheduledRetryHandler retryHandler = + new ScheduledRetryHandler<>(DefaultNanoClock.getDefaultClock(), executorService); + + @After + public void after() { + executorService.shutdownNow(); + } + + @Override + protected RetryHandler getRetryHandler() { + return retryHandler; + } + + @Test(expected = CancellationException.class) + public void testCancelOuterFutureAfterStart() throws ExecutionException, InterruptedException { + RetryHandler handler = getRetryHandler(); + FailingCallable callable = new FailingCallable(4, "SUCCESS"); + RetrySettings retrySettings = + FAST_RETRY_SETTINGS + .toBuilder() + .setInitialRetryDelay(Duration.millis(1_000L)) + .setMaxRetryDelay(Duration.millis(1_000L)) + .setTotalTimeout(Duration.millis(10_0000L)) + .build(); + + RetryFuture future = handler.createFirstAttempt(callable, retrySettings); + future.setAttemptFuture(handler.executeAttempt(callable, future.getAttemptSettings())); + future.cancel(false); + assertTrue(future.isDone()); + assertTrue(future.isCancelled()); + assertTrue(future.getAttemptSettings().getAttemptCount() < 4); + future.get(); + } + + @Test(expected = CancellationException.class) + public void testCancelProxiedFutureAfterStart() throws ExecutionException, InterruptedException { + RetryHandler handler = getRetryHandler(); + FailingCallable callable = new FailingCallable(5, "SUCCESS"); + RetrySettings retrySettings = + FAST_RETRY_SETTINGS + .toBuilder() + .setInitialRetryDelay(Duration.millis(1_000L)) + .setMaxRetryDelay(Duration.millis(1_000L)) + .setTotalTimeout(Duration.millis(10_0000L)) + .build(); + + RetryFuture future = handler.createFirstAttempt(callable, retrySettings); + future.setAttemptFuture(handler.executeAttempt(callable, future.getAttemptSettings())); + Thread.sleep(50L); + + //Note that shutdownNow() will not cancel internal FutureTasks automatically, which + //may potentially cause another thread handing on RetryFuture#get() call forever. + //Canceling the tasks returned by shutdownNow() also does not help, because of missing feature + //in guava's ListenableScheduledFuture, which does not cancel itself, when its delegate is canceled. + //So only the graceful shutdown() is supported properly. + executorService.shutdown(); + + try { + future.get(); + } catch (CancellationException e) { + //Used to wait for cancellation to propagate, so isDone() is guaranteed to return true. + } + assertTrue(future.isDone()); + assertTrue(future.isCancelled()); + assertTrue(future.getAttemptSettings().getAttemptCount() < 4); + future.get(); + } +} diff --git a/version.txt b/version.txt index 66ec8b17f..cf2499627 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -0.3.1-SNAPSHOT +0.3.2-SNAPSHOT From b31ce2b106bd1e1575aec148798a69384203c512 Mon Sep 17 00:00:00 2001 From: vam Date: Fri, 10 Mar 2017 17:22:11 -0800 Subject: [PATCH 02/15] Removed System.out.println --- .../com/google/api/gax/retrying/ScheduledRetryHandler.java | 4 ---- .../java/com/google/api/gax/retrying/FailingCallable.java | 1 - 2 files changed, 5 deletions(-) diff --git a/src/main/java/com/google/api/gax/retrying/ScheduledRetryHandler.java b/src/main/java/com/google/api/gax/retrying/ScheduledRetryHandler.java index 5be4d741d..07997e95b 100644 --- a/src/main/java/com/google/api/gax/retrying/ScheduledRetryHandler.java +++ b/src/main/java/com/google/api/gax/retrying/ScheduledRetryHandler.java @@ -74,13 +74,9 @@ public ScheduledRetryHandler(NanoClock clock, ScheduledExecutorService scheduler public Future executeAttempt( Callable callable, RetryAttemptSettings attemptSettings) { try { - System.out.println( - "Scheduling with delay = " + attemptSettings.getRandomizedRetryDelay().getMillis()); - return scheduler.schedule( callable, attemptSettings.getRandomizedRetryDelay().getMillis(), TimeUnit.MILLISECONDS); } catch (RejectedExecutionException e) { - System.out.println("Rejected"); return Futures.immediateCancelledFuture(); } } diff --git a/src/test/java/com/google/api/gax/retrying/FailingCallable.java b/src/test/java/com/google/api/gax/retrying/FailingCallable.java index 3b2f0aeac..68a3ee5f8 100644 --- a/src/test/java/com/google/api/gax/retrying/FailingCallable.java +++ b/src/test/java/com/google/api/gax/retrying/FailingCallable.java @@ -59,7 +59,6 @@ protected FailingCallable(int expectedFailuresCount, String result) { @Override public String call() throws Exception { if (attemptsCount.getAndIncrement() < expectedFailuresCount) { - System.out.println("Throw: " + attemptsCount.get()); throw new CustomException(); } return result; From 922bdf586a79474d0eee28a863fdc45675454fd3 Mon Sep 17 00:00:00 2001 From: vam Date: Fri, 10 Mar 2017 17:50:35 -0800 Subject: [PATCH 03/15] Removed unused code, increased test coverage. --- .../com/google/api/gax/core/SystemClock.java | 3 ++ .../google/api/gax/grpc/UnaryCallable.java | 32 ++++--------------- .../gax/retrying/DirectRetryHandlerTest.java | 4 +-- 3 files changed, 11 insertions(+), 28 deletions(-) diff --git a/src/main/java/com/google/api/gax/core/SystemClock.java b/src/main/java/com/google/api/gax/core/SystemClock.java index 22a4e87c9..c57ea5069 100644 --- a/src/main/java/com/google/api/gax/core/SystemClock.java +++ b/src/main/java/com/google/api/gax/core/SystemClock.java @@ -35,6 +35,7 @@ import java.util.concurrent.TimeUnit; public final class SystemClock implements NanoClock, Serializable { + private static final long serialVersionUID = -6019259882852183285L; private static final NanoClock DEFAULT_CLOCK = new SystemClock(); @@ -42,6 +43,8 @@ public static NanoClock getDefaultClock() { return DEFAULT_CLOCK; } + private SystemClock() {} + @Override public long nanoTime() { return TimeUnit.NANOSECONDS.convert(millisTime(), TimeUnit.MILLISECONDS); diff --git a/src/main/java/com/google/api/gax/grpc/UnaryCallable.java b/src/main/java/com/google/api/gax/grpc/UnaryCallable.java index 604553314..25202da4f 100644 --- a/src/main/java/com/google/api/gax/grpc/UnaryCallable.java +++ b/src/main/java/com/google/api/gax/grpc/UnaryCallable.java @@ -99,30 +99,6 @@ */ public final class UnaryCallable { - interface Scheduler { - ScheduledFuture schedule(Runnable runnable, long delay, TimeUnit unit); - - List shutdownNow(); - } - - static class DelegatingScheduler implements Scheduler { - private final ScheduledExecutorService executor; - - DelegatingScheduler(ScheduledExecutorService executor) { - this.executor = executor; - } - - @Override - public ScheduledFuture schedule(Runnable runnable, long delay, TimeUnit unit) { - return executor.schedule(runnable, delay, unit); - } - - @Override - public List shutdownNow() { - return executor.shutdownNow(); - } - } - private final FutureCallable callable; private final Channel channel; @Nullable private final UnaryCallSettings settings; @@ -217,7 +193,9 @@ public UnaryCallSettings getSettings() { return settings; } - /** Package-private for internal use. */ + /** + * Package-private for internal use. + */ UnaryCallable( FutureCallable callable, Channel channel, UnaryCallSettings settings) { this.callable = Preconditions.checkNotNull(callable); @@ -225,7 +203,9 @@ public UnaryCallSettings getSettings() { this.settings = settings; } - /** Package-private for internal use. */ + /** + * Package-private for internal use. + */ UnaryCallable(FutureCallable callable) { this(callable, null, null); } diff --git a/src/test/java/com/google/api/gax/retrying/DirectRetryHandlerTest.java b/src/test/java/com/google/api/gax/retrying/DirectRetryHandlerTest.java index 9d6070529..81eb66af0 100644 --- a/src/test/java/com/google/api/gax/retrying/DirectRetryHandlerTest.java +++ b/src/test/java/com/google/api/gax/retrying/DirectRetryHandlerTest.java @@ -29,7 +29,7 @@ */ package com.google.api.gax.retrying; -import com.google.api.gax.core.DefaultNanoClock; +import com.google.api.gax.core.SystemClock; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -38,6 +38,6 @@ public class DirectRetryHandlerTest extends AbstractRetryHandlerTest { @Override protected RetryHandler getRetryHandler() { - return new DirectRetryHandler<>(DefaultNanoClock.getDefaultClock()); + return new DirectRetryHandler<>(SystemClock.getDefaultClock()); } } From 97568959c6baff6c4266bfb844aeb9109329927c Mon Sep 17 00:00:00 2001 From: vam Date: Mon, 13 Mar 2017 10:25:52 -0700 Subject: [PATCH 04/15] Fixed some documentation grammatical mistakes. --- .../api/gax/retrying/AbstractRetryHandler.java | 4 ++-- .../com/google/api/gax/retrying/RetryFuture.java | 4 ++-- .../com/google/api/gax/retrying/RetryFutureImpl.java | 12 ++++++------ .../com/google/api/gax/retrying/RetryHandler.java | 8 ++++---- .../api/gax/retrying/ScheduledRetryHandler.java | 6 +++--- .../java/com/google/api/gax/grpc/FakeNanoClock.java | 3 ++- 6 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/main/java/com/google/api/gax/retrying/AbstractRetryHandler.java b/src/main/java/com/google/api/gax/retrying/AbstractRetryHandler.java index 053d360a6..d9310f579 100644 --- a/src/main/java/com/google/api/gax/retrying/AbstractRetryHandler.java +++ b/src/main/java/com/google/api/gax/retrying/AbstractRetryHandler.java @@ -37,7 +37,7 @@ import org.joda.time.Duration; /** - * Basic implementation of the retry {@link RetryHandler} interface. Responsible for defining and + * Basic implementation of the {@link RetryHandler} interface. It is responsible for defining and * checking attempt's basic properties (execution time and count limits). * * This class is thread-safe, and all inheriting classes are required to be thread-safe. @@ -53,7 +53,7 @@ protected AbstractRetryHandler(NanoClock clock) { } /** - * Ensures that the retry logic hasn't exceeded neither maximum number of retries nor the total + * Ensures that the retry logic hasn't exceeded either maximum number of retries or the total * execution timeout. * * @param e exception thrown by the previous attempt diff --git a/src/main/java/com/google/api/gax/retrying/RetryFuture.java b/src/main/java/com/google/api/gax/retrying/RetryFuture.java index 80672edc4..7c4a0260b 100644 --- a/src/main/java/com/google/api/gax/retrying/RetryFuture.java +++ b/src/main/java/com/google/api/gax/retrying/RetryFuture.java @@ -33,7 +33,7 @@ import java.util.concurrent.Future; /** - * Represents retriable future. This is a facade hiding all the complications of a an asynchronous + * Represents retriable future. This is a facade hiding all the complications of an asynchronous * execution of a retriable task. * * This interface is for advanced/internal use only. @@ -44,7 +44,7 @@ public interface RetryFuture extends ApiFuture { /** * Set the attempt future. This future represents a concrete retry attempt, potentially scheduled - * for execution in some form of {@link java.util.concurrent.ScheduledExecutorService}. + * for execution in a some form of {@link java.util.concurrent.ScheduledExecutorService}. * * @param attemptFuture the attempt future */ diff --git a/src/main/java/com/google/api/gax/retrying/RetryFutureImpl.java b/src/main/java/com/google/api/gax/retrying/RetryFutureImpl.java index 9c44b0bc1..23f6ecafc 100644 --- a/src/main/java/com/google/api/gax/retrying/RetryFutureImpl.java +++ b/src/main/java/com/google/api/gax/retrying/RetryFutureImpl.java @@ -42,11 +42,11 @@ * For internal use only. * * This class is the key component of the retry logic. It implements {@link RetryFuture} facade - * interface, and is doing the following: + * interface, and does the following: * *
    - *
  • Schedules next attempt in case of failure using callback chaining technique.
    • - *
  • Terminates retrying process if more retries are not accepted.
    • + *
  • Schedules next attempt in case of a failure using callback chaining technique.
    • + *
  • Terminates retrying process if no more retries are accepted.
    • *
  • Propagates future cancellation in both directions (from this to the attempt and from the * attempt to this)
    • *
@@ -115,9 +115,9 @@ public void setAttemptFuture(Future attemptFuture) { } } - private void scheduleRetry(Throwable delegateThrowable, Future prevProxiedFuture) { + private void executeAttempt(Throwable delegateThrowable, Future prevAttemptFuture) { try { - if (prevProxiedFuture.isCancelled()) { + if (prevAttemptFuture.isCancelled()) { cancel(false); } if (isDone()) { @@ -166,7 +166,7 @@ public void onFailure(Throwable t) { synchronized (lock) { if (this == callbackFutureCallback && !isDone()) { setAttemptFuture(null); - scheduleRetry(t, this.attemptFuture); + executeAttempt(t, this.attemptFuture); } } } diff --git a/src/main/java/com/google/api/gax/retrying/RetryHandler.java b/src/main/java/com/google/api/gax/retrying/RetryHandler.java index 8ea6037f2..659256179 100644 --- a/src/main/java/com/google/api/gax/retrying/RetryHandler.java +++ b/src/main/java/com/google/api/gax/retrying/RetryHandler.java @@ -36,14 +36,14 @@ /** * A retry handler is responsible for the following operations: *
    - *
  1. Accepting or rejecting a task for retry, depending on the previous attempt result (exception) + *
  2. Accepting or rejecting a task for retry depending on the previous attempt result (exception) * and/or other attempt properties (like number of already executed attempts or total time spent * retrying).
    3. * *
  3. Creating first attempt {@link RetryFuture}, which acts as a facade, hiding from client code - * the actual scheduled retry task.
    4. + * the actual scheduled retry attempts execution. * - *
  4. Creating {@link RetryAttemptSettings} for each subsequnt retry attempt.
    5. + *
  5. Creating {@link RetryAttemptSettings} for each subsequent retry attempt.
    6. * *
  6. Executing the actual {@link Callable} in a retriable context.
    7. * @@ -56,7 +56,7 @@ public interface RetryHandler { /** - * Returns {@code true} if another attempt should be made, or {@code false} otherwise + * Returns {@code true} if another attempt should be made, or {@code false} otherwise. * * @param e exception thrown by the previous attempt * @param nextAttemptSettings attempt settings, which will be used for the next attempt, if diff --git a/src/main/java/com/google/api/gax/retrying/ScheduledRetryHandler.java b/src/main/java/com/google/api/gax/retrying/ScheduledRetryHandler.java index 07997e95b..3a56fefe4 100644 --- a/src/main/java/com/google/api/gax/retrying/ScheduledRetryHandler.java +++ b/src/main/java/com/google/api/gax/retrying/ScheduledRetryHandler.java @@ -52,8 +52,8 @@ public class ScheduledRetryHandler extends AbstractRetryHandler Date: Tue, 14 Mar 2017 00:10:04 -0700 Subject: [PATCH 05/15] Addressed review comments, PTAL. --- .../google/api/gax/core/RetrySettings.java | 6 +++--- .../internal/ApiFutureToListenableFuture.java | 2 +- .../google/api/gax/grpc/RetryingCallable.java | 9 +++++---- .../gax/retrying/AbstractRetryHandler.java | 2 +- .../api/gax/retrying/DirectRetryHandler.java | 4 ++-- .../google/api/gax/retrying/RetryFuture.java | 2 +- .../api/gax/retrying/RetryFutureImpl.java | 20 ++++++++++++------- .../google/api/gax/retrying/RetryHandler.java | 2 +- .../gax/retrying/ScheduledRetryHandler.java | 9 ++++----- .../api/gax/{grpc => core}/FakeNanoClock.java | 5 ++--- .../google/api/gax/grpc/CancellationTest.java | 17 ++++++++++------ .../api/gax/grpc/RecordingScheduler.java | 3 ++- .../api/gax/grpc/UnaryCallableTest.java | 3 ++- .../api/gax/retrying/FailingCallable.java | 2 +- 14 files changed, 49 insertions(+), 37 deletions(-) rename src/test/java/com/google/api/gax/{grpc => core}/FakeNanoClock.java (94%) diff --git a/src/main/java/com/google/api/gax/core/RetrySettings.java b/src/main/java/com/google/api/gax/core/RetrySettings.java index 09415e0ce..25eb14ecf 100644 --- a/src/main/java/com/google/api/gax/core/RetrySettings.java +++ b/src/main/java/com/google/api/gax/core/RetrySettings.java @@ -95,9 +95,9 @@ public abstract class RetrySettings implements Serializable { public abstract Duration getMaxRetryDelay(); /** - * MaxAttempts defines the maximum number of attempts to perform. If number of attempts reaches - * this limit the logic will give up retrying even if the total retry time is still lower than - * TotalTimeout. + * MaxAttempts defines the maximum number of attempts to perform. The default value is 0. If this + * value is greater than 0, and the number of attempts reaches this limit, the logic will give up + * retrying even if the total retry time is still lower than TotalTimeout. */ public abstract int getMaxAttempts(); diff --git a/src/main/java/com/google/api/gax/core/internal/ApiFutureToListenableFuture.java b/src/main/java/com/google/api/gax/core/internal/ApiFutureToListenableFuture.java index 88e07d254..dd9b29478 100644 --- a/src/main/java/com/google/api/gax/core/internal/ApiFutureToListenableFuture.java +++ b/src/main/java/com/google/api/gax/core/internal/ApiFutureToListenableFuture.java @@ -42,7 +42,7 @@ */ @ExperimentalApi public class ApiFutureToListenableFuture implements ListenableFuture { - private ApiFuture apiFuture; + private final ApiFuture apiFuture; public ApiFutureToListenableFuture(ApiFuture apiFuture) { this.apiFuture = apiFuture; diff --git a/src/main/java/com/google/api/gax/grpc/RetryingCallable.java b/src/main/java/com/google/api/gax/grpc/RetryingCallable.java index 72fe3e3c7..374f95450 100644 --- a/src/main/java/com/google/api/gax/grpc/RetryingCallable.java +++ b/src/main/java/com/google/api/gax/grpc/RetryingCallable.java @@ -73,11 +73,12 @@ class RetryingCallable implements FutureCallable futureCall(RequestT request, CallContext context) { GrpcRetryCallable retryCallable = new GrpcRetryCallable<>(callable, request, context); - GrpcRetryHandler retryHelper = new GrpcRetryHandler<>(clock, scheduler); - RetryFuture rv = retryHelper.createFirstAttempt(retryCallable, retryParams); - retryCallable.setExternalFuture(rv); + GrpcRetryHandler retryHandler = new GrpcRetryHandler<>(clock, scheduler); + RetryFuture retryFuture = + retryHandler.createFirstAttempt(retryCallable, retryParams); + retryCallable.setExternalFuture(retryFuture); retryCallable.call(); - return rv; + return retryFuture; } @Override diff --git a/src/main/java/com/google/api/gax/retrying/AbstractRetryHandler.java b/src/main/java/com/google/api/gax/retrying/AbstractRetryHandler.java index d9310f579..307dac193 100644 --- a/src/main/java/com/google/api/gax/retrying/AbstractRetryHandler.java +++ b/src/main/java/com/google/api/gax/retrying/AbstractRetryHandler.java @@ -80,7 +80,7 @@ public boolean accept(Throwable e, RetryAttemptSettings nextAttemptSettings) { } /** - * Crates next attempt settings. It increments the current attempt count and uses randomized + * Creates next attempt settings. It increments the current attempt count and uses randomized * exponential backoff factor for calculating next attempt execution time. * * @param e exception thrown by the previous attempt diff --git a/src/main/java/com/google/api/gax/retrying/DirectRetryHandler.java b/src/main/java/com/google/api/gax/retrying/DirectRetryHandler.java index 093a4ac3c..2b0634a65 100644 --- a/src/main/java/com/google/api/gax/retrying/DirectRetryHandler.java +++ b/src/main/java/com/google/api/gax/retrying/DirectRetryHandler.java @@ -38,8 +38,8 @@ import org.joda.time.Duration; /** - * The retry handler, which executes attempts in the current thread, potentially causing the current - * thread to sleep for a specified amount of type before execution. + * The retry handler which executes attempts in the current thread, potentially causing the current + * thread to sleep for a specified amount of time before execution. * * This class is thread-safe. * diff --git a/src/main/java/com/google/api/gax/retrying/RetryFuture.java b/src/main/java/com/google/api/gax/retrying/RetryFuture.java index 7c4a0260b..7ae748f85 100644 --- a/src/main/java/com/google/api/gax/retrying/RetryFuture.java +++ b/src/main/java/com/google/api/gax/retrying/RetryFuture.java @@ -43,7 +43,7 @@ public interface RetryFuture extends ApiFuture { /** - * Set the attempt future. This future represents a concrete retry attempt, potentially scheduled + * Sets the attempt future. This future represents a concrete retry attempt, potentially scheduled * for execution in a some form of {@link java.util.concurrent.ScheduledExecutorService}. * * @param attemptFuture the attempt future diff --git a/src/main/java/com/google/api/gax/retrying/RetryFutureImpl.java b/src/main/java/com/google/api/gax/retrying/RetryFutureImpl.java index 23f6ecafc..0abe15e6f 100644 --- a/src/main/java/com/google/api/gax/retrying/RetryFutureImpl.java +++ b/src/main/java/com/google/api/gax/retrying/RetryFutureImpl.java @@ -41,11 +41,11 @@ /** * For internal use only. * - * This class is the key component of the retry logic. It implements {@link RetryFuture} facade + * This class is the key component of the retry logic. It implements the {@link RetryFuture} facade * interface, and does the following: * *
      - *
    • Schedules next attempt in case of a failure using callback chaining technique.
      • + *
    • Schedules the next attempt in case of a failure using the callback chaining technique.
      • *
    • Terminates retrying process if no more retries are accepted.
      • *
    • Propagates future cancellation in both directions (from this to the attempt and from the * attempt to this)
      • @@ -74,22 +74,28 @@ class RetryFutureImpl extends AbstractFuture @Override protected boolean set(ResponseT value) { - return super.set(value); + synchronized (lock) { + return super.set(value); + } } @Override protected boolean setException(Throwable throwable) { - return super.setException(throwable); + synchronized (lock) { + return super.setException(throwable); + } } @Override public boolean cancel(boolean mayInterruptIfRunning) { synchronized (lock) { - boolean rv = super.cancel(mayInterruptIfRunning); if (callbackFutureCallback != null) { - callbackFutureCallback.attemptFuture.cancel(mayInterruptIfRunning); + boolean rv = callbackFutureCallback.attemptFuture.cancel(mayInterruptIfRunning); + super.cancel(mayInterruptIfRunning); + return rv; + } else { + return super.cancel(mayInterruptIfRunning); } - return rv; } } diff --git a/src/main/java/com/google/api/gax/retrying/RetryHandler.java b/src/main/java/com/google/api/gax/retrying/RetryHandler.java index 659256179..a9a7d86dd 100644 --- a/src/main/java/com/google/api/gax/retrying/RetryHandler.java +++ b/src/main/java/com/google/api/gax/retrying/RetryHandler.java @@ -77,7 +77,7 @@ RetryFuture createFirstAttempt( Callable callable, RetrySettings globalSettings); /** - * Creates next attempt {@link RetryAttemptSettings}, which defines properties of the next + * Creates the next attempt {@link RetryAttemptSettings}, which defines properties of the next * attempt. Eventually this object will be passed to * {@link RetryHandler#accept(Throwable, RetryAttemptSettings)} and * {@link RetryHandler#executeAttempt(Callable, RetryAttemptSettings)}. diff --git a/src/main/java/com/google/api/gax/retrying/ScheduledRetryHandler.java b/src/main/java/com/google/api/gax/retrying/ScheduledRetryHandler.java index 3a56fefe4..a63b06c81 100644 --- a/src/main/java/com/google/api/gax/retrying/ScheduledRetryHandler.java +++ b/src/main/java/com/google/api/gax/retrying/ScheduledRetryHandler.java @@ -40,15 +40,14 @@ import java.util.concurrent.TimeUnit; /** - * A retry handler which uses {@link ScheduledExecutorService} to schedule attempt tasks. Unless a - * direct executor service is used, this handler will schedule attempts for an execution in another - * thread. + * A retry handler which uses {@link ScheduledExecutorService} to schedule call attempt tasks. + * Unless a direct executor service is used, this handler will schedule attempts for execution in + * another thread. * * This class is thread-safe. - * - * @param */ public class ScheduledRetryHandler extends AbstractRetryHandler { + private final ListeningScheduledExecutorService scheduler; /** diff --git a/src/test/java/com/google/api/gax/grpc/FakeNanoClock.java b/src/test/java/com/google/api/gax/core/FakeNanoClock.java similarity index 94% rename from src/test/java/com/google/api/gax/grpc/FakeNanoClock.java rename to src/test/java/com/google/api/gax/core/FakeNanoClock.java index e1872a816..68394a241 100644 --- a/src/test/java/com/google/api/gax/grpc/FakeNanoClock.java +++ b/src/test/java/com/google/api/gax/core/FakeNanoClock.java @@ -27,12 +27,11 @@ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -package com.google.api.gax.grpc; +package com.google.api.gax.core; -import com.google.api.gax.core.NanoClock; import java.util.concurrent.TimeUnit; -class FakeNanoClock implements NanoClock { +public class FakeNanoClock implements NanoClock { private volatile long currentNanoTime; public FakeNanoClock(long initialNanoTime) { diff --git a/src/test/java/com/google/api/gax/grpc/CancellationTest.java b/src/test/java/com/google/api/gax/grpc/CancellationTest.java index d951805e6..a666c1124 100644 --- a/src/test/java/com/google/api/gax/grpc/CancellationTest.java +++ b/src/test/java/com/google/api/gax/grpc/CancellationTest.java @@ -35,6 +35,7 @@ import com.google.api.gax.core.AbstractApiFuture; import com.google.api.gax.core.ApiFuture; +import com.google.api.gax.core.FakeNanoClock; import com.google.api.gax.core.RetrySettings; import com.google.api.gax.core.SettableApiFuture; import com.google.common.collect.ImmutableSet; @@ -95,7 +96,7 @@ public class CancellationTest { @Before public void resetClock() { fakeClock = new FakeNanoClock(System.nanoTime()); - executor = RecordingScheduler.get(fakeClock); + executor = RecordingScheduler.create(fakeClock); } @After @@ -109,9 +110,9 @@ public void cancellationBeforeGetOnRetryingCallable() throws Exception { Mockito.when(callInt.futureCall((Integer) Mockito.any(), (CallContext) Mockito.any())) .thenReturn(SettableApiFuture.create()); - ImmutableSet retryable = ImmutableSet.of(Status.Code.UNAVAILABLE); + ImmutableSet retryable = ImmutableSet.of(Status.Code.UNAVAILABLE); UnaryCallable callable = - UnaryCallable.create(callInt) + UnaryCallable.create(callInt) .retryableOn(retryable) .retrying(FAST_RETRY_SETTINGS, executor, fakeClock); @@ -201,11 +202,11 @@ public void cancellationDuringFirstCall() throws Exception { CancellationTrackingFuture innerFuture = CancellationTrackingFuture.create(); CountDownLatch callIssuedLatch = new CountDownLatch(1); FutureCallable innerCallable = - new LatchCountDownFutureCallable(callIssuedLatch, innerFuture); + new LatchCountDownFutureCallable<>(callIssuedLatch, innerFuture); - ImmutableSet retryable = ImmutableSet.of(Status.Code.UNAVAILABLE); + ImmutableSet retryable = ImmutableSet.of(Status.Code.UNAVAILABLE); UnaryCallable callable = - UnaryCallable.create(innerCallable) + UnaryCallable.create(innerCallable) .retryableOn(retryable) .retrying(FAST_RETRY_SETTINGS, new ScheduledThreadPoolExecutor(1), fakeClock); @@ -246,6 +247,8 @@ public void cancellationDuringRetryDelay() throws Exception { gotException = e; } Truth.assertThat(gotException).isNotNull(); + Truth.assertThat(resultFuture.isDone()).isTrue(); + Truth.assertThat(resultFuture.isCancelled()).isTrue(); Truth.assertThat(innerFuture.isCancelled()).isFalse(); } @@ -275,6 +278,8 @@ public void cancellationDuringSecondCall() throws Exception { gotException = e; } Truth.assertThat(gotException).isNotNull(); + Truth.assertThat(resultFuture.isDone()).isTrue(); + Truth.assertThat(resultFuture.isCancelled()).isTrue(); Truth.assertThat(innerFuture.isCancelled()).isTrue(); } } diff --git a/src/test/java/com/google/api/gax/grpc/RecordingScheduler.java b/src/test/java/com/google/api/gax/grpc/RecordingScheduler.java index 01a3f6e16..c618727a4 100644 --- a/src/test/java/com/google/api/gax/grpc/RecordingScheduler.java +++ b/src/test/java/com/google/api/gax/grpc/RecordingScheduler.java @@ -33,6 +33,7 @@ import static org.mockito.Matchers.anyLong; import static org.mockito.Mockito.when; +import com.google.api.gax.core.FakeNanoClock; import java.util.ArrayList; import java.util.List; import java.util.concurrent.ScheduledExecutorService; @@ -48,7 +49,7 @@ abstract class RecordingScheduler implements ScheduledExecutorService { abstract List getSleepDurations(); - static RecordingScheduler get(final FakeNanoClock clock) { + static RecordingScheduler create(final FakeNanoClock clock) { RecordingScheduler mock = Mockito.mock(RecordingScheduler.class); // mock class fields: diff --git a/src/test/java/com/google/api/gax/grpc/UnaryCallableTest.java b/src/test/java/com/google/api/gax/grpc/UnaryCallableTest.java index 2b0dffe5d..fc0616da3 100644 --- a/src/test/java/com/google/api/gax/grpc/UnaryCallableTest.java +++ b/src/test/java/com/google/api/gax/grpc/UnaryCallableTest.java @@ -33,6 +33,7 @@ import com.google.api.gax.bundling.RequestBuilder; import com.google.api.gax.core.ApiFuture; import com.google.api.gax.core.ApiFutures; +import com.google.api.gax.core.FakeNanoClock; import com.google.api.gax.core.FixedSizeCollection; import com.google.api.gax.core.Page; import com.google.api.gax.core.RetrySettings; @@ -92,7 +93,7 @@ static ApiFuture immediateFailedFuture(Throwable t) { @Before public void resetClock() { fakeClock = new FakeNanoClock(System.nanoTime()); - executor = RecordingScheduler.get(fakeClock); + executor = RecordingScheduler.create(fakeClock); } @After diff --git a/src/test/java/com/google/api/gax/retrying/FailingCallable.java b/src/test/java/com/google/api/gax/retrying/FailingCallable.java index 68a3ee5f8..6bfab7a2d 100644 --- a/src/test/java/com/google/api/gax/retrying/FailingCallable.java +++ b/src/test/java/com/google/api/gax/retrying/FailingCallable.java @@ -44,7 +44,7 @@ class FailingCallable implements Callable { .setInitialRpcTimeout(Duration.millis(2L)) .setRpcTimeoutMultiplier(1) .setMaxRpcTimeout(Duration.millis(2L)) - .setTotalTimeout(Duration.millis(10L)) + .setTotalTimeout(Duration.millis(50L)) .build(); private AtomicInteger attemptsCount = new AtomicInteger(0); From eeb49b22815972707291702ea3968c0b5c40a2a2 Mon Sep 17 00:00:00 2001 From: vam Date: Tue, 14 Mar 2017 00:14:51 -0700 Subject: [PATCH 06/15] reverted versions back to 0.3.1-SNAPSHOT --- version.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.txt b/version.txt index cf2499627..66ec8b17f 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -0.3.2-SNAPSHOT +0.3.1-SNAPSHOT From 8da0218774cbcf9dd723c46d88a48f6face34c99 Mon Sep 17 00:00:00 2001 From: vam Date: Tue, 14 Mar 2017 12:55:30 -0700 Subject: [PATCH 07/15] renamed rv variable --- .../java/com/google/api/gax/retrying/RetryFutureImpl.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/google/api/gax/retrying/RetryFutureImpl.java b/src/main/java/com/google/api/gax/retrying/RetryFutureImpl.java index 0abe15e6f..72ce7c836 100644 --- a/src/main/java/com/google/api/gax/retrying/RetryFutureImpl.java +++ b/src/main/java/com/google/api/gax/retrying/RetryFutureImpl.java @@ -90,9 +90,9 @@ protected boolean setException(Throwable throwable) { public boolean cancel(boolean mayInterruptIfRunning) { synchronized (lock) { if (callbackFutureCallback != null) { - boolean rv = callbackFutureCallback.attemptFuture.cancel(mayInterruptIfRunning); + boolean canceled = callbackFutureCallback.attemptFuture.cancel(mayInterruptIfRunning); super.cancel(mayInterruptIfRunning); - return rv; + return canceled; } else { return super.cancel(mayInterruptIfRunning); } From ff0ff36abcaebf7e37c47f0529ab7ccea6e90230 Mon Sep 17 00:00:00 2001 From: vam Date: Thu, 16 Mar 2017 18:24:51 -0700 Subject: [PATCH 08/15] 1. Split RetryHandler on 3 different components: 1) scheduling logic, 2) time-specific and 3) exception-specific retry logics. 2. Removed abstract classes 3. Renamed NanoClock to ApiClock, as it has System.currentTimeMillis() implementation, and NanoClock name is misleading now. --- .../{DefaultNanoClock.java => ApiClock.java} | 44 +++---- .../com/google/api/gax/core/NanoClock.java | 44 ++++--- .../com/google/api/gax/core/SystemClock.java | 10 +- .../google/api/gax/grpc/RetryingCallable.java | 94 +++++++++------ .../google/api/gax/grpc/UnaryCallable.java | 9 +- ...ndler.java => DirectRetryingExecutor.java} | 57 ++++++--- .../gax/retrying/ExceptionRetryAlgorithm.java | 62 ++++++++++ ...er.java => ExponentialRetryAlgorithm.java} | 108 ++++++++--------- .../api/gax/retrying/RetryAlgorithm.java | 98 ++++++++++++++++ .../google/api/gax/retrying/RetryHandler.java | 101 ---------------- .../api/gax/retrying/RetryingExecutor.java | 65 +++++++++++ .../{RetryFuture.java => RetryingFuture.java} | 12 +- ...utureImpl.java => RetryingFutureImpl.java} | 110 +++++++++--------- ...er.java => ScheduledRetryingExecutor.java} | 55 ++++++--- ...ettings.java => TimedAttemptSettings.java} | 9 +- .../api/gax/retrying/TimedRetryAlgorithm.java | 72 ++++++++++++ .../{FakeNanoClock.java => FakeApiClock.java} | 4 +- .../api/gax/grpc/CancellationHelpers.java | 4 +- .../google/api/gax/grpc/CancellationTest.java | 8 +- .../api/gax/grpc/RecordingScheduler.java | 4 +- .../api/gax/grpc/UnaryCallableTest.java | 63 +++++++++- ...java => AbstractRetryingExecutorTest.java} | 57 +++++---- ...t.java => DirectRetryingExecutorTest.java} | 12 +- .../api/gax/retrying/FailingCallable.java | 2 +- ...ava => ScheduledRetryingExecutorTest.java} | 31 ++--- 25 files changed, 740 insertions(+), 395 deletions(-) rename src/main/java/com/google/api/gax/core/{DefaultNanoClock.java => ApiClock.java} (66%) rename src/main/java/com/google/api/gax/retrying/{DirectRetryHandler.java => DirectRetryingExecutor.java} (50%) create mode 100644 src/main/java/com/google/api/gax/retrying/ExceptionRetryAlgorithm.java rename src/main/java/com/google/api/gax/retrying/{AbstractRetryHandler.java => ExponentialRetryAlgorithm.java} (66%) create mode 100644 src/main/java/com/google/api/gax/retrying/RetryAlgorithm.java delete mode 100644 src/main/java/com/google/api/gax/retrying/RetryHandler.java create mode 100644 src/main/java/com/google/api/gax/retrying/RetryingExecutor.java rename src/main/java/com/google/api/gax/retrying/{RetryFuture.java => RetryingFuture.java} (84%) rename src/main/java/com/google/api/gax/retrying/{RetryFutureImpl.java => RetryingFutureImpl.java} (67%) rename src/main/java/com/google/api/gax/retrying/{ScheduledRetryHandler.java => ScheduledRetryingExecutor.java} (56%) rename src/main/java/com/google/api/gax/retrying/{RetryAttemptSettings.java => TimedAttemptSettings.java} (93%) create mode 100644 src/main/java/com/google/api/gax/retrying/TimedRetryAlgorithm.java rename src/test/java/com/google/api/gax/core/{FakeNanoClock.java => FakeApiClock.java} (95%) rename src/test/java/com/google/api/gax/retrying/{AbstractRetryHandlerTest.java => AbstractRetryingExecutorTest.java} (76%) rename src/test/java/com/google/api/gax/retrying/{DirectRetryHandlerTest.java => DirectRetryingExecutorTest.java} (78%) rename src/test/java/com/google/api/gax/retrying/{ScheduledRetryHandlerTest.java => ScheduledRetryingExecutorTest.java} (80%) diff --git a/src/main/java/com/google/api/gax/core/DefaultNanoClock.java b/src/main/java/com/google/api/gax/core/ApiClock.java similarity index 66% rename from src/main/java/com/google/api/gax/core/DefaultNanoClock.java rename to src/main/java/com/google/api/gax/core/ApiClock.java index 38adf29b2..90ccff877 100644 --- a/src/main/java/com/google/api/gax/core/DefaultNanoClock.java +++ b/src/main/java/com/google/api/gax/core/ApiClock.java @@ -29,33 +29,23 @@ */ package com.google.api.gax.core; -import java.io.ObjectStreamException; -import java.io.Serializable; -import java.util.concurrent.TimeUnit; - -/** Default implementation of the NanoClock interface, using call to System.nanoTime(). */ -public final class DefaultNanoClock implements NanoClock, Serializable { - - private static final NanoClock DEFAULT_CLOCK = new DefaultNanoClock(); - private static final long serialVersionUID = 5541462688633944865L; - - public static NanoClock getDefaultClock() { - return DEFAULT_CLOCK; - } - - private DefaultNanoClock() {} - - @Override - public final long nanoTime() { - return System.nanoTime(); - } +/** + * An interface for getting the current value of a high-resolution time source, in nanoseconds. + * + * Clocks other than NanoClock are typically used only for testing. + * + * This interface is required in addition to Java 8's Clock, because nanoTime is required to compare + * values with io.grpc.CallOptions.getDeadlineNanoTime(). + */ +public interface ApiClock { - @Override - public final long millisTime() { - return TimeUnit.MILLISECONDS.convert(nanoTime(), TimeUnit.NANOSECONDS); - } + /** + * Returns the current value of this clock's high-resolution time source, in nanoseconds. + */ + long nanoTime(); - private Object readResolve() throws ObjectStreamException { - return DEFAULT_CLOCK; - } + /** + * Returns the current value of this clock's high-resolution time source, in milliseconds. + */ + long millisTime(); } diff --git a/src/main/java/com/google/api/gax/core/NanoClock.java b/src/main/java/com/google/api/gax/core/NanoClock.java index e7f01fbaf..4de6f47c0 100644 --- a/src/main/java/com/google/api/gax/core/NanoClock.java +++ b/src/main/java/com/google/api/gax/core/NanoClock.java @@ -29,23 +29,33 @@ */ package com.google.api.gax.core; -/** - * An interface for getting the current value of a high-resolution time source, in nanoseconds. - * - * Clocks other than DefaultNanoClock are typically used only for testing. - * - * This interface is required in addition to Java 8's Clock, because nanoTime is required to compare - * values with io.grpc.CallOptions.getDeadlineNanoTime(). - */ -public interface NanoClock { +import java.io.ObjectStreamException; +import java.io.Serializable; +import java.util.concurrent.TimeUnit; + +/** Default implementation of the ApiClock interface, using call to System.nanoTime(). */ +public final class NanoClock implements ApiClock, Serializable { + + private static final ApiClock DEFAULT_CLOCK = new NanoClock(); + private static final long serialVersionUID = 5541462688633944865L; + + public static ApiClock getDefaultClock() { + return DEFAULT_CLOCK; + } + + private NanoClock() {} + + @Override + public final long nanoTime() { + return System.nanoTime(); + } - /** - * Returns the current value of this clock's high-resolution time source, in nanoseconds. - */ - long nanoTime(); + @Override + public final long millisTime() { + return TimeUnit.MILLISECONDS.convert(nanoTime(), TimeUnit.NANOSECONDS); + } - /** - * Returns the current value of this clock's high-resolution time source, in milliseconds. - */ - long millisTime(); + private Object readResolve() throws ObjectStreamException { + return DEFAULT_CLOCK; + } } diff --git a/src/main/java/com/google/api/gax/core/SystemClock.java b/src/main/java/com/google/api/gax/core/SystemClock.java index c57ea5069..5e27a15f0 100644 --- a/src/main/java/com/google/api/gax/core/SystemClock.java +++ b/src/main/java/com/google/api/gax/core/SystemClock.java @@ -34,12 +34,16 @@ import java.io.Serializable; import java.util.concurrent.TimeUnit; -public final class SystemClock implements NanoClock, Serializable { +/** + * Implementation of the {@link ApiClock} interface, which uses {@link System#currentTimeMillis()} + * as time source. + */ +public final class SystemClock implements ApiClock, Serializable { private static final long serialVersionUID = -6019259882852183285L; - private static final NanoClock DEFAULT_CLOCK = new SystemClock(); + private static final ApiClock DEFAULT_CLOCK = new SystemClock(); - public static NanoClock getDefaultClock() { + public static ApiClock getDefaultClock() { return DEFAULT_CLOCK; } diff --git a/src/main/java/com/google/api/gax/grpc/RetryingCallable.java b/src/main/java/com/google/api/gax/grpc/RetryingCallable.java index 374f95450..a38a927a3 100644 --- a/src/main/java/com/google/api/gax/grpc/RetryingCallable.java +++ b/src/main/java/com/google/api/gax/grpc/RetryingCallable.java @@ -30,13 +30,19 @@ package com.google.api.gax.grpc; import com.google.api.gax.core.ApiFuture; -import com.google.api.gax.core.NanoClock; +import com.google.api.gax.core.ApiClock; import com.google.api.gax.core.RetrySettings; import com.google.api.gax.core.internal.ApiFutureToListenableFuture; -import com.google.api.gax.retrying.RetryAttemptSettings; -import com.google.api.gax.retrying.RetryFuture; -import com.google.api.gax.retrying.ScheduledRetryHandler; +import com.google.api.gax.retrying.ExceptionRetryAlgorithm; +import com.google.api.gax.retrying.ExponentialRetryAlgorithm; +import com.google.api.gax.retrying.RetryAlgorithm; +import com.google.api.gax.retrying.RetryingExecutor; +import com.google.api.gax.retrying.RetryingFuture; +import com.google.api.gax.retrying.ScheduledRetryingExecutor; +import com.google.api.gax.retrying.TimedAttemptSettings; import com.google.common.base.Preconditions; +import com.google.common.util.concurrent.AbstractFuture; +import com.google.common.util.concurrent.Futures; import io.grpc.CallOptions; import io.grpc.Status.Code; import java.util.concurrent.Callable; @@ -47,6 +53,7 @@ /** * Implements the retry and timeout functionality used in {@link UnaryCallable}. * + *

      * The behavior is controlled by the given {@link RetrySettings}. */ class RetryingCallable implements FutureCallable { @@ -54,31 +61,30 @@ class RetryingCallable implements FutureCallable callable; - private final RetrySettings retryParams; - private final ScheduledExecutorService scheduler; - private final NanoClock clock; + private final RetryingExecutor scheduler; RetryingCallable( FutureCallable callable, RetrySettings retrySettings, ScheduledExecutorService scheduler, - NanoClock clock) { + ApiClock clock) { this.callable = Preconditions.checkNotNull(callable); - this.retryParams = Preconditions.checkNotNull(retrySettings); - this.scheduler = scheduler; - this.clock = clock; + RetryAlgorithm retryAlgorithm = + new RetryAlgorithm( + new GrpcExceptionRetryAlgorithm(), new ExponentialRetryAlgorithm(retrySettings, clock)); + this.scheduler = new ScheduledRetryingExecutor<>(retryAlgorithm, scheduler); } @Override public ApiFuture futureCall(RequestT request, CallContext context) { GrpcRetryCallable retryCallable = new GrpcRetryCallable<>(callable, request, context); - GrpcRetryHandler retryHandler = new GrpcRetryHandler<>(clock, scheduler); - RetryFuture retryFuture = - retryHandler.createFirstAttempt(retryCallable, retryParams); - retryCallable.setExternalFuture(retryFuture); + + RetryingFuture retryingFuture = scheduler.createFuture(retryCallable); + retryCallable.setExternalFuture(retryingFuture); retryCallable.call(); - return retryFuture; + + return retryingFuture; } @Override @@ -105,8 +111,8 @@ private static class GrpcRetryCallable implements Callable< private final FutureCallable callable; private final RequestT request; - private RetryFuture externalFuture; - private CallContext callContext; + private volatile RetryingFuture externalFuture; + private volatile CallContext callContext; private GrpcRetryCallable( FutureCallable callable, RequestT request, CallContext callContext) { @@ -115,7 +121,7 @@ private GrpcRetryCallable( this.callContext = callContext; } - private void setExternalFuture(RetryFuture externalFuture) { + private void setExternalFuture(RetryingFuture externalFuture) { this.externalFuture = externalFuture; } @@ -124,29 +130,29 @@ public ResponseT call() { callContext = getCallContextWithDeadlineAfter( callContext, externalFuture.getAttemptSettings().getRpcTimeout()); - ApiFuture internalFuture = callable.futureCall(request, callContext); - externalFuture.setAttemptFuture(new ApiFutureToListenableFuture<>(internalFuture)); - return null; - } - } - private static class GrpcRetryHandler extends ScheduledRetryHandler { - private GrpcRetryHandler(NanoClock clock, ScheduledExecutorService scheduler) { - super(clock, scheduler); - } + try { + externalFuture.setAttemptFuture(new NonCancelableFuture()); + if (externalFuture.isDone()) { + return null; + } + ApiFuture internalFuture = callable.futureCall(request, callContext); + externalFuture.setAttemptFuture(new ApiFutureToListenableFuture<>(internalFuture)); + } catch (Throwable e) { + externalFuture.setAttemptFuture(Futures.immediateFailedFuture(e)); + throw e; + } - @Override - public boolean accept(Throwable e, RetryAttemptSettings nextAttemptSettings) { - return super.accept(e, nextAttemptSettings) - && (e instanceof ApiException) - && ((ApiException) e).isRetryable(); + return null; } + } + private static class GrpcExceptionRetryAlgorithm implements ExceptionRetryAlgorithm { @Override - public RetryAttemptSettings createNextAttemptSettings( - Throwable e, RetryAttemptSettings prevSettings) { - if (((ApiException) e).getStatusCode() == Code.DEADLINE_EXCEEDED) { - return new RetryAttemptSettings( + public TimedAttemptSettings createNextAttempt( + Throwable prevThrowable, TimedAttemptSettings prevSettings) { + if (((ApiException) prevThrowable).getStatusCode() == Code.DEADLINE_EXCEEDED) { + return new TimedAttemptSettings( prevSettings.getGlobalSettings(), prevSettings.getRetryDelay(), prevSettings.getRpcTimeout(), @@ -154,8 +160,20 @@ public RetryAttemptSettings createNextAttemptSettings( prevSettings.getAttemptCount() + 1, prevSettings.getFirstAttemptStartTime()); } + return prevSettings; + } + + @Override + public boolean accept(Throwable prevThrowable) { + return (prevThrowable instanceof ApiException) + && ((ApiException) prevThrowable).isRetryable(); + } + } - return super.createNextAttemptSettings(e, prevSettings); + private static class NonCancelableFuture extends AbstractFuture { + @Override + public boolean cancel(boolean mayInterruptIfRunning) { + return false; } } } diff --git a/src/main/java/com/google/api/gax/grpc/UnaryCallable.java b/src/main/java/com/google/api/gax/grpc/UnaryCallable.java index 25202da4f..bc627f78d 100644 --- a/src/main/java/com/google/api/gax/grpc/UnaryCallable.java +++ b/src/main/java/com/google/api/gax/grpc/UnaryCallable.java @@ -29,8 +29,8 @@ */ package com.google.api.gax.grpc; +import com.google.api.gax.core.ApiClock; import com.google.api.gax.core.ApiFuture; -import com.google.api.gax.core.DefaultNanoClock; import com.google.api.gax.core.NanoClock; import com.google.api.gax.core.RetrySettings; import com.google.common.annotations.VisibleForTesting; @@ -39,10 +39,7 @@ import com.google.common.util.concurrent.UncheckedExecutionException; import io.grpc.Channel; import io.grpc.Status; -import java.util.List; import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ScheduledFuture; -import java.util.concurrent.TimeUnit; import javax.annotation.Nullable; /** @@ -305,7 +302,7 @@ UnaryCallable retryableOn(ImmutableSet retryab */ UnaryCallable retrying( RetrySettings retrySettings, ScheduledExecutorService executor) { - return retrying(retrySettings, executor, DefaultNanoClock.getDefaultClock()); + return retrying(retrySettings, executor, NanoClock.getDefaultClock()); } /** @@ -318,7 +315,7 @@ UnaryCallable retrying( */ @VisibleForTesting UnaryCallable retrying( - RetrySettings retrySettings, ScheduledExecutorService executor, NanoClock clock) { + RetrySettings retrySettings, ScheduledExecutorService executor, ApiClock clock) { return new UnaryCallable<>( new RetryingCallable<>(callable, retrySettings, executor, clock), channel, settings); } diff --git a/src/main/java/com/google/api/gax/retrying/DirectRetryHandler.java b/src/main/java/com/google/api/gax/retrying/DirectRetryingExecutor.java similarity index 50% rename from src/main/java/com/google/api/gax/retrying/DirectRetryHandler.java rename to src/main/java/com/google/api/gax/retrying/DirectRetryingExecutor.java index 2b0634a65..9f23bf78d 100644 --- a/src/main/java/com/google/api/gax/retrying/DirectRetryHandler.java +++ b/src/main/java/com/google/api/gax/retrying/DirectRetryingExecutor.java @@ -29,7 +29,8 @@ */ package com.google.api.gax.retrying; -import com.google.api.gax.core.NanoClock; +import static com.google.common.base.Preconditions.checkNotNull; + import com.google.common.util.concurrent.Futures; import java.io.InterruptedIOException; import java.nio.channels.ClosedByInterruptException; @@ -38,44 +39,62 @@ import org.joda.time.Duration; /** - * The retry handler which executes attempts in the current thread, potentially causing the current - * thread to sleep for a specified amount of time before execution. + * The retry executor which executes attempts in the current thread, potentially causing the current + * thread to sleep for the specified amount of time before execution. * + *

      * This class is thread-safe. * * @param response type */ -public class DirectRetryHandler extends AbstractRetryHandler { +public class DirectRetryingExecutor implements RetryingExecutor { + + private final RetryAlgorithm retryAlgorithm; + + /** + * Creates a new direct retrying executor instance, which will be using {@code retryAlgorithm} to + * determine retrying strategy. + * + * @param retryAlgorithm retry algorithm to use for attempts execution + * @throws NullPointerException if {@code retryAlgorithm} is null + */ + public DirectRetryingExecutor(RetryAlgorithm retryAlgorithm) { + this.retryAlgorithm = checkNotNull(retryAlgorithm); + } /** - * Creates a new default direct retry handler + * Creates a {@link RetryingFuture}, which is a facade, returned to the client code to wait for + * any retriable operation to complete. The future is bounded to {@code this} executor instance. * - * @param clock clock to use during execution scheduling + * @param callable the actual callable, which should be executed in a retriable context + * @return retrying future facade */ - public DirectRetryHandler(NanoClock clock) { - super(clock); + @Override + public RetryingFuture createFuture(Callable callable) { + return new RetryingFutureImpl<>(callable, retryAlgorithm, this); } /** - * Executes attempt in the current thread. Causes the current thread to sleep, if it is not the - * first attempt. + * Submits an attempt for execution in the current thread, causing the current thread to sleep for + * the specified by the {@link RetryingFuture#getAttemptSettings()} amount of time. * - * @param callable the actual callable to execute - * @param attemptSettings current attempt settings + * @param retryingFuture the future previously returned by {@link #createFuture(Callable)} */ @Override - public Future executeAttempt( - Callable callable, RetryAttemptSettings attemptSettings) { + public void submit(RetryingFuture retryingFuture) { + Future attemptFuture; try { - if (Duration.ZERO.compareTo(attemptSettings.getRandomizedRetryDelay()) < 0) { - Thread.sleep(attemptSettings.getRandomizedRetryDelay().getMillis()); + Duration delay = retryingFuture.getAttemptSettings().getRandomizedRetryDelay(); + if (Duration.ZERO.compareTo(delay) < 0) { + Thread.sleep(delay.getMillis()); } - return Futures.immediateFuture(callable.call()); + attemptFuture = Futures.immediateFuture(retryingFuture.getCallable().call()); } catch (InterruptedException | InterruptedIOException | ClosedByInterruptException e) { Thread.currentThread().interrupt(); - return Futures.immediateFailedFuture(e); + attemptFuture = Futures.immediateFailedFuture(e); } catch (Throwable e) { - return Futures.immediateFailedFuture(e); + attemptFuture = Futures.immediateFailedFuture(e); } + retryingFuture.setAttemptFuture(attemptFuture); } } diff --git a/src/main/java/com/google/api/gax/retrying/ExceptionRetryAlgorithm.java b/src/main/java/com/google/api/gax/retrying/ExceptionRetryAlgorithm.java new file mode 100644 index 000000000..f1806e681 --- /dev/null +++ b/src/main/java/com/google/api/gax/retrying/ExceptionRetryAlgorithm.java @@ -0,0 +1,62 @@ +/* + * Copyright 2017, Google Inc. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package com.google.api.gax.retrying; + +/** + * An exception retry algorithm is responsible for the following operations: + * + *

        + *
      1. Accepting or rejecting a task for retry depending on the exception thrown by the previous + * attempt. + *
      2. Creating {@link TimedAttemptSettings} for each subsequent retry attempt. + *
      + * + * Implementations of this interface must be thread-safe. + */ +public interface ExceptionRetryAlgorithm { + /** + * Creates a next attempt {@link TimedAttemptSettings}. + * + * @param prevThrowable exception thrown by the previous attempt + * @param prevSettings previous attempt settings + * @return next attempt settings or {@code prevSettings}, if the implementing algorithm does not + * provide specific settings for the next attempt + */ + TimedAttemptSettings createNextAttempt( + Throwable prevThrowable, TimedAttemptSettings prevSettings); + + /** + * Returns {@code true} if another attempt should be made, or {@code false} otherwise. + * + * @param prevThrowable exception thrown by the previous attempt + * @return {@code true} if another attempt should be made, or {@code false} otherwise + */ + boolean accept(Throwable prevThrowable); +} diff --git a/src/main/java/com/google/api/gax/retrying/AbstractRetryHandler.java b/src/main/java/com/google/api/gax/retrying/ExponentialRetryAlgorithm.java similarity index 66% rename from src/main/java/com/google/api/gax/retrying/AbstractRetryHandler.java rename to src/main/java/com/google/api/gax/retrying/ExponentialRetryAlgorithm.java index 307dac193..1db1429ba 100644 --- a/src/main/java/com/google/api/gax/retrying/AbstractRetryHandler.java +++ b/src/main/java/com/google/api/gax/retrying/ExponentialRetryAlgorithm.java @@ -29,67 +29,65 @@ */ package com.google.api.gax.retrying; -import com.google.api.gax.core.NanoClock; +import static com.google.common.base.Preconditions.checkNotNull; + +import com.google.api.gax.core.ApiClock; import com.google.api.gax.core.RetrySettings; -import java.util.concurrent.Callable; import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.TimeUnit; import org.joda.time.Duration; /** - * Basic implementation of the {@link RetryHandler} interface. It is responsible for defining and - * checking attempt's basic properties (execution time and count limits). - * - * This class is thread-safe, and all inheriting classes are required to be thread-safe. + * The timed retry algorithm which uses randomized exponential backoff factor for calculating the next + * attempt execution time. * - * @param response type + *

      + * This class is thread-safe. */ -public abstract class AbstractRetryHandler implements RetryHandler { +public class ExponentialRetryAlgorithm implements TimedRetryAlgorithm { - private final NanoClock clock; + private final RetrySettings globalSettings; + private final ApiClock clock; - protected AbstractRetryHandler(NanoClock clock) { - this.clock = clock; + /** + * Creates a new exponential retry algorithm instance. + * + * @param globalSettings global retry settings (attempt independent) + * @param clock clock to use for time-specific calculations + * @throws NullPointerException if either {@code globalSettings} or {@code clock} is null + */ + public ExponentialRetryAlgorithm(RetrySettings globalSettings, ApiClock clock) { + this.globalSettings = checkNotNull(globalSettings); + this.clock = checkNotNull(clock); } /** - * Ensures that the retry logic hasn't exceeded either maximum number of retries or the total - * execution timeout. + * Creates a first attempt {@link TimedAttemptSettings}. By default the first attempt is + * configured to be executed immediately. * - * @param e exception thrown by the previous attempt - * @param nextAttemptSettings attempt settings, which will be used for the next attempt, if - * accepted - * @return {@code true} if none of the retry limits are exceeded + * @return first attempt settings */ @Override - public boolean accept(Throwable e, RetryAttemptSettings nextAttemptSettings) { - RetrySettings globalSettings = nextAttemptSettings.getGlobalSettings(); - long randRetryDelayMillis = nextAttemptSettings.getRandomizedRetryDelay().getMillis(); - long totalTimeSpentNanos = - clock.nanoTime() - - nextAttemptSettings.getFirstAttemptStartTime() - + TimeUnit.NANOSECONDS.convert(randRetryDelayMillis, TimeUnit.MILLISECONDS); - - long totalTimeoutMillis = globalSettings.getTotalTimeout().getMillis(); - long totalTimeoutNanos = - TimeUnit.NANOSECONDS.convert(totalTimeoutMillis, TimeUnit.MILLISECONDS); - - return totalTimeSpentNanos <= totalTimeoutNanos - && (globalSettings.getMaxAttempts() <= 0 - || nextAttemptSettings.getAttemptCount() < globalSettings.getMaxAttempts()); + public TimedAttemptSettings createFirstAttempt() { + return new TimedAttemptSettings( + globalSettings, + Duration.ZERO, + globalSettings.getTotalTimeout(), + Duration.ZERO, + 0, + clock.nanoTime()); } /** - * Creates next attempt settings. It increments the current attempt count and uses randomized - * exponential backoff factor for calculating next attempt execution time. + * Creates a next attempt {@link TimedAttemptSettings}. The implementation increments the + * current attempt count and uses randomized exponential backoff factor for calculating next + * attempt execution time. * - * @param e exception thrown by the previous attempt * @param prevSettings previous attempt settings * @return next attempt settings */ @Override - public RetryAttemptSettings createNextAttemptSettings( - Throwable e, RetryAttemptSettings prevSettings) { + public TimedAttemptSettings createNextAttempt(TimedAttemptSettings prevSettings) { RetrySettings settings = prevSettings.getGlobalSettings(); long newRetryDelay = settings.getInitialRetryDelay().getMillis(); @@ -104,7 +102,7 @@ public RetryAttemptSettings createNextAttemptSettings( newRpcTimeout = Math.min(newRpcTimeout, settings.getMaxRpcTimeout().getMillis()); } - return new RetryAttemptSettings( + return new TimedAttemptSettings( prevSettings.getGlobalSettings(), Duration.millis(newRetryDelay), Duration.millis(newRpcTimeout), @@ -114,24 +112,28 @@ public RetryAttemptSettings createNextAttemptSettings( } /** - * Creates first attempt future. By default the first attempt is configured to be executed - * immediately. + * Returns {@code true} if another attempt should be made, or {@code false} otherwise. * - * @param callable the actual callable, which should be executed in a retriable context - * @param globalSettings global retry settings (attempt independent) + * @param nextAttemptSettings attempt settings, which will be used for the next attempt, if + * accepted + * @return {@code true} if {@code nextAttemptSettings} does not exceed either maxAttempts limit or + * totalTimeout limit, or {@code false} otherwise */ @Override - public RetryFuture createFirstAttempt( - Callable callable, RetrySettings globalSettings) { - RetryAttemptSettings firstAttemptSettings = - new RetryAttemptSettings( - globalSettings, - Duration.ZERO, - globalSettings.getTotalTimeout(), - Duration.ZERO, - 0, - clock.nanoTime()); + public boolean accept(TimedAttemptSettings nextAttemptSettings) { + RetrySettings globalSettings = nextAttemptSettings.getGlobalSettings(); + long randRetryDelayMillis = nextAttemptSettings.getRandomizedRetryDelay().getMillis(); + long totalTimeSpentNanos = + clock.nanoTime() + - nextAttemptSettings.getFirstAttemptStartTime() + + TimeUnit.NANOSECONDS.convert(randRetryDelayMillis, TimeUnit.MILLISECONDS); - return new RetryFutureImpl<>(callable, firstAttemptSettings, this); + long totalTimeoutMillis = globalSettings.getTotalTimeout().getMillis(); + long totalTimeoutNanos = + TimeUnit.NANOSECONDS.convert(totalTimeoutMillis, TimeUnit.MILLISECONDS); + + return totalTimeSpentNanos <= totalTimeoutNanos + && (globalSettings.getMaxAttempts() <= 0 + || nextAttemptSettings.getAttemptCount() < globalSettings.getMaxAttempts()); } } diff --git a/src/main/java/com/google/api/gax/retrying/RetryAlgorithm.java b/src/main/java/com/google/api/gax/retrying/RetryAlgorithm.java new file mode 100644 index 000000000..6d1807cf9 --- /dev/null +++ b/src/main/java/com/google/api/gax/retrying/RetryAlgorithm.java @@ -0,0 +1,98 @@ +/* + * Copyright 2017, Google Inc. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package com.google.api.gax.retrying; + +import static com.google.common.base.Preconditions.checkNotNull; + +/** + * The retry algorithm, which makes decision based on the thrown exception and execution time + * settings of the previous attempt. + * + * This class is thread-safe. + */ +public class RetryAlgorithm { + private final TimedRetryAlgorithm timedAlgorithm; + private final ExceptionRetryAlgorithm exceptionAlgorithm; + + /** + * Creates a new retry algorithm instance, which uses {@code exceptionAlgorithm} and + * {@code timedAlgorithm} to make a decision. {@code exceptionAlgorithm} has higher priority than + * the {@code timedAlgorithm}. + * + * @param timedAlgorithm timed algorithm to use + * @param exceptionAlgorithm exception algorithm to use + */ + public RetryAlgorithm( + ExceptionRetryAlgorithm exceptionAlgorithm, TimedRetryAlgorithm timedAlgorithm) { + this.timedAlgorithm = checkNotNull(timedAlgorithm); + this.exceptionAlgorithm = checkNotNull(exceptionAlgorithm); + } + + /** + * Creates a first attempt {@link TimedAttemptSettings}. + * + * @return first attempt settings + */ + public TimedAttemptSettings createFirstAttempt() { + return timedAlgorithm.createFirstAttempt(); + } + + /** + * Creates a next attempt {@link TimedAttemptSettings}. This method will return the + * exception-specific next attempt settings, if there are any, otherwise it will default to the + * time-specific settings. + * + * @param prevThrowable exception thrown by the previous attempt + * @param prevSettings previous attempt settings + * @return next attempt settings or {@code prevSettings}, if the implementing algorithm does not + * provide specific settings for the next attempt + */ + public TimedAttemptSettings createNextAttempt( + Throwable prevThrowable, TimedAttemptSettings prevSettings) { + TimedAttemptSettings newSettings = + exceptionAlgorithm.createNextAttempt(prevThrowable, prevSettings); + if (prevSettings.equals(newSettings)) { + newSettings = timedAlgorithm.createNextAttempt(prevSettings); + } + return newSettings; + } + + /** + * Returns {@code true} if another attempt should be made, or {@code false} otherwise. This method + * will return {@code true} only if both timed and exception algorithms return true. + * + * @param nextAttemptSettings attempt settings, which will be used for the next attempt, if + * accepted + * @return {@code true} if another attempt should be made, or {@code false} otherwise + */ + boolean accept(Throwable prevThrowable, TimedAttemptSettings nextAttemptSettings) { + return exceptionAlgorithm.accept(prevThrowable) && timedAlgorithm.accept(nextAttemptSettings); + } +} diff --git a/src/main/java/com/google/api/gax/retrying/RetryHandler.java b/src/main/java/com/google/api/gax/retrying/RetryHandler.java deleted file mode 100644 index a9a7d86dd..000000000 --- a/src/main/java/com/google/api/gax/retrying/RetryHandler.java +++ /dev/null @@ -1,101 +0,0 @@ -/* - * Copyright 2017, Google Inc. All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are - * met: - * - * * Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above - * copyright notice, this list of conditions and the following disclaimer - * in the documentation and/or other materials provided with the - * distribution. - * * Neither the name of Google Inc. nor the names of its - * contributors may be used to endorse or promote products derived from - * this software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS - * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT - * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR - * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT - * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, - * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT - * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, - * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY - * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE - * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - */ -package com.google.api.gax.retrying; - -import com.google.api.gax.core.RetrySettings; -import java.util.concurrent.Callable; -import java.util.concurrent.Future; - -/** - * A retry handler is responsible for the following operations: - *

        - *
      1. Accepting or rejecting a task for retry depending on the previous attempt result (exception) - * and/or other attempt properties (like number of already executed attempts or total time spent - * retrying).
        2. - * - *
      2. Creating first attempt {@link RetryFuture}, which acts as a facade, hiding from client code - * the actual scheduled retry attempts execution.
        3. - * - *
      3. Creating {@link RetryAttemptSettings} for each subsequent retry attempt.
        4. - * - *
      4. Executing the actual {@link Callable} in a retriable context.
        5. - * - *
      - * - * This interface is for internal/advanced use only. - * - * @param response type - */ -public interface RetryHandler { - - /** - * Returns {@code true} if another attempt should be made, or {@code false} otherwise. - * - * @param e exception thrown by the previous attempt - * @param nextAttemptSettings attempt settings, which will be used for the next attempt, if - * accepted - * @return {@code true} if another attempt should be made, or {@code false} otherwise - */ - boolean accept(Throwable e, RetryAttemptSettings nextAttemptSettings); - - /** - * Creates a first try {@link RetryFuture}, which is a facade, returned to the client code to wait - * for any retriable operation to complete. - * - * @param callable the actual callable, which should be executed in a retriable context - * @param globalSettings global retry settings (attempt independent) - * @return retriable future facade - */ - RetryFuture createFirstAttempt( - Callable callable, RetrySettings globalSettings); - - /** - * Creates the next attempt {@link RetryAttemptSettings}, which defines properties of the next - * attempt. Eventually this object will be passed to - * {@link RetryHandler#accept(Throwable, RetryAttemptSettings)} and - * {@link RetryHandler#executeAttempt(Callable, RetryAttemptSettings)}. - * - * @param e exception thrown by the previous attempt - * @param prevSettings previous attempt settings - * @return next attempt settings - */ - RetryAttemptSettings createNextAttemptSettings(Throwable e, RetryAttemptSettings prevSettings); - - /** - * Executes an attempt. A typical implementation will either try to execute in the current thread - * or schedule it for an execution, using some sort of async execution service. - * - * @param callable the actual callable to execute - * @param attemptSettings current attempt settings - * @return the {@link Future}, representing the scheduled execution - */ - Future executeAttempt( - Callable callable, RetryAttemptSettings attemptSettings); -} diff --git a/src/main/java/com/google/api/gax/retrying/RetryingExecutor.java b/src/main/java/com/google/api/gax/retrying/RetryingExecutor.java new file mode 100644 index 000000000..31be02c70 --- /dev/null +++ b/src/main/java/com/google/api/gax/retrying/RetryingExecutor.java @@ -0,0 +1,65 @@ +/* + * Copyright 2017, Google Inc. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package com.google.api.gax.retrying; + +import java.util.concurrent.Callable; + +/** + * A retrying executor is responsible for the following operations: + * + *
        + *
      1. Creating first attempt {@link RetryingFuture}, which acts as a facade, hiding from client + * code the actual scheduled retry attempts execution. + *
      2. Executing the actual {@link Callable} in a retriable context. + *
      + * + * This interface is for internal/advanced use only. + * + * @param response type + */ +public interface RetryingExecutor { + /** + * Creates the {@link RetryingFuture}, which is a facade, returned to the client code to wait for + * any retriable operation to complete. + * + * @param callable the actual callable, which should be executed in a retriable context + * @return retrying future facade + */ + RetryingFuture createFuture(Callable callable); + + /** + * Submits an attempt for execution. A typical implementation will either try to execute the + * attempt in the current thread or schedule it for an execution, using some sort of async + * execution service. + * + * @param retryingFuture the future previously returned by {@link #createFuture(Callable)} + */ + void submit(RetryingFuture retryingFuture); +} diff --git a/src/main/java/com/google/api/gax/retrying/RetryFuture.java b/src/main/java/com/google/api/gax/retrying/RetryingFuture.java similarity index 84% rename from src/main/java/com/google/api/gax/retrying/RetryFuture.java rename to src/main/java/com/google/api/gax/retrying/RetryingFuture.java index 7ae748f85..1f2721bc9 100644 --- a/src/main/java/com/google/api/gax/retrying/RetryFuture.java +++ b/src/main/java/com/google/api/gax/retrying/RetryingFuture.java @@ -30,17 +30,18 @@ package com.google.api.gax.retrying; import com.google.api.gax.core.ApiFuture; +import java.util.concurrent.Callable; import java.util.concurrent.Future; /** - * Represents retriable future. This is a facade hiding all the complications of an asynchronous + * Represents a retrying future. This is a facade hiding all the complications of an asynchronous * execution of a retriable task. * * This interface is for advanced/internal use only. * * @param response type */ -public interface RetryFuture extends ApiFuture { +public interface RetryingFuture extends ApiFuture { /** * Sets the attempt future. This future represents a concrete retry attempt, potentially scheduled @@ -50,8 +51,11 @@ public interface RetryFuture extends ApiFuture { */ void setAttemptFuture(Future attemptFuture); + /** Returns current (active) attempt settings. */ + TimedAttemptSettings getAttemptSettings(); + /** - * Returns current (active) attempt settings. + * Returns callable tracked by this future. */ - RetryAttemptSettings getAttemptSettings(); + Callable getCallable(); } diff --git a/src/main/java/com/google/api/gax/retrying/RetryFutureImpl.java b/src/main/java/com/google/api/gax/retrying/RetryingFutureImpl.java similarity index 67% rename from src/main/java/com/google/api/gax/retrying/RetryFutureImpl.java rename to src/main/java/com/google/api/gax/retrying/RetryingFutureImpl.java index 72ce7c836..76bdf6166 100644 --- a/src/main/java/com/google/api/gax/retrying/RetryFutureImpl.java +++ b/src/main/java/com/google/api/gax/retrying/RetryingFutureImpl.java @@ -30,6 +30,8 @@ package com.google.api.gax.retrying; +import static com.google.common.base.Preconditions.checkNotNull; + import com.google.api.gax.core.ApiFutureCallback; import com.google.common.util.concurrent.AbstractFuture; import com.google.common.util.concurrent.FutureCallback; @@ -41,86 +43,89 @@ /** * For internal use only. * - * This class is the key component of the retry logic. It implements the {@link RetryFuture} facade - * interface, and does the following: + *

      + * This class is the key component of the retry logic. It implements the {@link RetryingFuture} + * facade interface, and does the following: * *

        - *
      • Schedules the next attempt in case of a failure using the callback chaining technique.
        • - *
      • Terminates retrying process if no more retries are accepted.
        • + *
      • Schedules the next attempt in case of a failure using the callback chaining technique. + *
      • Terminates retrying process if no more retries are accepted. *
      • Propagates future cancellation in both directions (from this to the attempt and from the - * attempt to this)
        • + * attempt to this) *
      * * This class is thread-safe. */ -class RetryFutureImpl extends AbstractFuture - implements RetryFuture { +class RetryingFutureImpl extends AbstractFuture + implements RetryingFuture { private final Object lock = new Object(); private final Callable callable; - private final RetryHandler retryHandler; - private volatile RetryAttemptSettings attemptSettings; - private volatile AttemptFutureCallback callbackFutureCallback; + private final RetryAlgorithm retryAlgorithm; + private final RetryingExecutor retryingExecutor; - RetryFutureImpl( - Callable callable, - RetryAttemptSettings attemptSettings, - RetryHandler retryHandler) { - this.callable = callable; - this.attemptSettings = attemptSettings; - this.retryHandler = retryHandler; - } + private volatile TimedAttemptSettings attemptSettings; + private volatile AttemptFutureCallback attemptFutureCallback; - @Override - protected boolean set(ResponseT value) { - synchronized (lock) { - return super.set(value); - } - } + RetryingFutureImpl( + Callable callable, + RetryAlgorithm retryAlgorithm, + RetryingExecutor retryingExecutor) { + this.callable = checkNotNull(callable); + this.retryAlgorithm = checkNotNull(retryAlgorithm); + this.retryingExecutor = checkNotNull(retryingExecutor); - @Override - protected boolean setException(Throwable throwable) { - synchronized (lock) { - return super.setException(throwable); - } + this.attemptSettings = retryAlgorithm.createFirstAttempt(); } @Override public boolean cancel(boolean mayInterruptIfRunning) { synchronized (lock) { - if (callbackFutureCallback != null) { - boolean canceled = callbackFutureCallback.attemptFuture.cancel(mayInterruptIfRunning); - super.cancel(mayInterruptIfRunning); - return canceled; + if (attemptFutureCallback != null) { + if (attemptFutureCallback.attemptFuture.cancel(mayInterruptIfRunning)) { + super.cancel(mayInterruptIfRunning); + } + return isCancelled(); } else { return super.cancel(mayInterruptIfRunning); } } } - @Override - public RetryAttemptSettings getAttemptSettings() { - synchronized (lock) { - return attemptSettings; - } - } - @Override public void setAttemptFuture(Future attemptFuture) { + if (isDone()) { + return; + } synchronized (lock) { + if (isDone()) { + return; + } if (attemptFuture != null) { - callbackFutureCallback = new AttemptFutureCallback(attemptFuture); - Futures.addCallback((ListenableFuture) attemptFuture, callbackFutureCallback); + attemptFutureCallback = new AttemptFutureCallback(attemptFuture); + Futures.addCallback((ListenableFuture) attemptFuture, attemptFutureCallback); if (isCancelled()) { attemptFuture.cancel(false); } } else { - callbackFutureCallback = null; + attemptFutureCallback = null; } } } + @Override + public TimedAttemptSettings getAttemptSettings() { + synchronized (lock) { + return attemptSettings; + } + } + + @Override + public Callable getCallable() { + return callable; + } + private void executeAttempt(Throwable delegateThrowable, Future prevAttemptFuture) { try { if (prevAttemptFuture.isCancelled()) { @@ -129,14 +134,11 @@ private void executeAttempt(Throwable delegateThrowable, Future prevA if (isDone()) { return; } - - RetryAttemptSettings nextAttemptSettings = - retryHandler.createNextAttemptSettings(delegateThrowable, attemptSettings); - if (retryHandler.accept(delegateThrowable, nextAttemptSettings)) { + TimedAttemptSettings nextAttemptSettings = + retryAlgorithm.createNextAttempt(delegateThrowable, attemptSettings); + if (retryAlgorithm.accept(delegateThrowable, nextAttemptSettings)) { attemptSettings = nextAttemptSettings; - Future nextInternalFuture = - retryHandler.executeAttempt(callable, attemptSettings); - setAttemptFuture(nextInternalFuture); + retryingExecutor.submit(this); } else { setException(delegateThrowable); } @@ -156,9 +158,9 @@ private AttemptFutureCallback(Future attemptFuture) { @Override public void onSuccess(ResponseT result) { - if (this == callbackFutureCallback && !isDone()) { + if (this == attemptFutureCallback && !isDone()) { synchronized (lock) { - if (this == callbackFutureCallback && !isDone()) { + if (this == attemptFutureCallback && !isDone()) { setAttemptFuture(null); set(result); } @@ -168,9 +170,9 @@ public void onSuccess(ResponseT result) { @Override public void onFailure(Throwable t) { - if (this == callbackFutureCallback && !isDone()) { + if (this == attemptFutureCallback && !isDone()) { synchronized (lock) { - if (this == callbackFutureCallback && !isDone()) { + if (this == attemptFutureCallback && !isDone()) { setAttemptFuture(null); executeAttempt(t, this.attemptFuture); } diff --git a/src/main/java/com/google/api/gax/retrying/ScheduledRetryHandler.java b/src/main/java/com/google/api/gax/retrying/ScheduledRetryingExecutor.java similarity index 56% rename from src/main/java/com/google/api/gax/retrying/ScheduledRetryHandler.java rename to src/main/java/com/google/api/gax/retrying/ScheduledRetryingExecutor.java index a63b06c81..b2f5e560f 100644 --- a/src/main/java/com/google/api/gax/retrying/ScheduledRetryHandler.java +++ b/src/main/java/com/google/api/gax/retrying/ScheduledRetryingExecutor.java @@ -29,7 +29,6 @@ */ package com.google.api.gax.retrying; -import com.google.api.gax.core.NanoClock; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListeningScheduledExecutorService; import com.google.common.util.concurrent.MoreExecutors; @@ -40,43 +39,61 @@ import java.util.concurrent.TimeUnit; /** - * A retry handler which uses {@link ScheduledExecutorService} to schedule call attempt tasks. - * Unless a direct executor service is used, this handler will schedule attempts for execution in + * The retry executor which uses {@link ScheduledExecutorService} to schedule an attempt tasks. + * Unless a direct executor service is used, this handler will schedule attempts for an execution in * another thread. * - * This class is thread-safe. + *

      This class is thread-safe. + * + * @param response type */ -public class ScheduledRetryHandler extends AbstractRetryHandler { +public class ScheduledRetryingExecutor implements RetryingExecutor { + private final RetryAlgorithm retryAlgorithm; private final ListeningScheduledExecutorService scheduler; /** - * Creates a new scheduled retry handler, which will be using {@link ScheduledExecutorService} for - * actual attempts scheduling. + * Creates a new scheduled retry executor, which will be using {@code scheduler} + * for actual attempts scheduling and {@code retryAlgorithm} for retrying strategy. * - * @param clock clock to use for scheduling operations + * @param retryAlgorithm retry algorithm to use * @param scheduler scheduler */ - public ScheduledRetryHandler(NanoClock clock, ScheduledExecutorService scheduler) { - super(clock); + public ScheduledRetryingExecutor( + RetryAlgorithm retryAlgorithm, ScheduledExecutorService scheduler) { + this.retryAlgorithm = retryAlgorithm; this.scheduler = MoreExecutors.listeningDecorator(scheduler); } /** - * Executes attempt using previously provided scheduler. + * Creates a {@link RetryingFuture}, which is a facade, returned to the client code to wait for + * any retriable operation to complete. The returned future is bounded to {@code this} executor + * instance. * - * @param callable the actual callable to execute - * @param attemptSettings current attempt settings - * @return actual attempt future + * @param callable the actual callable, which should be executed in a retriable context + * @return retrying future facade + */ + @Override + public RetryingFuture createFuture(Callable callable) { + return new RetryingFutureImpl<>(callable, retryAlgorithm, this); + } + + /** + * Submits an attempt for execution in a different thread. + * @param retryingFuture the future previously returned by {@link #createFuture(Callable)} */ @Override - public Future executeAttempt( - Callable callable, RetryAttemptSettings attemptSettings) { + public void submit(RetryingFuture retryingFuture) { + Future attemptFuture; try { - return scheduler.schedule( - callable, attemptSettings.getRandomizedRetryDelay().getMillis(), TimeUnit.MILLISECONDS); + attemptFuture = + scheduler.schedule( + retryingFuture.getCallable(), + retryingFuture.getAttemptSettings().getRandomizedRetryDelay().getMillis(), + TimeUnit.MILLISECONDS); } catch (RejectedExecutionException e) { - return Futures.immediateCancelledFuture(); + attemptFuture = Futures.immediateCancelledFuture(); } + retryingFuture.setAttemptFuture(attemptFuture); } } diff --git a/src/main/java/com/google/api/gax/retrying/RetryAttemptSettings.java b/src/main/java/com/google/api/gax/retrying/TimedAttemptSettings.java similarity index 93% rename from src/main/java/com/google/api/gax/retrying/RetryAttemptSettings.java rename to src/main/java/com/google/api/gax/retrying/TimedAttemptSettings.java index 0916e4c94..eb8da03ea 100644 --- a/src/main/java/com/google/api/gax/retrying/RetryAttemptSettings.java +++ b/src/main/java/com/google/api/gax/retrying/TimedAttemptSettings.java @@ -29,13 +29,14 @@ */ package com.google.api.gax.retrying; +import com.google.api.gax.core.ApiClock; import com.google.api.gax.core.RetrySettings; import org.joda.time.Duration; /** - * Execution attempt settings. Defines attempt-specific properties of a retry process. + * Timed attempt execution settings. Defines time-specific properties of a retry attempt. */ -public class RetryAttemptSettings { +public class TimedAttemptSettings { private final RetrySettings globalSettings; private final Duration retryDelay; @@ -44,7 +45,7 @@ public class RetryAttemptSettings { private final int attemptCount; private final long firstAttemptStartTime; - public RetryAttemptSettings( + public TimedAttemptSettings( RetrySettings globalSettings, Duration retryDelay, Duration rpcTimeout, @@ -98,7 +99,7 @@ public int getAttemptCount() { /** * The start time of the first attempt. Note that this value is dependent on the actual - * {@link com.google.api.gax.core.NanoClock} used during the process. + * {@link ApiClock} used during the process. */ public long getFirstAttemptStartTime() { return firstAttemptStartTime; diff --git a/src/main/java/com/google/api/gax/retrying/TimedRetryAlgorithm.java b/src/main/java/com/google/api/gax/retrying/TimedRetryAlgorithm.java new file mode 100644 index 000000000..851eac898 --- /dev/null +++ b/src/main/java/com/google/api/gax/retrying/TimedRetryAlgorithm.java @@ -0,0 +1,72 @@ +/* + * Copyright 2017, Google Inc. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package com.google.api.gax.retrying; + +/** + * A timed retry algorithm is responsible for the following operations: + * + *

        + *
      1. Creating first attempt {@link TimedAttemptSettings}. + *
      2. Accepting or rejecting a task for retry depending on the previous attempt settings and + * current time. + *
      3. Creating {@link TimedAttemptSettings} for each subsequent retry attempt. + *
      + * + * Implementations of this interface must be be thread-save. + */ +public interface TimedRetryAlgorithm { + + /** + * Creates a first attempt {@link TimedAttemptSettings}. + * + * @return first attempt settings + */ + TimedAttemptSettings createFirstAttempt(); + + /** + * Creates a next attempt {@link TimedAttemptSettings}, which defines properties of the next + * attempt. + * + * @param prevSettings previous attempt settings + * @return next attempt settings or {@code prevSettings} if the implementing algorithm does not + * provide specific settings for the next attempt + */ + TimedAttemptSettings createNextAttempt(TimedAttemptSettings prevSettings); + + /** + * Returns {@code true} if another attempt should be made, or {@code false} otherwise. + * + * @param nextAttemptSettings attempt settings, which will be used for the next attempt, if + * accepted + * @return {@code true} if another attempt should be made, or {@code false} otherwise + */ + boolean accept(TimedAttemptSettings nextAttemptSettings); +} diff --git a/src/test/java/com/google/api/gax/core/FakeNanoClock.java b/src/test/java/com/google/api/gax/core/FakeApiClock.java similarity index 95% rename from src/test/java/com/google/api/gax/core/FakeNanoClock.java rename to src/test/java/com/google/api/gax/core/FakeApiClock.java index 68394a241..d46df30f2 100644 --- a/src/test/java/com/google/api/gax/core/FakeNanoClock.java +++ b/src/test/java/com/google/api/gax/core/FakeApiClock.java @@ -31,10 +31,10 @@ import java.util.concurrent.TimeUnit; -public class FakeNanoClock implements NanoClock { +public class FakeApiClock implements ApiClock { private volatile long currentNanoTime; - public FakeNanoClock(long initialNanoTime) { + public FakeApiClock(long initialNanoTime) { currentNanoTime = initialNanoTime; } diff --git a/src/test/java/com/google/api/gax/grpc/CancellationHelpers.java b/src/test/java/com/google/api/gax/grpc/CancellationHelpers.java index c38eeb77b..f9891f7a1 100644 --- a/src/test/java/com/google/api/gax/grpc/CancellationHelpers.java +++ b/src/test/java/com/google/api/gax/grpc/CancellationHelpers.java @@ -47,10 +47,12 @@ public static void cancelInThreadAfterLatchCountDown( public void run() { try { latch.await(); + while (!resultFuture.cancel(true)) { + Thread.sleep(1L); + } } catch (InterruptedException e) { Thread.currentThread().interrupt(); } - resultFuture.cancel(true); } }); t.start(); diff --git a/src/test/java/com/google/api/gax/grpc/CancellationTest.java b/src/test/java/com/google/api/gax/grpc/CancellationTest.java index a666c1124..5a898d2e3 100644 --- a/src/test/java/com/google/api/gax/grpc/CancellationTest.java +++ b/src/test/java/com/google/api/gax/grpc/CancellationTest.java @@ -35,7 +35,7 @@ import com.google.api.gax.core.AbstractApiFuture; import com.google.api.gax.core.ApiFuture; -import com.google.api.gax.core.FakeNanoClock; +import com.google.api.gax.core.FakeApiClock; import com.google.api.gax.core.RetrySettings; import com.google.api.gax.core.SettableApiFuture; import com.google.common.collect.ImmutableSet; @@ -88,14 +88,14 @@ public class CancellationTest { .setTotalTimeout(Duration.millis(3000L)) .build(); - private FakeNanoClock fakeClock; + private FakeApiClock fakeClock; private RecordingScheduler executor; @Rule public ExpectedException thrown = ExpectedException.none(); @Before public void resetClock() { - fakeClock = new FakeNanoClock(System.nanoTime()); + fakeClock = new FakeApiClock(System.nanoTime()); executor = RecordingScheduler.create(fakeClock); } @@ -280,6 +280,6 @@ public void cancellationDuringSecondCall() throws Exception { Truth.assertThat(gotException).isNotNull(); Truth.assertThat(resultFuture.isDone()).isTrue(); Truth.assertThat(resultFuture.isCancelled()).isTrue(); - Truth.assertThat(innerFuture.isCancelled()).isTrue(); + Truth.assertThat(innerFuture.isDone()).isTrue(); } } diff --git a/src/test/java/com/google/api/gax/grpc/RecordingScheduler.java b/src/test/java/com/google/api/gax/grpc/RecordingScheduler.java index c618727a4..e82a80fb1 100644 --- a/src/test/java/com/google/api/gax/grpc/RecordingScheduler.java +++ b/src/test/java/com/google/api/gax/grpc/RecordingScheduler.java @@ -33,7 +33,7 @@ import static org.mockito.Matchers.anyLong; import static org.mockito.Mockito.when; -import com.google.api.gax.core.FakeNanoClock; +import com.google.api.gax.core.FakeApiClock; import java.util.ArrayList; import java.util.List; import java.util.concurrent.ScheduledExecutorService; @@ -49,7 +49,7 @@ abstract class RecordingScheduler implements ScheduledExecutorService { abstract List getSleepDurations(); - static RecordingScheduler create(final FakeNanoClock clock) { + static RecordingScheduler create(final FakeApiClock clock) { RecordingScheduler mock = Mockito.mock(RecordingScheduler.class); // mock class fields: diff --git a/src/test/java/com/google/api/gax/grpc/UnaryCallableTest.java b/src/test/java/com/google/api/gax/grpc/UnaryCallableTest.java index 9f03af00b..5f54ed25a 100644 --- a/src/test/java/com/google/api/gax/grpc/UnaryCallableTest.java +++ b/src/test/java/com/google/api/gax/grpc/UnaryCallableTest.java @@ -33,6 +33,7 @@ import com.google.api.gax.bundling.RequestBuilder; import com.google.api.gax.core.ApiFuture; import com.google.api.gax.core.ApiFutures; +import com.google.api.gax.core.FakeApiClock; import com.google.api.gax.core.FixedSizeCollection; import com.google.api.gax.core.FlowControlSettings; import com.google.api.gax.core.FlowController.LimitExceededBehavior; @@ -91,14 +92,14 @@ static ApiFuture immediateFailedFuture(Throwable t) { return ApiFutures.immediateFailedFuture(t); } - private FakeNanoClock fakeClock; + private FakeApiClock fakeClock; private RecordingScheduler executor; private ScheduledExecutorService bundlingExecutor; @Before public void resetClock() { - fakeClock = new FakeNanoClock(System.nanoTime()); - executor = new RecordingScheduler(fakeClock); + fakeClock = new FakeApiClock(System.nanoTime()); + executor = RecordingScheduler.create(fakeClock); } @Before @@ -276,6 +277,62 @@ public void retry() { Truth.assertThat(callable.call(1)).isEqualTo(2); } + @Test(expected = ApiException.class) + public void retryTotalTimeoutExceeded() { + ImmutableSet retryable = ImmutableSet.of(Status.Code.UNAVAILABLE); + Throwable throwable = Status.UNAVAILABLE.asException(); + Mockito.when(callInt.futureCall((Integer) Mockito.any(), (CallContext) Mockito.any())) + .thenReturn(UnaryCallableTest.immediateFailedFuture(throwable)) + .thenReturn(immediateFuture(2)); + + RetrySettings retrySettings = + FAST_RETRY_SETTINGS + .toBuilder() + .setInitialRetryDelay(Duration.millis(Integer.MAX_VALUE)) + .setMaxRetryDelay(Duration.millis(Integer.MAX_VALUE)) + .build(); + UnaryCallable callable = + UnaryCallable.create(callInt) + .retryableOn(retryable) + .retrying(retrySettings, executor, fakeClock); + callable.call(1); + } + + @Test(expected = ApiException.class) + public void retryMaxAttemptsExeeded() { + ImmutableSet retryable = ImmutableSet.of(Status.Code.UNAVAILABLE); + Throwable throwable = Status.UNAVAILABLE.asException(); + Mockito.when(callInt.futureCall((Integer) Mockito.any(), (CallContext) Mockito.any())) + .thenReturn(UnaryCallableTest.immediateFailedFuture(throwable)) + .thenReturn(UnaryCallableTest.immediateFailedFuture(throwable)) + .thenReturn(immediateFuture(2)); + + RetrySettings retrySettings = FAST_RETRY_SETTINGS.toBuilder().setMaxAttempts(2).build(); + UnaryCallable callable = + UnaryCallable.create(callInt) + .retryableOn(retryable) + .retrying(retrySettings, executor, fakeClock); + callable.call(1); + } + + @Test + public void retryWithinMaxAttempts() { + ImmutableSet retryable = ImmutableSet.of(Status.Code.UNAVAILABLE); + Throwable throwable = Status.UNAVAILABLE.asException(); + Mockito.when(callInt.futureCall((Integer) Mockito.any(), (CallContext) Mockito.any())) + .thenReturn(UnaryCallableTest.immediateFailedFuture(throwable)) + .thenReturn(UnaryCallableTest.immediateFailedFuture(throwable)) + .thenReturn(immediateFuture(2)); + + RetrySettings retrySettings = FAST_RETRY_SETTINGS.toBuilder().setMaxAttempts(3).build(); + UnaryCallable callable = + UnaryCallable.create(callInt) + .retryableOn(retryable) + .retrying(retrySettings, executor, fakeClock); + callable.call(1); + Truth.assertThat(callable.call(1)).isEqualTo(2); + } + @Test public void retryOnStatusUnknown() { ImmutableSet retryable = ImmutableSet.of(Status.Code.UNKNOWN); diff --git a/src/test/java/com/google/api/gax/retrying/AbstractRetryHandlerTest.java b/src/test/java/com/google/api/gax/retrying/AbstractRetryingExecutorTest.java similarity index 76% rename from src/test/java/com/google/api/gax/retrying/AbstractRetryHandlerTest.java rename to src/test/java/com/google/api/gax/retrying/AbstractRetryingExecutorTest.java index 85bf40d65..3e9338e91 100644 --- a/src/test/java/com/google/api/gax/retrying/AbstractRetryHandlerTest.java +++ b/src/test/java/com/google/api/gax/retrying/AbstractRetryingExecutorTest.java @@ -44,16 +44,32 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) -public abstract class AbstractRetryHandlerTest { +public abstract class AbstractRetryingExecutorTest { - protected abstract RetryHandler getRetryHandler(); + protected abstract RetryingExecutor getRetryingExecutor(RetrySettings retrySettings); + + protected ExceptionRetryAlgorithm getNoOpExceptionRetryAlgorithm() { + return new ExceptionRetryAlgorithm() { + @Override + public TimedAttemptSettings createNextAttempt( + Throwable prevThrowable, TimedAttemptSettings prevSettings) { + return prevSettings; + } + + @Override + public boolean accept(Throwable prevThrowable) { + return true; + } + }; + } @Test public void testNoFailures() throws ExecutionException, InterruptedException { - RetryHandler handler = getRetryHandler(); FailingCallable callable = new FailingCallable(0, "SUCCESS"); - RetryFuture future = handler.createFirstAttempt(callable, FAST_RETRY_SETTINGS); - future.setAttemptFuture(handler.executeAttempt(callable, future.getAttemptSettings())); + RetryingExecutor executor = getRetryingExecutor(FAST_RETRY_SETTINGS); + RetryingFuture future = executor.createFuture(callable); + executor.submit(future); + assertEquals("SUCCESS", future.get()); assertTrue(future.isDone()); assertFalse(future.isCancelled()); @@ -62,10 +78,11 @@ public void testNoFailures() throws ExecutionException, InterruptedException { @Test public void testSuccessWithFailures() throws ExecutionException, InterruptedException { - RetryHandler handler = getRetryHandler(); FailingCallable callable = new FailingCallable(5, "SUCCESS"); - RetryFuture future = handler.createFirstAttempt(callable, FAST_RETRY_SETTINGS); - future.setAttemptFuture(handler.executeAttempt(callable, future.getAttemptSettings())); + RetryingExecutor executor = getRetryingExecutor(FAST_RETRY_SETTINGS); + RetryingFuture future = executor.createFuture(callable); + executor.submit(future); + assertEquals("SUCCESS", future.get()); assertTrue(future.isDone()); assertFalse(future.isCancelled()); @@ -74,10 +91,10 @@ public void testSuccessWithFailures() throws ExecutionException, InterruptedExce @Test public void testMaxRetriesExcceeded() { - RetryHandler handler = getRetryHandler(); FailingCallable callable = new FailingCallable(6, "FAILURE"); - RetryFuture future = handler.createFirstAttempt(callable, FAST_RETRY_SETTINGS); - future.setAttemptFuture(handler.executeAttempt(callable, future.getAttemptSettings())); + RetryingExecutor executor = getRetryingExecutor(FAST_RETRY_SETTINGS); + RetryingFuture future = executor.createFuture(callable); + executor.submit(future); CustomException exception = null; try { @@ -86,7 +103,6 @@ public void testMaxRetriesExcceeded() { exception = (CustomException) e.getCause(); } assertEquals(CustomException.class, exception.getClass()); - assertEquals(5, future.getAttemptSettings().getAttemptCount()); assertTrue(future.isDone()); assertFalse(future.isCancelled()); @@ -94,16 +110,16 @@ public void testMaxRetriesExcceeded() { @Test public void testTotalTimeoutExcceeded() throws Exception { - RetryHandler handler = getRetryHandler(); - FailingCallable callable = new FailingCallable(6, "FAILURE"); RetrySettings retrySettings = FAST_RETRY_SETTINGS .toBuilder() .setInitialRetryDelay(Duration.millis(Integer.MAX_VALUE)) .setMaxRetryDelay(Duration.millis(Integer.MAX_VALUE)) .build(); - RetryFuture future = handler.createFirstAttempt(callable, retrySettings); - future.setAttemptFuture(handler.executeAttempt(callable, future.getAttemptSettings())); + RetryingExecutor executor = getRetryingExecutor(retrySettings); + FailingCallable callable = new FailingCallable(6, "FAILURE"); + RetryingFuture future = executor.createFuture(callable); + executor.submit(future); CustomException exception = null; try { @@ -119,8 +135,8 @@ public void testTotalTimeoutExcceeded() throws Exception { @Test(expected = CancellationException.class) public void testCancelOuterFuture() throws ExecutionException, InterruptedException { - RetryHandler handler = getRetryHandler(); FailingCallable callable = new FailingCallable(4, "SUCCESS"); + RetrySettings retrySettings = FAST_RETRY_SETTINGS .toBuilder() @@ -128,10 +144,11 @@ public void testCancelOuterFuture() throws ExecutionException, InterruptedExcept .setMaxRetryDelay(Duration.millis(1_000L)) .setTotalTimeout(Duration.millis(10_0000L)) .build(); - - RetryFuture future = handler.createFirstAttempt(callable, retrySettings); + RetryingExecutor executor = getRetryingExecutor(retrySettings); + RetryingFuture future = executor.createFuture(callable); future.cancel(false); - future.setAttemptFuture(handler.executeAttempt(callable, future.getAttemptSettings())); + executor.submit(future); + assertTrue(future.isDone()); assertTrue(future.isCancelled()); assertTrue(future.getAttemptSettings().getAttemptCount() < 4); diff --git a/src/test/java/com/google/api/gax/retrying/DirectRetryHandlerTest.java b/src/test/java/com/google/api/gax/retrying/DirectRetryingExecutorTest.java similarity index 78% rename from src/test/java/com/google/api/gax/retrying/DirectRetryHandlerTest.java rename to src/test/java/com/google/api/gax/retrying/DirectRetryingExecutorTest.java index 81eb66af0..b3d71bf23 100644 --- a/src/test/java/com/google/api/gax/retrying/DirectRetryHandlerTest.java +++ b/src/test/java/com/google/api/gax/retrying/DirectRetryingExecutorTest.java @@ -29,15 +29,21 @@ */ package com.google.api.gax.retrying; +import com.google.api.gax.core.RetrySettings; import com.google.api.gax.core.SystemClock; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @RunWith(JUnit4.class) -public class DirectRetryHandlerTest extends AbstractRetryHandlerTest { +public class DirectRetryingExecutorTest extends AbstractRetryingExecutorTest { @Override - protected RetryHandler getRetryHandler() { - return new DirectRetryHandler<>(SystemClock.getDefaultClock()); + protected RetryingExecutor getRetryingExecutor(RetrySettings retrySettings) { + RetryAlgorithm retryAlgorithm = + new RetryAlgorithm( + getNoOpExceptionRetryAlgorithm(), + new ExponentialRetryAlgorithm(retrySettings, SystemClock.getDefaultClock())); + + return new DirectRetryingExecutor<>(retryAlgorithm); } } diff --git a/src/test/java/com/google/api/gax/retrying/FailingCallable.java b/src/test/java/com/google/api/gax/retrying/FailingCallable.java index 6bfab7a2d..c411a24a7 100644 --- a/src/test/java/com/google/api/gax/retrying/FailingCallable.java +++ b/src/test/java/com/google/api/gax/retrying/FailingCallable.java @@ -44,7 +44,7 @@ class FailingCallable implements Callable { .setInitialRpcTimeout(Duration.millis(2L)) .setRpcTimeoutMultiplier(1) .setMaxRpcTimeout(Duration.millis(2L)) - .setTotalTimeout(Duration.millis(50L)) + .setTotalTimeout(Duration.millis(100L)) .build(); private AtomicInteger attemptsCount = new AtomicInteger(0); diff --git a/src/test/java/com/google/api/gax/retrying/ScheduledRetryHandlerTest.java b/src/test/java/com/google/api/gax/retrying/ScheduledRetryingExecutorTest.java similarity index 80% rename from src/test/java/com/google/api/gax/retrying/ScheduledRetryHandlerTest.java rename to src/test/java/com/google/api/gax/retrying/ScheduledRetryingExecutorTest.java index fc30861bb..436e93ee2 100644 --- a/src/test/java/com/google/api/gax/retrying/ScheduledRetryHandlerTest.java +++ b/src/test/java/com/google/api/gax/retrying/ScheduledRetryingExecutorTest.java @@ -32,7 +32,7 @@ import static com.google.api.gax.retrying.FailingCallable.FAST_RETRY_SETTINGS; import static org.junit.Assert.assertTrue; -import com.google.api.gax.core.DefaultNanoClock; +import com.google.api.gax.core.NanoClock; import com.google.api.gax.core.RetrySettings; import java.util.concurrent.CancellationException; import java.util.concurrent.ExecutionException; @@ -45,11 +45,8 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) -public class ScheduledRetryHandlerTest extends AbstractRetryHandlerTest { - +public class ScheduledRetryingExecutorTest extends AbstractRetryingExecutorTest { private ScheduledExecutorService executorService = Executors.newSingleThreadScheduledExecutor(); - private ScheduledRetryHandler retryHandler = - new ScheduledRetryHandler<>(DefaultNanoClock.getDefaultClock(), executorService); @After public void after() { @@ -57,13 +54,17 @@ public void after() { } @Override - protected RetryHandler getRetryHandler() { - return retryHandler; + protected RetryingExecutor getRetryingExecutor(RetrySettings retrySettings) { + RetryAlgorithm retryAlgorithm = + new RetryAlgorithm( + getNoOpExceptionRetryAlgorithm(), + new ExponentialRetryAlgorithm(retrySettings, NanoClock.getDefaultClock())); + + return new ScheduledRetryingExecutor<>(retryAlgorithm, executorService); } @Test(expected = CancellationException.class) public void testCancelOuterFutureAfterStart() throws ExecutionException, InterruptedException { - RetryHandler handler = getRetryHandler(); FailingCallable callable = new FailingCallable(4, "SUCCESS"); RetrySettings retrySettings = FAST_RETRY_SETTINGS @@ -72,9 +73,10 @@ public void testCancelOuterFutureAfterStart() throws ExecutionException, Interru .setMaxRetryDelay(Duration.millis(1_000L)) .setTotalTimeout(Duration.millis(10_0000L)) .build(); + RetryingExecutor executor = getRetryingExecutor(retrySettings); + RetryingFuture future = executor.createFuture(callable); + executor.submit(future); - RetryFuture future = handler.createFirstAttempt(callable, retrySettings); - future.setAttemptFuture(handler.executeAttempt(callable, future.getAttemptSettings())); future.cancel(false); assertTrue(future.isDone()); assertTrue(future.isCancelled()); @@ -84,7 +86,6 @@ public void testCancelOuterFutureAfterStart() throws ExecutionException, Interru @Test(expected = CancellationException.class) public void testCancelProxiedFutureAfterStart() throws ExecutionException, InterruptedException { - RetryHandler handler = getRetryHandler(); FailingCallable callable = new FailingCallable(5, "SUCCESS"); RetrySettings retrySettings = FAST_RETRY_SETTINGS @@ -93,13 +94,15 @@ public void testCancelProxiedFutureAfterStart() throws ExecutionException, Inter .setMaxRetryDelay(Duration.millis(1_000L)) .setTotalTimeout(Duration.millis(10_0000L)) .build(); + RetryingExecutor executor = getRetryingExecutor(retrySettings); + RetryingFuture future = executor.createFuture(callable); + executor.submit(future); - RetryFuture future = handler.createFirstAttempt(callable, retrySettings); - future.setAttemptFuture(handler.executeAttempt(callable, future.getAttemptSettings())); Thread.sleep(50L); + RetryingExecutor handler = getRetryingExecutor(retrySettings); //Note that shutdownNow() will not cancel internal FutureTasks automatically, which - //may potentially cause another thread handing on RetryFuture#get() call forever. + //may potentially cause another thread handing on RetryingFuture#get() call forever. //Canceling the tasks returned by shutdownNow() also does not help, because of missing feature //in guava's ListenableScheduledFuture, which does not cancel itself, when its delegate is canceled. //So only the graceful shutdown() is supported properly. From b380e16e22b2a9c5b51520eb810a3ca0ade51c62 Mon Sep 17 00:00:00 2001 From: vam Date: Thu, 16 Mar 2017 18:29:19 -0700 Subject: [PATCH 09/15] googleJavaFormat files --- .../java/com/google/api/gax/grpc/RetryingCallable.java | 2 +- .../api/gax/retrying/ExponentialRetryAlgorithm.java | 10 +++++----- .../api/gax/retrying/ScheduledRetryingExecutor.java | 8 +++++--- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/google/api/gax/grpc/RetryingCallable.java b/src/main/java/com/google/api/gax/grpc/RetryingCallable.java index a38a927a3..2aacd1136 100644 --- a/src/main/java/com/google/api/gax/grpc/RetryingCallable.java +++ b/src/main/java/com/google/api/gax/grpc/RetryingCallable.java @@ -29,8 +29,8 @@ */ package com.google.api.gax.grpc; -import com.google.api.gax.core.ApiFuture; import com.google.api.gax.core.ApiClock; +import com.google.api.gax.core.ApiFuture; import com.google.api.gax.core.RetrySettings; import com.google.api.gax.core.internal.ApiFutureToListenableFuture; import com.google.api.gax.retrying.ExceptionRetryAlgorithm; diff --git a/src/main/java/com/google/api/gax/retrying/ExponentialRetryAlgorithm.java b/src/main/java/com/google/api/gax/retrying/ExponentialRetryAlgorithm.java index 1db1429ba..17b0adeef 100644 --- a/src/main/java/com/google/api/gax/retrying/ExponentialRetryAlgorithm.java +++ b/src/main/java/com/google/api/gax/retrying/ExponentialRetryAlgorithm.java @@ -38,8 +38,8 @@ import org.joda.time.Duration; /** - * The timed retry algorithm which uses randomized exponential backoff factor for calculating the next - * attempt execution time. + * The timed retry algorithm which uses randomized exponential backoff factor for calculating the + * next attempt execution time. * *

      * This class is thread-safe. @@ -79,9 +79,9 @@ public TimedAttemptSettings createFirstAttempt() { } /** - * Creates a next attempt {@link TimedAttemptSettings}. The implementation increments the - * current attempt count and uses randomized exponential backoff factor for calculating next - * attempt execution time. + * Creates a next attempt {@link TimedAttemptSettings}. The implementation increments the current + * attempt count and uses randomized exponential backoff factor for calculating next attempt + * execution time. * * @param prevSettings previous attempt settings * @return next attempt settings diff --git a/src/main/java/com/google/api/gax/retrying/ScheduledRetryingExecutor.java b/src/main/java/com/google/api/gax/retrying/ScheduledRetryingExecutor.java index b2f5e560f..437f3d8ff 100644 --- a/src/main/java/com/google/api/gax/retrying/ScheduledRetryingExecutor.java +++ b/src/main/java/com/google/api/gax/retrying/ScheduledRetryingExecutor.java @@ -43,7 +43,8 @@ * Unless a direct executor service is used, this handler will schedule attempts for an execution in * another thread. * - *

      This class is thread-safe. + *

      + * This class is thread-safe. * * @param response type */ @@ -53,8 +54,8 @@ public class ScheduledRetryingExecutor implements RetryingExecutor createFuture(Callable callable) { /** * Submits an attempt for execution in a different thread. + * * @param retryingFuture the future previously returned by {@link #createFuture(Callable)} */ @Override From 9d79b03494e3d2f83c0b0507aa0dc6514ebf2fdf Mon Sep 17 00:00:00 2001 From: vam Date: Fri, 17 Mar 2017 14:28:28 -0700 Subject: [PATCH 10/15] Adressed PR comments, PTAL. --- .../google/api/gax/core/RetrySettings.java | 6 ++-- .../google/api/gax/grpc/RetryingCallable.java | 32 +++++++++---------- .../gax/retrying/DirectRetryingExecutor.java | 8 +++-- .../gax/retrying/ExceptionRetryAlgorithm.java | 1 - .../retrying/ExponentialRetryAlgorithm.java | 4 +-- .../api/gax/retrying/RetryingExecutor.java | 5 +-- .../api/gax/retrying/RetryingFuture.java | 3 +- .../api/gax/retrying/RetryingFutureImpl.java | 8 ++--- .../retrying/ScheduledRetryingExecutor.java | 7 ++-- .../api/gax/retrying/TimedRetryAlgorithm.java | 1 - 10 files changed, 38 insertions(+), 37 deletions(-) diff --git a/src/main/java/com/google/api/gax/core/RetrySettings.java b/src/main/java/com/google/api/gax/core/RetrySettings.java index 25eb14ecf..0d219da73 100644 --- a/src/main/java/com/google/api/gax/core/RetrySettings.java +++ b/src/main/java/com/google/api/gax/core/RetrySettings.java @@ -206,9 +206,9 @@ public abstract static class Builder { public abstract double getRetryDelayMultiplier(); /** - * MaxAttempts defines the maximum number of attempts to perform. If number of attempts reaches - * this limit the logic will give up retrying even if the total retry time is still lower than - * TotalTimeout. + * MaxAttempts defines the maximum number of attempts to perform. If the number of attempts + * reaches this limit, the logic will give up retrying even if the total retry time is still + * lower than TotalTimeout. */ public abstract int getMaxAttempts(); diff --git a/src/main/java/com/google/api/gax/grpc/RetryingCallable.java b/src/main/java/com/google/api/gax/grpc/RetryingCallable.java index 2aacd1136..efa83a8f2 100644 --- a/src/main/java/com/google/api/gax/grpc/RetryingCallable.java +++ b/src/main/java/com/google/api/gax/grpc/RetryingCallable.java @@ -29,10 +29,11 @@ */ package com.google.api.gax.grpc; +import com.google.api.gax.core.AbstractApiFuture; import com.google.api.gax.core.ApiClock; import com.google.api.gax.core.ApiFuture; +import com.google.api.gax.core.ApiFutures; import com.google.api.gax.core.RetrySettings; -import com.google.api.gax.core.internal.ApiFutureToListenableFuture; import com.google.api.gax.retrying.ExceptionRetryAlgorithm; import com.google.api.gax.retrying.ExponentialRetryAlgorithm; import com.google.api.gax.retrying.RetryAlgorithm; @@ -41,8 +42,6 @@ import com.google.api.gax.retrying.ScheduledRetryingExecutor; import com.google.api.gax.retrying.TimedAttemptSettings; import com.google.common.base.Preconditions; -import com.google.common.util.concurrent.AbstractFuture; -import com.google.common.util.concurrent.Futures; import io.grpc.CallOptions; import io.grpc.Status.Code; import java.util.concurrent.Callable; @@ -93,18 +92,19 @@ public String toString() { } private static CallContext getCallContextWithDeadlineAfter( - CallContext oldCtx, Duration rpcTimeout) { - CallOptions oldOpt = oldCtx.getCallOptions(); - CallOptions newOpt = oldOpt.withDeadlineAfter(rpcTimeout.getMillis(), TimeUnit.MILLISECONDS); - CallContext newCtx = oldCtx.withCallOptions(newOpt); - - if (oldOpt.getDeadlineNanoTime() == null) { - return newCtx; + CallContext oldContext, Duration rpcTimeout) { + CallOptions oldOptions = oldContext.getCallOptions(); + CallOptions newOptions = + oldOptions.withDeadlineAfter(rpcTimeout.getMillis(), TimeUnit.MILLISECONDS); + CallContext newContext = oldContext.withCallOptions(newOptions); + + if (oldOptions.getDeadlineNanoTime() == null) { + return newContext; } - if (oldOpt.getDeadlineNanoTime() < newOpt.getDeadlineNanoTime()) { - return oldCtx; + if (oldOptions.getDeadlineNanoTime() < newOptions.getDeadlineNanoTime()) { + return oldContext; } - return newCtx; + return newContext; } private static class GrpcRetryCallable implements Callable { @@ -137,9 +137,9 @@ public ResponseT call() { return null; } ApiFuture internalFuture = callable.futureCall(request, callContext); - externalFuture.setAttemptFuture(new ApiFutureToListenableFuture<>(internalFuture)); + externalFuture.setAttemptFuture(internalFuture); } catch (Throwable e) { - externalFuture.setAttemptFuture(Futures.immediateFailedFuture(e)); + externalFuture.setAttemptFuture(ApiFutures.immediateFailedFuture(e)); throw e; } @@ -170,7 +170,7 @@ public boolean accept(Throwable prevThrowable) { } } - private static class NonCancelableFuture extends AbstractFuture { + private static class NonCancelableFuture extends AbstractApiFuture { @Override public boolean cancel(boolean mayInterruptIfRunning) { return false; diff --git a/src/main/java/com/google/api/gax/retrying/DirectRetryingExecutor.java b/src/main/java/com/google/api/gax/retrying/DirectRetryingExecutor.java index 9f23bf78d..8bbc33c70 100644 --- a/src/main/java/com/google/api/gax/retrying/DirectRetryingExecutor.java +++ b/src/main/java/com/google/api/gax/retrying/DirectRetryingExecutor.java @@ -31,11 +31,12 @@ import static com.google.common.base.Preconditions.checkNotNull; +import com.google.api.gax.core.internal.ListenableFutureToApiFuture; import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.ListenableFuture; import java.io.InterruptedIOException; import java.nio.channels.ClosedByInterruptException; import java.util.concurrent.Callable; -import java.util.concurrent.Future; import org.joda.time.Duration; /** @@ -82,7 +83,7 @@ public RetryingFuture createFuture(Callable callable) { */ @Override public void submit(RetryingFuture retryingFuture) { - Future attemptFuture; + ListenableFuture attemptFuture; try { Duration delay = retryingFuture.getAttemptSettings().getRandomizedRetryDelay(); if (Duration.ZERO.compareTo(delay) < 0) { @@ -95,6 +96,7 @@ public void submit(RetryingFuture retryingFuture) { } catch (Throwable e) { attemptFuture = Futures.immediateFailedFuture(e); } - retryingFuture.setAttemptFuture(attemptFuture); + + retryingFuture.setAttemptFuture(new ListenableFutureToApiFuture<>(attemptFuture)); } } diff --git a/src/main/java/com/google/api/gax/retrying/ExceptionRetryAlgorithm.java b/src/main/java/com/google/api/gax/retrying/ExceptionRetryAlgorithm.java index f1806e681..e602fcda5 100644 --- a/src/main/java/com/google/api/gax/retrying/ExceptionRetryAlgorithm.java +++ b/src/main/java/com/google/api/gax/retrying/ExceptionRetryAlgorithm.java @@ -56,7 +56,6 @@ TimedAttemptSettings createNextAttempt( * Returns {@code true} if another attempt should be made, or {@code false} otherwise. * * @param prevThrowable exception thrown by the previous attempt - * @return {@code true} if another attempt should be made, or {@code false} otherwise */ boolean accept(Throwable prevThrowable); } diff --git a/src/main/java/com/google/api/gax/retrying/ExponentialRetryAlgorithm.java b/src/main/java/com/google/api/gax/retrying/ExponentialRetryAlgorithm.java index 17b0adeef..133c2d2ff 100644 --- a/src/main/java/com/google/api/gax/retrying/ExponentialRetryAlgorithm.java +++ b/src/main/java/com/google/api/gax/retrying/ExponentialRetryAlgorithm.java @@ -62,8 +62,8 @@ public ExponentialRetryAlgorithm(RetrySettings globalSettings, ApiClock clock) { } /** - * Creates a first attempt {@link TimedAttemptSettings}. By default the first attempt is - * configured to be executed immediately. + * Creates a first attempt {@link TimedAttemptSettings}. The first attempt is configured to be + * executed immediately. * * @return first attempt settings */ diff --git a/src/main/java/com/google/api/gax/retrying/RetryingExecutor.java b/src/main/java/com/google/api/gax/retrying/RetryingExecutor.java index 31be02c70..1a5bb0d82 100644 --- a/src/main/java/com/google/api/gax/retrying/RetryingExecutor.java +++ b/src/main/java/com/google/api/gax/retrying/RetryingExecutor.java @@ -36,7 +36,7 @@ * *

        *
      1. Creating first attempt {@link RetryingFuture}, which acts as a facade, hiding from client - * code the actual scheduled retry attempts execution. + * code the actual execution of scheduled retry attempts. *
      2. Executing the actual {@link Callable} in a retriable context. *
      * @@ -59,7 +59,8 @@ public interface RetryingExecutor { * attempt in the current thread or schedule it for an execution, using some sort of async * execution service. * - * @param retryingFuture the future previously returned by {@link #createFuture(Callable)} + * @param retryingFuture the future previously returned by {@link #createFuture(Callable)}, and + * reused for each subsequent attempt of same operation. */ void submit(RetryingFuture retryingFuture); } diff --git a/src/main/java/com/google/api/gax/retrying/RetryingFuture.java b/src/main/java/com/google/api/gax/retrying/RetryingFuture.java index 1f2721bc9..6138096a6 100644 --- a/src/main/java/com/google/api/gax/retrying/RetryingFuture.java +++ b/src/main/java/com/google/api/gax/retrying/RetryingFuture.java @@ -31,7 +31,6 @@ import com.google.api.gax.core.ApiFuture; import java.util.concurrent.Callable; -import java.util.concurrent.Future; /** * Represents a retrying future. This is a facade hiding all the complications of an asynchronous @@ -49,7 +48,7 @@ public interface RetryingFuture extends ApiFuture { * * @param attemptFuture the attempt future */ - void setAttemptFuture(Future attemptFuture); + void setAttemptFuture(ApiFuture attemptFuture); /** Returns current (active) attempt settings. */ TimedAttemptSettings getAttemptSettings(); diff --git a/src/main/java/com/google/api/gax/retrying/RetryingFutureImpl.java b/src/main/java/com/google/api/gax/retrying/RetryingFutureImpl.java index 76bdf6166..3a6a90c53 100644 --- a/src/main/java/com/google/api/gax/retrying/RetryingFutureImpl.java +++ b/src/main/java/com/google/api/gax/retrying/RetryingFutureImpl.java @@ -32,11 +32,11 @@ import static com.google.common.base.Preconditions.checkNotNull; +import com.google.api.gax.core.ApiFuture; import com.google.api.gax.core.ApiFutureCallback; +import com.google.api.gax.core.ApiFutures; import com.google.common.util.concurrent.AbstractFuture; import com.google.common.util.concurrent.FutureCallback; -import com.google.common.util.concurrent.Futures; -import com.google.common.util.concurrent.ListenableFuture; import java.util.concurrent.Callable; import java.util.concurrent.Future; @@ -94,7 +94,7 @@ public boolean cancel(boolean mayInterruptIfRunning) { } @Override - public void setAttemptFuture(Future attemptFuture) { + public void setAttemptFuture(ApiFuture attemptFuture) { if (isDone()) { return; } @@ -104,7 +104,7 @@ public void setAttemptFuture(Future attemptFuture) { } if (attemptFuture != null) { attemptFutureCallback = new AttemptFutureCallback(attemptFuture); - Futures.addCallback((ListenableFuture) attemptFuture, attemptFutureCallback); + ApiFutures.addCallback(attemptFuture, attemptFutureCallback); if (isCancelled()) { attemptFuture.cancel(false); } diff --git a/src/main/java/com/google/api/gax/retrying/ScheduledRetryingExecutor.java b/src/main/java/com/google/api/gax/retrying/ScheduledRetryingExecutor.java index 437f3d8ff..cb68554e7 100644 --- a/src/main/java/com/google/api/gax/retrying/ScheduledRetryingExecutor.java +++ b/src/main/java/com/google/api/gax/retrying/ScheduledRetryingExecutor.java @@ -29,11 +29,12 @@ */ package com.google.api.gax.retrying; +import com.google.api.gax.core.internal.ListenableFutureToApiFuture; import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListeningScheduledExecutorService; import com.google.common.util.concurrent.MoreExecutors; import java.util.concurrent.Callable; -import java.util.concurrent.Future; import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; @@ -86,7 +87,7 @@ public RetryingFuture createFuture(Callable callable) { */ @Override public void submit(RetryingFuture retryingFuture) { - Future attemptFuture; + ListenableFuture attemptFuture; try { attemptFuture = scheduler.schedule( @@ -96,6 +97,6 @@ public void submit(RetryingFuture retryingFuture) { } catch (RejectedExecutionException e) { attemptFuture = Futures.immediateCancelledFuture(); } - retryingFuture.setAttemptFuture(attemptFuture); + retryingFuture.setAttemptFuture(new ListenableFutureToApiFuture<>(attemptFuture)); } } diff --git a/src/main/java/com/google/api/gax/retrying/TimedRetryAlgorithm.java b/src/main/java/com/google/api/gax/retrying/TimedRetryAlgorithm.java index 851eac898..70df7b459 100644 --- a/src/main/java/com/google/api/gax/retrying/TimedRetryAlgorithm.java +++ b/src/main/java/com/google/api/gax/retrying/TimedRetryAlgorithm.java @@ -66,7 +66,6 @@ public interface TimedRetryAlgorithm { * * @param nextAttemptSettings attempt settings, which will be used for the next attempt, if * accepted - * @return {@code true} if another attempt should be made, or {@code false} otherwise */ boolean accept(TimedAttemptSettings nextAttemptSettings); } From 04f7f095213b434a65a8890797abafe8952f54f3 Mon Sep 17 00:00:00 2001 From: vam Date: Fri, 17 Mar 2017 16:04:24 -0700 Subject: [PATCH 11/15] changed retry algorithms to return null in case if they do not provide next attempt settings. renamed SystemClock to CurrentMillisClock. --- .../gax/core/{SystemClock.java => CurrentMillisClock.java} | 6 +++--- src/main/java/com/google/api/gax/grpc/RetryingCallable.java | 2 +- .../google/api/gax/retrying/ExceptionRetryAlgorithm.java | 4 ++-- .../java/com/google/api/gax/retrying/RetryAlgorithm.java | 5 ++--- .../com/google/api/gax/retrying/TimedRetryAlgorithm.java | 4 ++-- .../google/api/gax/retrying/DirectRetryingExecutorTest.java | 4 ++-- 6 files changed, 12 insertions(+), 13 deletions(-) rename src/main/java/com/google/api/gax/core/{SystemClock.java => CurrentMillisClock.java} (92%) diff --git a/src/main/java/com/google/api/gax/core/SystemClock.java b/src/main/java/com/google/api/gax/core/CurrentMillisClock.java similarity index 92% rename from src/main/java/com/google/api/gax/core/SystemClock.java rename to src/main/java/com/google/api/gax/core/CurrentMillisClock.java index 5e27a15f0..219f24c64 100644 --- a/src/main/java/com/google/api/gax/core/SystemClock.java +++ b/src/main/java/com/google/api/gax/core/CurrentMillisClock.java @@ -38,16 +38,16 @@ * Implementation of the {@link ApiClock} interface, which uses {@link System#currentTimeMillis()} * as time source. */ -public final class SystemClock implements ApiClock, Serializable { +public final class CurrentMillisClock implements ApiClock, Serializable { private static final long serialVersionUID = -6019259882852183285L; - private static final ApiClock DEFAULT_CLOCK = new SystemClock(); + private static final ApiClock DEFAULT_CLOCK = new CurrentMillisClock(); public static ApiClock getDefaultClock() { return DEFAULT_CLOCK; } - private SystemClock() {} + private CurrentMillisClock() {} @Override public long nanoTime() { diff --git a/src/main/java/com/google/api/gax/grpc/RetryingCallable.java b/src/main/java/com/google/api/gax/grpc/RetryingCallable.java index efa83a8f2..1de9bd5af 100644 --- a/src/main/java/com/google/api/gax/grpc/RetryingCallable.java +++ b/src/main/java/com/google/api/gax/grpc/RetryingCallable.java @@ -160,7 +160,7 @@ public TimedAttemptSettings createNextAttempt( prevSettings.getAttemptCount() + 1, prevSettings.getFirstAttemptStartTime()); } - return prevSettings; + return null; } @Override diff --git a/src/main/java/com/google/api/gax/retrying/ExceptionRetryAlgorithm.java b/src/main/java/com/google/api/gax/retrying/ExceptionRetryAlgorithm.java index e602fcda5..d6b443cab 100644 --- a/src/main/java/com/google/api/gax/retrying/ExceptionRetryAlgorithm.java +++ b/src/main/java/com/google/api/gax/retrying/ExceptionRetryAlgorithm.java @@ -46,8 +46,8 @@ public interface ExceptionRetryAlgorithm { * * @param prevThrowable exception thrown by the previous attempt * @param prevSettings previous attempt settings - * @return next attempt settings or {@code prevSettings}, if the implementing algorithm does not - * provide specific settings for the next attempt + * @return next attempt settings or {@code null}, if the implementing algorithm does not provide + * specific settings for the next attempt */ TimedAttemptSettings createNextAttempt( Throwable prevThrowable, TimedAttemptSettings prevSettings); diff --git a/src/main/java/com/google/api/gax/retrying/RetryAlgorithm.java b/src/main/java/com/google/api/gax/retrying/RetryAlgorithm.java index 6d1807cf9..9cfc007ab 100644 --- a/src/main/java/com/google/api/gax/retrying/RetryAlgorithm.java +++ b/src/main/java/com/google/api/gax/retrying/RetryAlgorithm.java @@ -71,14 +71,13 @@ public TimedAttemptSettings createFirstAttempt() { * * @param prevThrowable exception thrown by the previous attempt * @param prevSettings previous attempt settings - * @return next attempt settings or {@code prevSettings}, if the implementing algorithm does not - * provide specific settings for the next attempt + * @return next attempt settings */ public TimedAttemptSettings createNextAttempt( Throwable prevThrowable, TimedAttemptSettings prevSettings) { TimedAttemptSettings newSettings = exceptionAlgorithm.createNextAttempt(prevThrowable, prevSettings); - if (prevSettings.equals(newSettings)) { + if (newSettings == null) { newSettings = timedAlgorithm.createNextAttempt(prevSettings); } return newSettings; diff --git a/src/main/java/com/google/api/gax/retrying/TimedRetryAlgorithm.java b/src/main/java/com/google/api/gax/retrying/TimedRetryAlgorithm.java index 70df7b459..612e0e30d 100644 --- a/src/main/java/com/google/api/gax/retrying/TimedRetryAlgorithm.java +++ b/src/main/java/com/google/api/gax/retrying/TimedRetryAlgorithm.java @@ -56,8 +56,8 @@ public interface TimedRetryAlgorithm { * attempt. * * @param prevSettings previous attempt settings - * @return next attempt settings or {@code prevSettings} if the implementing algorithm does not - * provide specific settings for the next attempt + * @return next attempt settings or {@code null} if the implementing algorithm does not provide + * specific settings for the next attempt */ TimedAttemptSettings createNextAttempt(TimedAttemptSettings prevSettings); diff --git a/src/test/java/com/google/api/gax/retrying/DirectRetryingExecutorTest.java b/src/test/java/com/google/api/gax/retrying/DirectRetryingExecutorTest.java index b3d71bf23..03009d948 100644 --- a/src/test/java/com/google/api/gax/retrying/DirectRetryingExecutorTest.java +++ b/src/test/java/com/google/api/gax/retrying/DirectRetryingExecutorTest.java @@ -29,8 +29,8 @@ */ package com.google.api.gax.retrying; +import com.google.api.gax.core.CurrentMillisClock; import com.google.api.gax.core.RetrySettings; -import com.google.api.gax.core.SystemClock; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -42,7 +42,7 @@ protected RetryingExecutor getRetryingExecutor(RetrySettings retrySettin RetryAlgorithm retryAlgorithm = new RetryAlgorithm( getNoOpExceptionRetryAlgorithm(), - new ExponentialRetryAlgorithm(retrySettings, SystemClock.getDefaultClock())); + new ExponentialRetryAlgorithm(retrySettings, CurrentMillisClock.getDefaultClock())); return new DirectRetryingExecutor<>(retryAlgorithm); } From d3eabeca82ab630acc1c936ba50bd78cd7f44610 Mon Sep 17 00:00:00 2001 From: vam Date: Fri, 17 Mar 2017 16:08:11 -0700 Subject: [PATCH 12/15] fixed the test (failed because change of the contract for ExceptionRetryAlgorithm, which now returns null) --- .../google/api/gax/retrying/AbstractRetryingExecutorTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/com/google/api/gax/retrying/AbstractRetryingExecutorTest.java b/src/test/java/com/google/api/gax/retrying/AbstractRetryingExecutorTest.java index 3e9338e91..3d11e99a5 100644 --- a/src/test/java/com/google/api/gax/retrying/AbstractRetryingExecutorTest.java +++ b/src/test/java/com/google/api/gax/retrying/AbstractRetryingExecutorTest.java @@ -53,7 +53,7 @@ protected ExceptionRetryAlgorithm getNoOpExceptionRetryAlgorithm() { @Override public TimedAttemptSettings createNextAttempt( Throwable prevThrowable, TimedAttemptSettings prevSettings) { - return prevSettings; + return null; } @Override From 87caffd1cd75495061bd94a9fa47b0f835e6c852 Mon Sep 17 00:00:00 2001 From: vam Date: Fri, 17 Mar 2017 16:20:29 -0700 Subject: [PATCH 13/15] updated documentation for ScheduledExcecutorService --- .../com/google/api/gax/retrying/ScheduledRetryingExecutor.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/java/com/google/api/gax/retrying/ScheduledRetryingExecutor.java b/src/main/java/com/google/api/gax/retrying/ScheduledRetryingExecutor.java index cb68554e7..121308f81 100644 --- a/src/main/java/com/google/api/gax/retrying/ScheduledRetryingExecutor.java +++ b/src/main/java/com/google/api/gax/retrying/ScheduledRetryingExecutor.java @@ -41,8 +41,6 @@ /** * The retry executor which uses {@link ScheduledExecutorService} to schedule an attempt tasks. - * Unless a direct executor service is used, this handler will schedule attempts for an execution in - * another thread. * *

      * This class is thread-safe. From 7500a2da10fdce50725dd7fe776e51493b2c3448 Mon Sep 17 00:00:00 2001 From: vam Date: Mon, 20 Mar 2017 11:22:59 -0700 Subject: [PATCH 14/15] udated javadoc for ApiClock --- src/main/java/com/google/api/gax/core/ApiClock.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/google/api/gax/core/ApiClock.java b/src/main/java/com/google/api/gax/core/ApiClock.java index 90ccff877..da84d14b9 100644 --- a/src/main/java/com/google/api/gax/core/ApiClock.java +++ b/src/main/java/com/google/api/gax/core/ApiClock.java @@ -32,7 +32,7 @@ /** * An interface for getting the current value of a high-resolution time source, in nanoseconds. * - * Clocks other than NanoClock are typically used only for testing. + * Clocks other than {@link NanoClock} are typically used only for testing. * * This interface is required in addition to Java 8's Clock, because nanoTime is required to compare * values with io.grpc.CallOptions.getDeadlineNanoTime(). From db34591f618ca91c6d6ecd3800bffce0fbd5cbe2 Mon Sep 17 00:00:00 2001 From: vam Date: Mon, 20 Mar 2017 11:54:42 -0700 Subject: [PATCH 15/15] Updated docs and formatting --- src/main/java/com/google/api/gax/grpc/UnaryCallable.java | 2 +- src/main/java/com/google/api/gax/retrying/RetryingExecutor.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/google/api/gax/grpc/UnaryCallable.java b/src/main/java/com/google/api/gax/grpc/UnaryCallable.java index 0ec66586c..561adb085 100644 --- a/src/main/java/com/google/api/gax/grpc/UnaryCallable.java +++ b/src/main/java/com/google/api/gax/grpc/UnaryCallable.java @@ -29,8 +29,8 @@ */ package com.google.api.gax.grpc; -import com.google.api.gax.core.ApiClock; import com.google.api.gax.batching.BatchingSettings; +import com.google.api.gax.core.ApiClock; import com.google.api.gax.core.ApiFuture; import com.google.api.gax.core.NanoClock; import com.google.api.gax.core.RetrySettings; diff --git a/src/main/java/com/google/api/gax/retrying/RetryingExecutor.java b/src/main/java/com/google/api/gax/retrying/RetryingExecutor.java index 1a5bb0d82..38b4deac7 100644 --- a/src/main/java/com/google/api/gax/retrying/RetryingExecutor.java +++ b/src/main/java/com/google/api/gax/retrying/RetryingExecutor.java @@ -59,7 +59,7 @@ public interface RetryingExecutor { * attempt in the current thread or schedule it for an execution, using some sort of async * execution service. * - * @param retryingFuture the future previously returned by {@link #createFuture(Callable)}, and + * @param retryingFuture the future previously returned by {@link #createFuture(Callable)} and * reused for each subsequent attempt of same operation. */ void submit(RetryingFuture retryingFuture);