diff --git a/java-bigquery/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryImpl.java b/java-bigquery/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryImpl.java index 586becb710b2..1273ba5fc3bc 100644 --- a/java-bigquery/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryImpl.java +++ b/java-bigquery/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryImpl.java @@ -655,6 +655,9 @@ private static void configureReadSettings( .retryOnRegEx(BigQueryErrorMessages.RetryRegExPatterns.RATE_LIMIT_EXCEEDED_REGEX) .build(); // retry config with Error Messages and RegEx for RateLimitExceeded Error + private static final Pattern ALREADY_EXISTS_JOB_PATTERN = + Pattern.compile(".*Already.*Exists:.*Job.*", Pattern.CASE_INSENSITIVE); + BigQueryImpl(BigQueryOptions options) { super(options); bigQueryRpc = options.getBigQueryRpcV2(); @@ -980,8 +983,8 @@ public com.google.api.services.bigquery.model.Job call() throws IOException { && createException.getCause() != null && createException.getCause().getMessage() != null) { - Pattern pattern = Pattern.compile(".*Already.*Exists:.*Job.*", Pattern.CASE_INSENSITIVE); - Matcher matcher = pattern.matcher(createException.getCause().getMessage()); + Matcher matcher = + ALREADY_EXISTS_JOB_PATTERN.matcher(createException.getCause().getMessage()); if (matcher.find()) { // If the Job ALREADY EXISTS, retrieve it. diff --git a/java-bigquery/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryAlgorithm.java b/java-bigquery/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryAlgorithm.java index 140f2c6eba3c..e1d357034273 100644 --- a/java-bigquery/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryAlgorithm.java +++ b/java-bigquery/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryAlgorithm.java @@ -28,7 +28,7 @@ import com.google.api.services.bigquery.model.ErrorProto; import com.google.api.services.bigquery.model.Job; import java.time.Duration; -import java.util.Iterator; +import java.util.Locale; import java.util.UUID; import java.util.concurrent.CancellationException; import java.util.logging.Level; @@ -70,13 +70,14 @@ public boolean shouldRetry( nextAttemptSettings == null ? Duration.ZERO : nextAttemptSettings.getRetryDelayDuration(); String errorMessage = previousThrowable != null ? previousThrowable.getMessage() : ""; - // Implementing shouldRetryBasedOnBigQueryRetryConfig so that we can retry exceptions based on - // the exception messages + // Short-circuit when createNextAttempt returned null or timing disallows retry so + // shouldRetryBasedOnResult and shouldRetryBasedOnBigQueryRetryConfig are not evaluated twice boolean shouldRetry = - (shouldRetryBasedOnResult(context, previousThrowable, previousResponse) + nextAttemptSettings != null + && shouldRetryBasedOnTiming(context, nextAttemptSettings) + && (shouldRetryBasedOnResult(context, previousThrowable, previousResponse) || shouldRetryBasedOnBigQueryRetryConfig( - previousThrowable, bigQueryRetryConfig, previousResponse)) - && shouldRetryBasedOnTiming(context, nextAttemptSettings); + previousThrowable, bigQueryRetryConfig, previousResponse)); if (LOG.isLoggable(Level.FINEST)) { LOG.log( @@ -116,23 +117,15 @@ private boolean shouldRetryBasedOnBigQueryRetryConfig( } if (errorDesc != null) { - errorDesc = errorDesc.toLowerCase(); // for case insensitive comparison - for (Iterator retriableMessages = - bigQueryRetryConfig.getRetriableErrorMessages().iterator(); - retriableMessages.hasNext(); ) { - if (errorDesc.contains( - retriableMessages - .next() - .toLowerCase())) { // Error message should be retried, implementing cases - // insensitive match + String lowerErrorDesc = errorDesc.toLowerCase(Locale.ROOT); + for (String retriableMessage : bigQueryRetryConfig.getLowerCaseRetriableErrorMessages()) { + if (lowerErrorDesc.contains(retriableMessage)) { return true; } } - // Check if there's a regex which matches the error message. This avoids too many regex - // matches which is expensive - for (Iterator retriableRegExes = bigQueryRetryConfig.getRetriableRegExes().iterator(); - retriableRegExes.hasNext(); ) { - if (matchRegEx(retriableRegExes.next(), errorDesc)) { + // Check if there's a pre-compiled regex which matches the error message. + for (Pattern retriablePattern : bigQueryRetryConfig.getCompiledRetriableRegExes()) { + if (retriablePattern.matcher(errorDesc).matches()) { return true; } } @@ -142,7 +135,7 @@ private boolean shouldRetryBasedOnBigQueryRetryConfig( public static boolean matchRegEx( String retriableRegEx, String errorDesc) { // cases insensitive match regex matching - return Pattern.matches(retriableRegEx.toLowerCase(), errorDesc.toLowerCase()); + return Pattern.compile(retriableRegEx, Pattern.CASE_INSENSITIVE).matcher(errorDesc).matches(); } /*Duplicating this method as it can not be inherited from the RetryAlgorithm due to the default access modifier*/ diff --git a/java-bigquery/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryConfig.java b/java-bigquery/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryConfig.java index 2e1f7c0bdaa8..738f582e37f7 100644 --- a/java-bigquery/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryConfig.java +++ b/java-bigquery/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryConfig.java @@ -17,15 +17,32 @@ import static com.google.common.base.Preconditions.checkNotNull; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import java.util.Locale; +import java.util.regex.Pattern; public class BigQueryRetryConfig { private final ImmutableSet retriableErrorMessages; private final ImmutableSet retriableRegExes; + private final ImmutableList lowerCaseRetriableErrorMessages; + private final ImmutableList compiledRetriableRegExes; private BigQueryRetryConfig(Builder builder) { retriableErrorMessages = builder.retriableErrorMessages.build(); retriableRegExes = builder.retriableRegExes.build(); + ImmutableList.Builder lowerMessagesBuilder = + ImmutableList.builderWithExpectedSize(retriableErrorMessages.size()); + for (String errorMessage : retriableErrorMessages) { + lowerMessagesBuilder.add(errorMessage.toLowerCase(Locale.ROOT)); + } + lowerCaseRetriableErrorMessages = lowerMessagesBuilder.build(); + ImmutableList.Builder patternsBuilder = + ImmutableList.builderWithExpectedSize(retriableRegExes.size()); + for (String regExPattern : retriableRegExes) { + patternsBuilder.add(Pattern.compile(regExPattern, Pattern.CASE_INSENSITIVE)); + } + compiledRetriableRegExes = patternsBuilder.build(); } public ImmutableSet getRetriableErrorMessages() { @@ -36,6 +53,14 @@ public ImmutableSet getRetriableRegExes() { return retriableRegExes; } + ImmutableList getLowerCaseRetriableErrorMessages() { + return lowerCaseRetriableErrorMessages; + } + + ImmutableList getCompiledRetriableRegExes() { + return compiledRetriableRegExes; + } + // BigQueryRetryConfig builder public static class Builder { private final ImmutableSet.Builder retriableErrorMessages = ImmutableSet.builder();