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

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.

@hongalex hongalex requested a review from shollyman June 24, 2021 17:30
@hongalex hongalex requested review from a team as code owners June 24, 2021 17:30
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jun 24, 2021
@product-auto-label product-auto-label bot added the api: pubsub Issues related to the Pub/Sub API. label Jun 24, 2021
@hongalex hongalex added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 24, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 24, 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.

@hongalex hongalex added the automerge Merge the pull request once unit tests and other checks pass. label Jun 29, 2021
@gcf-merge-on-green gcf-merge-on-green bot merged commit 7076fef into googleapis:master Jun 29, 2021
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Jun 29, 2021
@hongalex hongalex deleted the fix-pubsub-goaway branch July 7, 2021 18:46
gcf-merge-on-green bot pushed a commit that referenced this pull request 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
Labels
api: pubsub Issues related to the Pub/Sub API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pubsub: non-retryable rpc errors in subscription Receive() method
3 participants