Skip to content

perf(bigquery): [T1-18] short-circuit duplicate retry evaluation and pre-compile regexes in BigQueryRetryAlgorithm #23

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lqiu96 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

perf(bigquery): [T1-18] short-circuit duplicate retry evaluation and pre-compile regexes in BigQueryRetryAlgorithm #23

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions ...-bigquery/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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.
Expand Down
35 changes: 14 additions & 21 deletions ...google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryAlgorithm.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -116,23 +117,15 @@ private boolean shouldRetryBasedOnBigQueryRetryConfig(
}

if (errorDesc != null) {
errorDesc = errorDesc.toLowerCase(); // for case insensitive comparison
for (Iterator<String> 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<String> 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;
}
}
Expand All @@ -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*/
Expand Down
25 changes: 25 additions & 0 deletions ...ry/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> retriableErrorMessages;
private final ImmutableSet<String> retriableRegExes;
private final ImmutableList<String> lowerCaseRetriableErrorMessages;
private final ImmutableList<Pattern> compiledRetriableRegExes;

private BigQueryRetryConfig(Builder builder) {
retriableErrorMessages = builder.retriableErrorMessages.build();
retriableRegExes = builder.retriableRegExes.build();
ImmutableList.Builder<String> lowerMessagesBuilder =
ImmutableList.builderWithExpectedSize(retriableErrorMessages.size());
for (String errorMessage : retriableErrorMessages) {
lowerMessagesBuilder.add(errorMessage.toLowerCase(Locale.ROOT));
}
lowerCaseRetriableErrorMessages = lowerMessagesBuilder.build();
ImmutableList.Builder<Pattern> patternsBuilder =
ImmutableList.builderWithExpectedSize(retriableRegExes.size());
for (String regExPattern : retriableRegExes) {
patternsBuilder.add(Pattern.compile(regExPattern, Pattern.CASE_INSENSITIVE));
}
compiledRetriableRegExes = patternsBuilder.build();
}

public ImmutableSet<String> getRetriableErrorMessages() {
Expand All @@ -36,6 +53,14 @@ public ImmutableSet<String> getRetriableRegExes() {
return retriableRegExes;
}

ImmutableList<String> getLowerCaseRetriableErrorMessages() {
return lowerCaseRetriableErrorMessages;
}

ImmutableList<Pattern> getCompiledRetriableRegExes() {
return compiledRetriableRegExes;
}

// BigQueryRetryConfig builder
public static class Builder {
private final ImmutableSet.Builder<String> retriableErrorMessages = ImmutableSet.builder();
Expand Down
Loading