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: expose a maxDurationPerAckExtension setting for subscribers #893

Closed
hongalex opened this issue Feb 20, 2020 · 3 comments
Closed
Assignees
Labels
api: pubsub Issues related to the googleapis/nodejs-pubsub API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@hongalex
Copy link
Member

hongalex commented Feb 20, 2020

For languages that support lease management, we have bound the percentile distribution to 10s - 10m. It would be useful if the upper bound were configurable.

A user might process messages very slowly (8m to ack)
If the application fails 5s after receiving a message (whose ack deadline was set to 8m, because that's the deadline the percentile distribution gave), the user has to wait 7m55s for the message to be redelivered
Users should be able to set maxDurationPerAckExtension = 20s, which caps an individual modAck RPC to 20s. This would have the downside of causing more modack RPCs to be sent, but the upside that the redelivery time would only be 15s
This setting differs from maxExtensionMinutes, which defines the total amount of time we'll hold a message.

maxDurationPerAckExtension should be disabled if set to be 0 (or less), and should default to 0.

For an idea of how this is implemented, please refer to the Go implementation, Github PR.

@product-auto-label product-auto-label bot added the api: pubsub Issues related to the googleapis/nodejs-pubsub API. label Feb 20, 2020
@hongalex
Copy link
Member Author

From a discussion from the previous maintainer, it seems like the percentile distribution is only used when ackDeadline is not set. This is inconsistent with the other languages, so this might need to be removed at a later point (possibly bundled with another breaking change).

@feywind feywind added priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Feb 21, 2020
@hongalex hongalex changed the title pubsub: expose a maxExtensionPeriod setting for subscribers pubsub: expose a maxDurationPerAckExtension setting for subscribers Feb 28, 2020
@feywind
Copy link
Collaborator

feywind commented Mar 24, 2022

Moving to internal backlog.

@feywind feywind closed this as completed Mar 24, 2022
@hongalex
Copy link
Member Author

This is now exposed as SubscriberOptions.maxAckDeadline and corresponding minAckDeadline

minAckDeadline?: Duration;
maxAckDeadline?: Duration;

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 googleapis/nodejs-pubsub API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

2 participants