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

fix(pubsub): retry GOAWAY errors #4313

Merged
merged 7 commits into from Jun 29, 2021

Conversation

@hongalex
Copy link
Member

@hongalex hongalex commented Jun 24, 2021

The frontend server for Pub/Sub might occasionally emit GOAWAY errors which are currently not retried. This is not unique to the Go client, though the categorization as UNKNOWN vs UNVAILABLE is a golang-GRPC issue. Although UNKNOWN should not generally be retried, this will unblock users of Receive until the grpc library can be changed. See internal cl/377393940 for a similar fix in another Go library.

Fixes #4257.

tmdiep
tmdiep approved these changes Jun 29, 2021
@@ -62,6 +62,14 @@ func (r *defaultRetryer) Retry(err error) (pause time.Duration, shouldRetry bool
return r.bo.Pause(), true
}
return 0, false
case codes.Unknown:
// Retry GOAWAY, see https://github.com/googleapis/google-cloud-go/issues/4257.
isGoaway := strings.Contains(s.Message(), "closing error reading from server: EOF") &&
Copy link
Contributor

@tmdiep tmdiep Jun 29, 2021

Choose a reason for hiding this comment

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

Just checking: does the error message contain exactly "closing error reading from server..."? I looked at the linked issue and that phrase isn't prefixed by "closing", so it may not match.

Loading

@gcf-merge-on-green gcf-merge-on-green bot merged commit 7076fef into googleapis:master Jun 29, 2021
4 checks passed
Loading
@hongalex hongalex deleted the fix-pubsub-goaway branch Jul 7, 2021
gcf-merge-on-green bot pushed a commit that referenced this issue Jul 7, 2021
The previous iteration of this PR, #4313, only retried GOAWAY in conjunction with an EOF string, which doesn't capture all GOAWAY errors. Now, all error messages with `GOAWAY` and error code `UNKNOWN` are retried.

Fixes #4257
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants