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): update batching and flow control parameters to be same as the other client libraries #9597

Merged
merged 3 commits into from
Nov 11, 2019

Conversation

pradn
Copy link
Contributor

@pradn pradn commented Nov 4, 2019

Changes:

  • Max batch size is reduced from 10 MB to 1 MB
  • Max latency before a batch is sent is reduced from 50 ms to 10 ms
  • Max messages in a batch is reduced from 1000 messages to 100 messages
  • Max messages in flight is increased from 100 messages to 1000 messages
  • Max ack extension is reduced from 2 hours to 1 hour
  • Also fix tests

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 4, 2019
@pradn pradn changed the title Update batching and flow control parameters to be same as the other client libraries fix(pubsub): Update batching and flow control parameters to be same as the other client libraries Nov 4, 2019
@hongalex hongalex added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 4, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 4, 2019
@pradn pradn changed the title fix(pubsub): Update batching and flow control parameters to be same as the other client libraries fix(pubsub): update batching and flow control parameters to be same as the other client libraries Nov 4, 2019
@hongalex hongalex requested a review from busunkim96 November 4, 2019 22:18
@hongalex
Copy link
Member

hongalex commented Nov 4, 2019

@busunkim96 This PR is part of an effort to standardize our client library default settings. We're looking at two changes. One change increases how often we publish bundles of messages (which shouldn't be a breaking change).

The other has to do with flow control (how many messages we can hold and how long we hold onto messages before releasing them to be redelivered). I'm more worried about this change negatively affecting users who aren't setting their own defaults. What do you recommend here?

Copy link
Contributor

@plamut plamut left a comment

Choose a reason for hiding this comment

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

The changes look good per se. I am not totally familiar with the defaults in other libraries, thus I'll leave that aspect to others to verify.

@plamut
Copy link
Contributor

plamut commented Nov 11, 2019

Merging to unblock a related PR. If it turns out that additional changes are needed, we can make them in a follow-up PR.

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.

5 participants