Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: Implemented BigQueryRetryAlgorithm to retry on the basis of the configured re-triable error messages #1426
feat: Implemented BigQueryRetryAlgorithm to retry on the basis of the configured re-triable error messages #1426
Changes from 29 commits
c48231d
e7c33e6
faaee99
dbcfa7d
e902f50
ab25cea
77a874d
ca153b8
cb15bc6
7774662
a271259
6d7e4bf
68299bf
480b13b
43752b3
0e99fa3
38f3977
40393b3
2e234b0
28fa870
67f4ce1
a986f37
3da3430
66fd067
a72a2f5
cc5f730
0648071
286e3f1
68c06e2
22b1706
84a3418
13e8132
8f5fc45
c5d6b70
2d21e11
592eea8
d448132
9833823
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Should we just name this
DEFAULT_RETRY_CONFIG
? It currently has ratelimitexceeded and we can potentially add more in the future.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.
sure, updated it
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.
I find this a little extraneous -- why do we need to specify every message we need to retry on? If we build a config, I imagine we just retry on all the specified error messages in the config.
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.
Sure Stephanie. As discussed, I have tried to create it on the lines of
com.google.cloud.ExceptionHandler
and this may give us additional flexibility to configure hooks likeretryOnStatus
orretryOnReason
later as necessary. Happy to refactor it as needed.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.
Can we add this also to
TableResult query(...)
methods? That should also allow you to verify idempotency usingrequestId
for jobs.query API.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.
Sure, I have added
BigQueryRetryHelper.runWithRetries
forTableResult query(...)
and have implemented testcasetestFastQueryRateLimitIdempotency
to test the idempotencyThere 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.
Okay great but I think we need it for the
jobs.insert
endpoint too (line 1269). There are 2 query paths here. The one you've updated is the "fast" query path (jobs.query).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.
Sure, I have implemented
BigQueryRetryHelper.runWithRetries
onQueryResponse waitForQueryResults
method, which is used byTableResult getQueryResults
method