diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelServiceConfig.java b/core/src/main/java/io/grpc/internal/ManagedChannelServiceConfig.java index b6f7ae9f5c..e01bb1b524 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelServiceConfig.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelServiceConfig.java @@ -18,21 +18,17 @@ package io.grpc.internal; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; -import static com.google.common.base.Verify.verify; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.MoreObjects; import com.google.common.base.Objects; import com.google.common.base.Strings; import io.grpc.MethodDescriptor; -import io.grpc.Status.Code; import io.grpc.internal.RetriableStream.Throttle; import java.util.Collections; -import java.util.EnumSet; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Set; import javax.annotation.Nullable; /** @@ -256,21 +252,9 @@ final class ManagedChannelServiceConfig { "backoffMultiplier must be greater than 0: %s", backoffMultiplier); - List rawCodes = - ServiceConfigUtil.getRetryableStatusCodesFromRetryPolicy(retryPolicy); - checkNotNull(rawCodes, "rawCodes must be present"); - checkArgument(!rawCodes.isEmpty(), "rawCodes can't be empty"); - EnumSet codes = EnumSet.noneOf(Code.class); - // service config doesn't say if duplicates are allowed, so just accept them. - for (String rawCode : rawCodes) { - verify(!"OK".equals(rawCode), "rawCode can not be \"OK\""); - codes.add(Code.valueOf(rawCode)); - } - Set retryableStatusCodes = Collections.unmodifiableSet(codes); - return new RetryPolicy( maxAttempts, initialBackoffNanos, maxBackoffNanos, backoffMultiplier, - retryableStatusCodes); + ServiceConfigUtil.getRetryableStatusCodesFromRetryPolicy(retryPolicy)); } private static HedgingPolicy hedgingPolicy( @@ -287,19 +271,9 @@ final class ManagedChannelServiceConfig { checkArgument( hedgingDelayNanos >= 0, "hedgingDelay must not be negative: %s", hedgingDelayNanos); - List rawCodes = - ServiceConfigUtil.getNonFatalStatusCodesFromHedgingPolicy(hedgingPolicy); - checkNotNull(rawCodes, "rawCodes must be present"); - checkArgument(!rawCodes.isEmpty(), "rawCodes can't be empty"); - EnumSet codes = EnumSet.noneOf(Code.class); - // service config doesn't say if duplicates are allowed, so just accept them. - for (String rawCode : rawCodes) { - verify(!"OK".equals(rawCode), "rawCode can not be \"OK\""); - codes.add(Code.valueOf(rawCode)); - } - Set nonFatalStatusCodes = Collections.unmodifiableSet(codes); - - return new HedgingPolicy(maxAttempts, hedgingDelayNanos, nonFatalStatusCodes); + return new HedgingPolicy( + maxAttempts, hedgingDelayNanos, + ServiceConfigUtil.getNonFatalStatusCodesFromHedgingPolicy(hedgingPolicy)); } } } diff --git a/core/src/main/java/io/grpc/internal/ServiceConfigUtil.java b/core/src/main/java/io/grpc/internal/ServiceConfigUtil.java index cade9e1905..5196727445 100644 --- a/core/src/main/java/io/grpc/internal/ServiceConfigUtil.java +++ b/core/src/main/java/io/grpc/internal/ServiceConfigUtil.java @@ -18,18 +18,23 @@ package io.grpc.internal; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; +import static com.google.common.base.Verify.verify; import static com.google.common.math.LongMath.checkedAdd; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.MoreObjects; import com.google.common.base.Objects; +import com.google.common.base.VerifyException; +import io.grpc.Status; import io.grpc.internal.RetriableStream.Throttle; import java.text.ParseException; import java.util.ArrayList; import java.util.Collections; +import java.util.EnumSet; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Set; import java.util.concurrent.TimeUnit; import javax.annotation.Nullable; @@ -176,12 +181,43 @@ public final class ServiceConfigUtil { return getDouble(retryPolicy, RETRY_POLICY_BACKOFF_MULTIPLIER_KEY); } - @Nullable - static List getRetryableStatusCodesFromRetryPolicy(Map retryPolicy) { - if (!retryPolicy.containsKey(RETRY_POLICY_RETRYABLE_STATUS_CODES_KEY)) { - return null; + private static Set getStatusCodesFromList(List statuses) { + EnumSet codes = EnumSet.noneOf(Status.Code.class); + for (Object status : statuses) { + Status.Code code; + if (status instanceof Double) { + Double statusD = (Double) status; + int codeValue = statusD.intValue(); + verify((double) codeValue == statusD, "Status code %s is not integral", status); + code = Status.fromCodeValue(codeValue).getCode(); + verify(code.value() == statusD.intValue(), "Status code %s is not valid", status); + } else if (status instanceof String) { + try { + code = Status.Code.valueOf((String) status); + } catch (IllegalArgumentException iae) { + throw new VerifyException("Status code " + status + " is not valid", iae); + } + } else { + throw new VerifyException( + "Can not convert status code " + status + " to Status.Code, because it's type is " + + status.getClass()); + } + codes.add(code); } - return checkStringList(getList(retryPolicy, RETRY_POLICY_RETRYABLE_STATUS_CODES_KEY)); + return Collections.unmodifiableSet(codes); + } + + static Set getRetryableStatusCodesFromRetryPolicy(Map retryPolicy) { + verify( + retryPolicy.containsKey(RETRY_POLICY_RETRYABLE_STATUS_CODES_KEY), + "%s is required in retry policy", RETRY_POLICY_RETRYABLE_STATUS_CODES_KEY); + Set codes = + getStatusCodesFromList(getList(retryPolicy, RETRY_POLICY_RETRYABLE_STATUS_CODES_KEY)); + verify(!codes.isEmpty(), "%s must not be empty", RETRY_POLICY_RETRYABLE_STATUS_CODES_KEY); + verify( + !codes.contains(Status.Code.OK), + "%s must not contain OK", RETRY_POLICY_RETRYABLE_STATUS_CODES_KEY); + return codes; } @Nullable @@ -205,12 +241,16 @@ public final class ServiceConfigUtil { } } - @Nullable - static List getNonFatalStatusCodesFromHedgingPolicy(Map hedgingPolicy) { + static Set getNonFatalStatusCodesFromHedgingPolicy(Map hedgingPolicy) { if (!hedgingPolicy.containsKey(HEDGING_POLICY_NON_FATAL_STATUS_CODES_KEY)) { - return null; + return Collections.unmodifiableSet(EnumSet.noneOf(Status.Code.class)); } - return checkStringList(getList(hedgingPolicy, HEDGING_POLICY_NON_FATAL_STATUS_CODES_KEY)); + Set codes = + getStatusCodesFromList(getList(hedgingPolicy, HEDGING_POLICY_NON_FATAL_STATUS_CODES_KEY)); + verify( + !codes.contains(Status.Code.OK), + "%s must not contain OK", HEDGING_POLICY_NON_FATAL_STATUS_CODES_KEY); + return codes; } @Nullable diff --git a/examples/README.md b/examples/README.md index f0f587b672..d98fc0d72d 100644 --- a/examples/README.md +++ b/examples/README.md @@ -47,10 +47,7 @@ before trying out the examples. ```json "hedgingPolicy": { "maxAttempts": 3, - "hedgingDelay": "1s", - "nonFatalStatusCodes": [ - "UNAVAILABLE" - ] + "hedgingDelay": "1s" } ``` Then the latency summary in the client log is like the following diff --git a/examples/src/main/resources/io/grpc/examples/hedging/hedging_service_config.json b/examples/src/main/resources/io/grpc/examples/hedging/hedging_service_config.json index 54615f24d9..c3718626b9 100644 --- a/examples/src/main/resources/io/grpc/examples/hedging/hedging_service_config.json +++ b/examples/src/main/resources/io/grpc/examples/hedging/hedging_service_config.json @@ -10,10 +10,7 @@ "hedgingPolicy": { "maxAttempts": 3, - "hedgingDelay": "1s", - "nonFatalStatusCodes": [ - "UNAVAILABLE" - ] + "hedgingDelay": "1s" } } ],