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.

Sign up for GitHub

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

Jump to bottom

Reconsider calculation of time between leaser runs #675

Closed
pradn opened this issue Apr 27, 2022 · 2 comments · Fixed by #678
Closed

Reconsider calculation of time between leaser runs #675

pradn opened this issue Apr 27, 2022 · 2 comments · Fixed by #678
Assignees
@acocuzzo
Labels
api: pubsub Issues related to the googleapis/python-pubsub API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@pradn
Copy link
Contributor

pradn commented Apr 27, 2022 •
edited
Loading

It's currently random.uniform(0.0, ack_deadline * 0.9) here.

  1. We should probably have some minimum so it doesn't lease too frequently.
  2. The point of setting the max to ack_deadline * 0.9 is to leave enough wiggle room so the lease requests can be sent and processed by the server before the previous leases expire. We should probably make sure this time allotted to request-sending/server-processing to be some reasonable minimum (like 50 ms). No point in leaving wiggle room if the acks are going to expire by the time the new lease modacks arrive at the server.

This came up in the review of #662.
@product-auto-label product-auto-label bot added the api: pubsub Issues related to the googleapis/python-pubsub API. label Apr 27, 2022
@pradn pradn mentioned this issue Apr 27, 2022
Merged
4 tasks
@yoshi-automation yoshi-automation added triage me I really want to be triaged. 🚨 This issue needs some love. labels Apr 28, 2022
@acocuzzo acocuzzo added the type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. label May 4, 2022
@acocuzzo acocuzzo mentioned this issue May 4, 2022
Merged
4 tasks
@acocuzzo acocuzzo linked a pull request May 4, 2022 that will close this issue
fix: set min snooze on lease management to .01 sec #678
Merged
4 tasks
@acocuzzo acocuzzo reopened this May 4, 2022
@acocuzzo
Copy link
Contributor

acocuzzo commented May 4, 2022

PR #678 addresses issue #1, addressing 2. in a followup

@acocuzzo acocuzzo added the priority: p2 Moderately-important priority. Fix may not be included in next release. label May 4, 2022
@yoshi-automation yoshi-automation removed 🚨 This issue needs some love. triage me I really want to be triaged. labels May 4, 2022
@acocuzzo acocuzzo self-assigned this Aug 4, 2022
@acocuzzo acocuzzo mentioned this issue Oct 28, 2022
Merged
@acocuzzo
Copy link
Contributor

Fixed in: https://github.com/googleapis/python-pubsub/pull/816/files

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@acocuzzo acocuzzo

Labels
api: pubsub Issues related to the googleapis/python-pubsub API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: set min snooze on lease management to .01 sec acocuzzo/python-pubsub
3 participants
@pradn @yoshi-automation @acocuzzo