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

Add -c/--max-connections parameter to pulsar-perf-go and set it to 1 by default #765

Merged
merged 2 commits into from
Apr 29, 2022
Merged

Add -c/--max-connections parameter to pulsar-perf-go and set it to 1 by default #765

merged 2 commits into from
Apr 29, 2022

Conversation

lhotari
Copy link
Member

@lhotari lhotari commented Apr 25, 2022

Motivation

  • The Java based pulsar-perf uses 100 connections by default

Modifications

  • add support for setting MaxConnectionsPerBroker in pulsar-perf-go

@lhotari lhotari self-assigned this Apr 25, 2022
@lhotari lhotari force-pushed the lh-max-connections-per-broker branch from 5ceb788 to 1d8faaf Compare April 25, 2022 15:35
@lhotari lhotari requested a review from merlimat April 25, 2022 16:07
@lhotari
Add -c/--max-connections parameter to pulsar-perf-go and set it to 10…
e6c0a17 
…0 by default

- The Java based pulsar-perf uses 100 connections by default
@lhotari lhotari force-pushed the lh-max-connections-per-broker branch from 1d8faaf to e6c0a17 Compare April 26, 2022 11:08
@zzzming
Copy link
Contributor

zzzming commented Apr 28, 2022

+1

@merlimat
Copy link
Contributor

The Java default to 100 is not a very good one (I put it a very long time ago).. there usually not an advantage to have these many connections per host. I'd stick to 1 by default (and change Java too! :D )

@lhotari
Copy link
Member Author

lhotari commented Apr 29, 2022

The Java default to 100 is not a very good one (I put it a very long time ago).. there usually not an advantage to have these many connections per host. I'd stick to 1 by default (and change Java too! :D )

@merlimat I changed it to 1 here. PTAL

@lhotari lhotari mentioned this pull request Apr 29, 2022
Merged
@lhotari
Copy link
Member Author

lhotari commented Apr 29, 2022

The Java default to 100 is not a very good one (I put it a very long time ago).. there usually not an advantage to have these many connections per host. I'd stick to 1 by default (and change Java too! :D )

@merlimat #15387 is the change for the Java pulsar-perf.

@merlimat merlimat changed the title Add -c/--max-connections parameter to pulsar-perf-go and set it to 100 by default Add -c/--max-connections parameter to pulsar-perf-go and set it to 1 by default Apr 29, 2022
@merlimat merlimat added this to the v0.9.0 milestone Apr 29, 2022
@merlimat merlimat merged commit 7c5846d into apache:master Apr 29, 2022
@lhotari lhotari deleted the lh-max-connections-per-broker branch May 1, 2022 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@merlimat merlimat merlimat approved these changes

@michaeljmarshall michaeljmarshall michaeljmarshall approved these changes

@cckellogg cckellogg Awaiting requested review from cckellogg

@wolfstudy wolfstudy Awaiting requested review from wolfstudy

Assignees

@lhotari lhotari

Labels
type/enhancement
Projects
None yet
Milestone
v0.9.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@lhotari @zzzming @merlimat @michaeljmarshall