Skip to content

Commit

Permalink
feat(bigquery): widen retry predicate (#6976)
Browse files Browse the repository at this point in the history
This PR adds 500,504 http response codes for the default retry predicate on unary retries.

This doesn't introduce job-level retries (jobs must be recreated wholly, they can't be effectively restarted), so the primary risk of this change is terminal job state propagating into the HTTP response code of one of the polling methods (e.g. job.getQueryResults, jobs.get, etc).  In this situation, job execution may appear to hang indefinitely or until context cancellation/expiry.

Related: https://togithub.com/googleapis/google-cloud-go/issues/5248
  • Loading branch information
shollyman authored Nov 3, 2022
1 parent 0e15a92 commit 753b751
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 7 deletions.
18 changes: 12 additions & 6 deletions bigquery/bigquery.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,12 +203,16 @@ func runWithRetryExplicit(ctx context.Context, call func() error, allowedReasons
var (
defaultRetryReasons = []string{"backendError", "rateLimitExceeded"}
jobRetryReasons = []string{"backendError", "rateLimitExceeded", "internalError"}
retry5xxCodes = []int{
http.StatusInternalServerError,
http.StatusBadGateway,
http.StatusServiceUnavailable,
http.StatusGatewayTimeout,
}
)

// This is the correct definition of retryable according to the BigQuery team. It
// also considers 502 ("Bad Gateway") and 503 ("Service Unavailable") errors
// retryable; these are returned by systems between the client and the BigQuery
// service.
// retryableError is the unary retry predicate for this library. In addition to structured error
// reasons, it specifies some HTTP codes (500, 502, 503, 504) and network/transport reasons.
func retryableError(err error, allowedReasons []string) bool {
if err == nil {
return false
Expand Down Expand Up @@ -237,8 +241,10 @@ func retryableError(err error, allowedReasons []string) bool {
}
}
}
if e.Code == http.StatusServiceUnavailable || e.Code == http.StatusBadGateway {
return true
for _, code := range retry5xxCodes {
if e.Code == code {
return true
}
}
case *url.Error:
retryable := []string{"connection refused", "connection reset"}
Expand Down
2 changes: 1 addition & 1 deletion bigquery/bigquery_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func TestRetryableErrors(t *testing.T) {
&googleapi.Error{
Code: http.StatusInternalServerError,
},
false,
true,
},
{
"internal w/backend reason",
Expand Down

0 comments on commit 753b751

Please sign in to comment.