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

PubSub client is missing ABORTED from the list of retryable errors #1241

Closed
gracenoah opened this issue Dec 4, 2018 · 5 comments
Closed
Assignees
Labels
api: pubsub Issues related to the Pub/Sub API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@gracenoah
Copy link

The PubSub go client seems to have gotten out of sync in terms of the list of retryable errors, comparing with the Java client. It is missing "ABORTED".

Client

PubSub

Describe Your Environment

Ubuntu Docker on GKE

Expected Behavior

Retryable error is retried

Actual Behavior

Process dies

@gracenoah
Copy link
Author

diff --git pubsub/service.go pubsub/service.go
index 63d4aa4e..edf8df1a 100644
--- pubsub/service.go
+++ pubsub/service.go
@@ -68,7 +68,7 @@ func isRetryable(err error) bool {
                return true
        }
        switch s.Code() {
-       case codes.DeadlineExceeded, codes.Internal, codes.ResourceExhausted:
+       case codes.OK, codes.Canceled, codes.Unknown, codes.DeadlineExceeded, codes.Internal, codes.ResourceExhausted, codes.Aborted, codes.Unavailable, codes.DataLoss:
                return true
        case codes.Unavailable:
                return !strings.Contains(s.Message(), "Server shutdownNow invoked")

This would be the necessary change it seems. I'm not in a position to contribute at this moment.

@jeanbza
Copy link
Contributor

jeanbza commented Dec 4, 2018

@pongad @jba Do either of you guys happen to know whether this is an omission by design, or if it's unintentional? I might be forgetting something..

@jeanbza jeanbza added api: pubsub Issues related to the Pub/Sub API. status: investigating The issue is under investigation, which is determined to be non-trivial. labels Dec 4, 2018
@jeanbza jeanbza self-assigned this Dec 4, 2018
@jeanbza jeanbza added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. and removed status: investigating The issue is under investigation, which is determined to be non-trivial. labels Dec 4, 2018
@jeanbza jeanbza removed their assignment Dec 4, 2018
@pongad
Copy link
Contributor

pongad commented Dec 5, 2018

I can't remember the events but googleapis/google-cloud-java#2461 suggests pubsub team is OK with it :)

gopherbot pushed a commit that referenced this issue Dec 6, 2018
Part 1 of 2 commits related to #1241.

Change-Id: I52ba65b31d207cf559c085b00d679eb56533cff3
Reviewed-on: https://code-review.googlesource.com/c/36110
Reviewed-by: kokoro <[email protected]>
Reviewed-by: Michael Darakananda <[email protected]>
@jeanbza
Copy link
Contributor

jeanbza commented Feb 24, 2019

Pub/Sub now retries aborted.

@jeanbza jeanbza closed this as completed Feb 24, 2019
@jeanbza
Copy link
Contributor

jeanbza commented Feb 24, 2019

@jeanbza jeanbza self-assigned this Feb 24, 2019
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. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

3 participants