From 70676418202bc6fdaceccdc6127824a29343f10e Mon Sep 17 00:00:00 2001 From: stephwang Date: Mon, 16 Aug 2021 12:45:13 -0400 Subject: [PATCH 1/5] fix: add retry logging for BigQueryRetryAlgorithm.java --- .../bigquery/BigQueryRetryAlgorithm.java | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryAlgorithm.java b/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryAlgorithm.java index c694e2c06..e2410bc69 100644 --- a/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryAlgorithm.java +++ b/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryAlgorithm.java @@ -27,7 +27,10 @@ import com.google.api.gax.retrying.TimedRetryAlgorithmWithContext; import java.util.Iterator; import java.util.concurrent.CancellationException; +import java.util.logging.Level; +import java.util.logging.Logger; import java.util.regex.Pattern; +import org.threeten.bp.Duration; public class BigQueryRetryAlgorithm extends RetryAlgorithm { private final BigQueryRetryConfig bigQueryRetryConfig; @@ -36,6 +39,8 @@ public class BigQueryRetryAlgorithm extends RetryAlgorithm private final ResultRetryAlgorithmWithContext resultAlgorithmWithContext; private final TimedRetryAlgorithmWithContext timedAlgorithmWithContext; + private static final Logger LOG = Logger.getLogger(BigQueryRetryAlgorithm.class.getName()); + public BigQueryRetryAlgorithm( ResultRetryAlgorithm resultAlgorithm, TimedRetryAlgorithm timedAlgorithm, @@ -55,6 +60,22 @@ public boolean shouldRetry( ResponseT previousResponse, TimedAttemptSettings nextAttemptSettings) throws CancellationException { + // Log retry info + int attemptCount = nextAttemptSettings == null ? 0 : nextAttemptSettings.getAttemptCount(); + Duration retryDelay = + nextAttemptSettings == null ? Duration.ZERO : nextAttemptSettings.getRetryDelay(); + + if (LOG.isLoggable(Level.FINEST)) { + LOG.log( + Level.FINEST, + "Retrying with:\n{0}\n{1}\n{2}", + new Object[] { + "BigQuery attemptCount: " + attemptCount, + "BigQuery delay: " + retryDelay, + "BigQuery retriableException: " + previousThrowable + }); + } + // Implementing shouldRetryBasedOnBigQueryRetryConfig so that we can retry exceptions based on // the exception messages return (shouldRetryBasedOnResult(context, previousThrowable, previousResponse) From 7ba63ae362d22f7292c831568ffa2bef2bbfee63 Mon Sep 17 00:00:00 2001 From: Prashant Mishra Date: Tue, 17 Aug 2021 10:09:00 +0530 Subject: [PATCH 2/5] Added shouldRetry and Error Message to Logging --- .../bigquery/BigQueryRetryAlgorithm.java | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryAlgorithm.java b/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryAlgorithm.java index e2410bc69..9924ee7c1 100644 --- a/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryAlgorithm.java +++ b/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryAlgorithm.java @@ -65,22 +65,28 @@ public boolean shouldRetry( Duration retryDelay = nextAttemptSettings == null ? Duration.ZERO : nextAttemptSettings.getRetryDelay(); + // Implementing shouldRetryBasedOnBigQueryRetryConfig so that we can retry exceptions based on + // the exception messages + boolean shouldRetry = + (shouldRetryBasedOnResult(context, previousThrowable, previousResponse) + || shouldRetryBasedOnBigQueryRetryConfig(previousThrowable, bigQueryRetryConfig)) + && shouldRetryBasedOnTiming(context, nextAttemptSettings); + if (LOG.isLoggable(Level.FINEST)) { LOG.log( Level.FINEST, - "Retrying with:\n{0}\n{1}\n{2}", + "Retrying with:\n{0}\n{1}\n{2}\n{4}\n{5}", new Object[] { "BigQuery attemptCount: " + attemptCount, "BigQuery delay: " + retryDelay, - "BigQuery retriableException: " + previousThrowable + "BigQuery retriableException: " + previousThrowable, + "BigQuery shouldRetry: " + shouldRetry, + "BigQuery previousThrowable.getMessage: " + previousThrowable != null + ? previousThrowable.getMessage() + : previousThrowable }); } - - // Implementing shouldRetryBasedOnBigQueryRetryConfig so that we can retry exceptions based on - // the exception messages - return (shouldRetryBasedOnResult(context, previousThrowable, previousResponse) - || shouldRetryBasedOnBigQueryRetryConfig(previousThrowable, bigQueryRetryConfig)) - && shouldRetryBasedOnTiming(context, nextAttemptSettings); + return shouldRetry; } private boolean shouldRetryBasedOnBigQueryRetryConfig( From 2e09b57405b663d84b6af9c198bf8283a0ed5dd3 Mon Sep 17 00:00:00 2001 From: Prashant Mishra Date: Tue, 17 Aug 2021 20:55:36 +0530 Subject: [PATCH 3/5] Added shouldRetry and Error Message to Logging --- .../java/com/google/cloud/bigquery/BigQueryRetryAlgorithm.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryAlgorithm.java b/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryAlgorithm.java index 9924ee7c1..974c2732b 100644 --- a/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryAlgorithm.java +++ b/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryAlgorithm.java @@ -75,7 +75,7 @@ public boolean shouldRetry( if (LOG.isLoggable(Level.FINEST)) { LOG.log( Level.FINEST, - "Retrying with:\n{0}\n{1}\n{2}\n{4}\n{5}", + "Retrying with:\n{0}\n{1}\n{2}\n{3}\n{4}", new Object[] { "BigQuery attemptCount: " + attemptCount, "BigQuery delay: " + retryDelay, From 58c338c762335ef72f918a1ebca4da08a7011182 Mon Sep 17 00:00:00 2001 From: stephwang Date: Tue, 17 Aug 2021 15:49:44 -0400 Subject: [PATCH 4/5] update based on comments - to add in retried method (enclosingMethod) --- .../bigquery/BigQueryRetryAlgorithm.java | 9 ++++---- .../cloud/bigquery/BigQueryRetryHelper.java | 23 ++++++++++++++++++- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryAlgorithm.java b/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryAlgorithm.java index 974c2732b..abe7b0090 100644 --- a/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryAlgorithm.java +++ b/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryAlgorithm.java @@ -64,6 +64,7 @@ public boolean shouldRetry( int attemptCount = nextAttemptSettings == null ? 0 : nextAttemptSettings.getAttemptCount(); Duration retryDelay = nextAttemptSettings == null ? Duration.ZERO : nextAttemptSettings.getRetryDelay(); + String errorMessage = previousThrowable != null ? previousThrowable.getMessage() : ""; // Implementing shouldRetryBasedOnBigQueryRetryConfig so that we can retry exceptions based on // the exception messages @@ -72,18 +73,16 @@ public boolean shouldRetry( || shouldRetryBasedOnBigQueryRetryConfig(previousThrowable, bigQueryRetryConfig)) && shouldRetryBasedOnTiming(context, nextAttemptSettings); - if (LOG.isLoggable(Level.FINEST)) { + if (LOG.isLoggable(Level.INFO)) { LOG.log( - Level.FINEST, + Level.INFO, "Retrying with:\n{0}\n{1}\n{2}\n{3}\n{4}", new Object[] { "BigQuery attemptCount: " + attemptCount, "BigQuery delay: " + retryDelay, "BigQuery retriableException: " + previousThrowable, "BigQuery shouldRetry: " + shouldRetry, - "BigQuery previousThrowable.getMessage: " + previousThrowable != null - ? previousThrowable.getMessage() - : previousThrowable + "BigQuery previousThrowable.getMessage: " + errorMessage }); } return shouldRetry; diff --git a/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryHelper.java b/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryHelper.java index 7c1e0e592..78cddc341 100644 --- a/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryHelper.java +++ b/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryHelper.java @@ -16,13 +16,24 @@ package com.google.cloud.bigquery; import com.google.api.core.ApiClock; -import com.google.api.gax.retrying.*; +import com.google.api.gax.retrying.DirectRetryingExecutor; +import com.google.api.gax.retrying.ExponentialRetryAlgorithm; +import com.google.api.gax.retrying.ResultRetryAlgorithm; +import com.google.api.gax.retrying.RetryAlgorithm; +import com.google.api.gax.retrying.RetrySettings; +import com.google.api.gax.retrying.RetryingExecutor; +import com.google.api.gax.retrying.RetryingFuture; +import com.google.api.gax.retrying.TimedRetryAlgorithm; import com.google.cloud.RetryHelper; import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; +import java.util.logging.Level; +import java.util.logging.Logger; public class BigQueryRetryHelper extends RetryHelper { + private static final Logger LOG = Logger.getLogger(BigQueryRetryHelper.class.getName()); + public static V runWithRetries( Callable callable, RetrySettings retrySettings, @@ -60,6 +71,16 @@ private static V run( // BigQueryRetryAlgorithm retries considering bigQueryRetryConfig RetryingExecutor executor = new DirectRetryingExecutor<>(retryAlgorithm); + // Log retry info + if (LOG.isLoggable(Level.INFO)) { + LOG.log( + Level.INFO, + "Retrying with:\n{0}", + new Object[] { + "BigQuery retried method: " + callable.getClass().getEnclosingMethod().getName(), + }); + } + RetryingFuture retryingFuture = executor.createFuture(callable); executor.submit(retryingFuture); return retryingFuture.get(); From 6d6c8d974f7bf393ba5d894938e202b1fe3b36ec Mon Sep 17 00:00:00 2001 From: stephwang Date: Tue, 17 Aug 2021 15:50:30 -0400 Subject: [PATCH 5/5] set log level to FINEST --- .../com/google/cloud/bigquery/BigQueryRetryAlgorithm.java | 4 ++-- .../java/com/google/cloud/bigquery/BigQueryRetryHelper.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryAlgorithm.java b/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryAlgorithm.java index abe7b0090..756377d21 100644 --- a/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryAlgorithm.java +++ b/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryAlgorithm.java @@ -73,9 +73,9 @@ public boolean shouldRetry( || shouldRetryBasedOnBigQueryRetryConfig(previousThrowable, bigQueryRetryConfig)) && shouldRetryBasedOnTiming(context, nextAttemptSettings); - if (LOG.isLoggable(Level.INFO)) { + if (LOG.isLoggable(Level.FINEST)) { LOG.log( - Level.INFO, + Level.FINEST, "Retrying with:\n{0}\n{1}\n{2}\n{3}\n{4}", new Object[] { "BigQuery attemptCount: " + attemptCount, diff --git a/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryHelper.java b/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryHelper.java index 78cddc341..623228d6c 100644 --- a/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryHelper.java +++ b/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryRetryHelper.java @@ -72,9 +72,9 @@ private static V run( RetryingExecutor executor = new DirectRetryingExecutor<>(retryAlgorithm); // Log retry info - if (LOG.isLoggable(Level.INFO)) { + if (LOG.isLoggable(Level.FINEST)) { LOG.log( - Level.INFO, + Level.FINEST, "Retrying with:\n{0}", new Object[] { "BigQuery retried method: " + callable.getClass().getEnclosingMethod().getName(),