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] make the keepalive robuster #382

Closed
tmatsuo opened this issue Sep 23, 2016 · 3 comments
Closed

[pubsub] make the keepalive robuster #382

tmatsuo opened this issue Sep 23, 2016 · 3 comments
Assignees
Labels
api: pubsub Issues related to the Pub/Sub API. 🚨 This issue needs some love. triage me I really want to be triaged. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@tmatsuo
Copy link
Contributor

tmatsuo commented Sep 23, 2016

I have just migrated from the legacy client code to the newer pubsub client. In that project, I'm using pubsub for task queuing. Tasks can take from 30 seconds, or up to 8 hours.

I used to extend the ackDeadline within my code, with 15 seconds grace period and 3 times retries.

Now we're seeing significantly more failures on extending the ackDeadline.

I briefly looked at the source code of the current library and found the code below:

keepAlivePeriod := po.ackDeadline - 5*time.Second
ackTicker := time.NewTicker(keepAlivePeriod / 2) // Stopped in it.Stop

And keepalive retries just one time when modifyAckDeadline fails.

If I'm not mistaken, if the first attempt for modifying the ackDeadline fails with somewhat long latency (for example, somehow the request hangs for 5 seconds and fails), then it fails to modify the ackDeadline, thus we get the same message again.

Here are my suggestions for making it robuster:

  • Retry few more times (or configurable retryCount)
  • Configurable keepAlivePeriod (or automatic calculate more appropriate value)
@tmatsuo tmatsuo changed the title pubsub: make the keepalive more robust [pubsub] make the keepalive robuster Feb 14, 2017
@tmatsuo
Copy link
Contributor Author

tmatsuo commented Feb 14, 2017

Ping, no news?

@jba
Copy link
Contributor

jba commented Feb 14, 2017

Sorry, we are working on a greatly improved pull implementation, so we haven't looked carefully at this yet.

@jba jba self-assigned this Feb 14, 2017
@jba jba added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. api: pubsub Issues related to the Pub/Sub API. labels Feb 14, 2017
@tmatsuo
Copy link
Contributor Author

tmatsuo commented Feb 16, 2017

@jba Alright, then maybe you can close this issue as long as you consider this problem when designing and implementing the new one.

@jba jba closed this as completed Jun 7, 2017
@yoshi-automation yoshi-automation added triage me I really want to be triaged. 🚨 This issue needs some love. labels Apr 6, 2020
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. 🚨 This issue needs some love. triage me I really want to be triaged. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

3 participants