-
Notifications
You must be signed in to change notification settings - Fork 127
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.
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
fix: retry ExceptionHandler not retrying on IOException #3668
Conversation
…ion translation
…ion translation
…ion translation
…ion translation
…ion translation
…le thrown IOExceptions
google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/BigQueryImplTest.java
Show resolved
Hide resolved
google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryImpl.java
Outdated
Show resolved
Hide resolved
google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryImpl.java
Outdated
Show resolved
Hide resolved
google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryImpl.java
Outdated
Show resolved
Hide resolved
google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryImpl.java
Outdated
Show resolved
Hide resolved
google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryImpl.java
Outdated
Show resolved
Hide resolved
google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryImpl.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Nice work on such a massive PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more minor callout for delete behavior, but otherwise LGTM.
Also, "SkipExceptionTranslation" may forever be burned into my retinas, but thanks for slogging through this refactor.
google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryImpl.java
Show resolved
Hide resolved
Thanks for the review. I've comment on the |
This PR fix an issue where the retry algorithm did not retrying based on the BIGQUERY_EXCEPTION_HANDLER configuration. This is caused by HttpBigQueryRpc translating IOExceptions into BigQueryException does not match the configurations set by BIGQUERY_EXCEPTION_HANDLER. To resolve this, we implement new internal methods (.*SkipExceptionTranslation) in HttpBigQueryRpc which do not translate the IOException. All retryable calls to HttpBigQueryRpc are updated to use these new methods.
In addition,
Fixes b/394167052 ☕️