From c9cca05ca81c9d3e9197e8f3aafda837d17d2db9 Mon Sep 17 00:00:00 2001 From: Shin Fan Date: Wed, 30 Mar 2016 11:26:30 -0700 Subject: [PATCH] Flatten RetryParams by removing BackoffParams layer. --- .../google/api/gax/core/BackoffParams.java | 81 ------------------- .../com/google/api/gax/core/RetryParams.java | 34 +++++++- .../google/api/gax/grpc/RetryingCallable.java | 12 +-- .../google/api/gax/grpc/ApiCallableTest.java | 18 ++--- 4 files changed, 44 insertions(+), 101 deletions(-) delete mode 100644 src/main/java/com/google/api/gax/core/BackoffParams.java diff --git a/src/main/java/com/google/api/gax/core/BackoffParams.java b/src/main/java/com/google/api/gax/core/BackoffParams.java deleted file mode 100644 index ffda52097..000000000 --- a/src/main/java/com/google/api/gax/core/BackoffParams.java +++ /dev/null @@ -1,81 +0,0 @@ -/* - * 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 com.google.auto.value.AutoValue; - -import org.joda.time.Duration; - -/** - * {@code BackoffParams} encapsulates parameters for exponential backoff. - */ -@AutoValue -public abstract class BackoffParams { - public abstract Duration getInitialDelay(); - - public abstract double getDelayMultiplier(); - - public abstract Duration getMaxDelay(); - - public static Builder newBuilder() { - return new AutoValue_BackoffParams.Builder(); - } - - public Builder toBuilder() { - return new AutoValue_BackoffParams.Builder(this); - } - - @AutoValue.Builder - public abstract static class Builder { - public abstract Builder setInitialDelay(Duration initialDelayDuration); - - public abstract Builder setDelayMultiplier(double delayMultiplier); - - public abstract Builder setMaxDelay(Duration maxDelayDuration); - - abstract BackoffParams autoBuild(); - - public BackoffParams build() { - BackoffParams backoff = autoBuild(); - if (backoff.getInitialDelay().getMillis() < 0) { - throw new IllegalStateException("initial delay must not be negative"); - } - if (backoff.getDelayMultiplier() < 1.0) { - throw new IllegalStateException("delay multiplier must be at least 1"); - } - if (backoff.getMaxDelay().compareTo(backoff.getInitialDelay()) < 0) { - throw new IllegalStateException("max delay must not be smaller than initial delay"); - } - return backoff; - } - } -} diff --git a/src/main/java/com/google/api/gax/core/RetryParams.java b/src/main/java/com/google/api/gax/core/RetryParams.java index 9a2621ef8..c5ae88862 100644 --- a/src/main/java/com/google/api/gax/core/RetryParams.java +++ b/src/main/java/com/google/api/gax/core/RetryParams.java @@ -43,12 +43,18 @@ */ @AutoValue public abstract class RetryParams { - public abstract BackoffParams getRetryBackoff(); - public abstract BackoffParams getTimeoutBackoff(); public abstract Duration getTotalTimeout(); + public abstract Duration getInitialRetryDelay(); + public abstract double getRetryDelayMultiplier(); + public abstract Duration getMaxRetryDelay(); + + public abstract Duration getInitialRpcTimeout(); + public abstract double getRpcTimeoutMultiplier(); + public abstract Duration getMaxRpcTimeout(); + public static Builder newBuilder() { return new AutoValue_RetryParams.Builder(); } @@ -59,9 +65,14 @@ public Builder toBuilder() { @AutoValue.Builder public abstract static class Builder { - public abstract Builder setRetryBackoff(BackoffParams retryBackoff); - public abstract Builder setTimeoutBackoff(BackoffParams timeoutBackoff); + public abstract Builder setInitialRetryDelay(Duration initialDelay); + public abstract Builder setRetryDelayMultiplier(double multiplier); + public abstract Builder setMaxRetryDelay(Duration maxDelay); + + public abstract Builder setInitialRpcTimeout(Duration initialTimeout); + public abstract Builder setRpcTimeoutMultiplier(double multiplier); + public abstract Builder setMaxRpcTimeout(Duration maxTimeout); public abstract Builder setTotalTimeout(Duration totalTimeout); @@ -72,6 +83,21 @@ public RetryParams build() { if (params.getTotalTimeout().getMillis() < 0) { throw new IllegalStateException("total timeout must not be negative"); } + if (params.getInitialRetryDelay().getMillis() < 0) { + throw new IllegalStateException("initial retry delay must not be negative"); + } + if (params.getInitialRpcTimeout().getMillis() < 0) { + throw new IllegalStateException("initial rpc timeout must not be negative"); + } + if (params.getRetryDelayMultiplier() < 1.0 || params.getRpcTimeoutMultiplier() < 1.0) { + throw new IllegalStateException("multiplier must be at least 1"); + } + if (params.getMaxRetryDelay().compareTo(params.getInitialRetryDelay()) < 0) { + throw new IllegalStateException("max retry delay must not be shorter than initial delay"); + } + if (params.getMaxRpcTimeout().compareTo(params.getInitialRpcTimeout()) < 0) { + throw new IllegalStateException("max rpc timeout must not be shorter than initial timeout"); + } return params; } } 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 1733361d3..b3d4a3207 100644 --- a/src/main/java/com/google/api/gax/grpc/RetryingCallable.java +++ b/src/main/java/com/google/api/gax/grpc/RetryingCallable.java @@ -75,8 +75,8 @@ public ListenableFuture futureCall(CallContext context) { new Retryer( context, result, - retryParams.getRetryBackoff().getInitialDelay(), - retryParams.getTimeoutBackoff().getInitialDelay(), + retryParams.getInitialRetryDelay(), + retryParams.getInitialRpcTimeout(), null); retryer.run(); return result; @@ -138,17 +138,17 @@ public void onFailure(Throwable throwable) { } long newRetryDelay = (long) (retryDelay.getMillis() * - retryParams.getRetryBackoff().getDelayMultiplier()); + retryParams.getRetryDelayMultiplier()); newRetryDelay = Math.min(newRetryDelay, - retryParams.getRetryBackoff().getMaxDelay().getMillis()); + retryParams.getMaxRetryDelay().getMillis()); long newRpcTimeout = (long) (rpcTimeout.getMillis() * - retryParams.getTimeoutBackoff().getDelayMultiplier()); + retryParams.getRpcTimeoutMultiplier()); newRpcTimeout = Math.min(newRpcTimeout, - retryParams.getTimeoutBackoff().getMaxDelay().getMillis()); + retryParams.getMaxRpcTimeout().getMillis()); long randomRetryDelay = ThreadLocalRandom.current().nextLong(retryDelay.getMillis()); Retryer retryer = new Retryer(context, result, diff --git a/src/test/java/com/google/api/gax/grpc/ApiCallableTest.java b/src/test/java/com/google/api/gax/grpc/ApiCallableTest.java index b8240fb12..ddd5abb93 100644 --- a/src/test/java/com/google/api/gax/grpc/ApiCallableTest.java +++ b/src/test/java/com/google/api/gax/grpc/ApiCallableTest.java @@ -31,7 +31,6 @@ package com.google.api.gax.grpc; -import com.google.api.gax.core.BackoffParams; import com.google.api.gax.core.RetryParams; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Lists; @@ -61,6 +60,7 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledThreadPoolExecutor; + /** * Tests for {@link ApiCallable}. */ @@ -71,17 +71,15 @@ public class ApiCallableTest { private static final RetryParams testRetryParams; static { - BackoffParams backoff = - BackoffParams.newBuilder() - .setInitialDelay(Duration.millis(2L)) - .setDelayMultiplier(1) - .setMaxDelay(Duration.millis(2L)) - .build(); testRetryParams = RetryParams.newBuilder() - .setRetryBackoff(backoff) - .setTimeoutBackoff(backoff) - .setTotalTimeout(Duration.millis(100L)) + .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(); }