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): respect streamAckDeadlineSeconds with MaxExtensionPeriod #3367

Merged
merged 5 commits into from Dec 7, 2020

Conversation

hongalex
Copy link
Member

@hongalex hongalex commented Dec 3, 2020

StreamAckDeadlineSeconds comes into effect when a stream shuts down prior to message modack (which the client library does automatically when a message is received). Users may want to lower this bound to decrease message redelivery times, which is specified by MaxExtensionPeriod.

This PR also refactors the field into the pullOptions struct, where it lies with the maxExtension field.

Fixes #3278

@hongalex hongalex requested a review from a team as a code owner December 3, 2020 23:07
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Dec 3, 2020
@product-auto-label product-auto-label bot added the api: pubsub Issues related to the Pub/Sub API. label Dec 3, 2020
@hongalex
Copy link
Member Author

hongalex commented Dec 3, 2020

cc @beltran, who opened the issue for Java

Copy link
Contributor

@shollyman shollyman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extracting values from function args to struct fields looks straightforward.

nit: pullOptions may deserve some inline comments similar to messageIterator; as a casual reader I don't understand what maxExtension vs maxExtensionPeriod convey.

@hongalex hongalex merged commit 45131b6 into googleapis:master Dec 7, 2020
@hongalex hongalex deleted the max-extension-period-fix branch December 7, 2020 17:56
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: StreamAckDeadlineSeconds should respect MaxExtensionPeriod
2 participants