Skip to content
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

Added custom retry with backoff for error not covered in BQ client (4… #1174

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

sechegaray
Copy link
Contributor

…00, please retry with backoff)

@sechegaray sechegaray force-pushed the fix/bigquery-retry-batch-job branch from 4e7ef71 to 2a4dcd4 Compare October 24, 2022 23:45
@sechegaray sechegaray added the build Trigger unit test build label Oct 24, 2022
@sechegaray sechegaray force-pushed the fix/bigquery-retry-batch-job branch from 2a4dcd4 to 7e7298a Compare October 24, 2022 23:50
@sechegaray sechegaray force-pushed the fix/bigquery-retry-batch-job branch from 7e7298a to de02031 Compare October 24, 2022 23:55

// Wait for the query to complete
queryJob.waitFor();
final String retryableStringPattern = "Retrying the job with back-off";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: make this a constant

List<Function<BigQueryException, Boolean>> retryRules = new ArrayList<>();
retryRules.add(
(BigQueryException e) -> e.getCode() == 400
&& (e.getMessage().contains(retryableStringPattern) || e.getReason().contains(retryableStringPattern))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the methods on BigQueryException like getMessage() and getReason() guaranteed to be non null?

@@ -169,6 +173,46 @@ public void run(ActionContext context) throws Exception {
context.getMetrics().gauge(RECORDS_PROCESSED, rows);
}

/**
* Executes Query with added retry rules following:
* https://cloud.google.com/bigquery/sla
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should also apply to BigQuery Sink plugin and may be even replication BQ plugins? Or for the time being it is only done for execute? It would be good to put this in a different class if it is applicable for other plugins.

*/
private Job executeQueryJobWithCustomRetry(BigQuery bigQuery, QueryJobConfiguration queryConfig,
List<Function<BigQueryException, Boolean>> retryRules) throws Exception {
// The longest amount of time to wait in-between retries.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: mention the timeunit

return queryJob;
} catch (BigQueryException bigQueryException) {
if (retries >= maxRetries) {
LOG.error("Run out of retries while executing query with backoff.");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Ran out of...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1


int retries = 0;

while (true) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a unit test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Trigger unit test build
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants