FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(pubsub): update batching and flow control parameters to be same as the other client libraries by pradn · Pull Request #9597 · googleapis/google-cloud-python · GitHub

/ google-cloud-python Public

fix(pubsub): update batching and flow control parameters to be same as the other client libraries - #9597

Merged
plamut merged 3 commits into
googleapis:masterfrom
pradn:harmonize-default-parameters
Nov 11, 2019
ConversationCommits (3)ChecksFiles changed
Merged

fix(pubsub): update batching and flow control parameters to be same as the other client libraries#9597
plamut merged 3 commits into
googleapis:masterfrom
pradn:harmonize-default-parameters

Conversation

pradn commented Nov 4, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

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 added the cla: yes This human has signed the Contributor License Agreement. label Nov 4, 2019
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 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 4, 2019
yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 4, 2019
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 requested a review from busunkim96 November 4, 2019 22:18

hongalex commented Nov 4, 2019

Copy link
Copy Markdown
Member

@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?

plamut approved these changes Nov 5, 2019
View reviewed changes

plamut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 added the api: pubsub Issues related to the Pub/Sub API. label Nov 7, 2019
This was referenced Nov 7, 2019
Closed
Merged

plamut commented Nov 11, 2019

Copy link
Copy Markdown
Contributor

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

plamut merged commit 6a46ac2 into googleapis:master Nov 11, 2019
yoshi-automation mentioned this pull request Jan 29, 2020
Merged
parthea pushed a commit that referenced this pull request Mar 2, 2026
fix(pubsub): update batching and flow control parameters to be same a…
c5eba0f 
…s the other client libraries (#9597)

* Update batching and flow control parameters to be same as the other client libraries.

* Fix tests

* Update test leaser param
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

anguillanneuf Awaiting requested review from anguillanneuf
busunkim96 Awaiting requested review from busunkim96
1 more reviewer
plamut plamut approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

api: pubsub Issues related to the Pub/Sub API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL