-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[Cli tools] Disable Pulsar client memory limit by default #15748
[Cli tools] Disable Pulsar client memory limit by default #15748
Conversation
- There's a regression with the tools since the memory limit cannot be adjusted - It's better to default to the previous setting of disabling memory limits so that the performance profile doesn't change because of the memory limit.
3e13d58
to
8742024
Compare
@lhotari Can you elaborate on what kind of regression it was seen there? I have in general experienced the opposite, going from msg-count based to memory based. I think this change goes in the wrong direction as it completely removes the memory limit and it will easily cause OOM in the client, since there's no limitation on the producer queue size. I believe a better change here is to make the memory limit configurable for the perf producer. |
We have run into regressions with pulsar-perf that seem to be caused by the change in the memory limit. Some test results are very sensitive of the producer's For a performance testing tool such as
I agree. Until that is done, it's better to set the limit to 0 by default. I added #15912 for tracking this. |
- There's a regression with the tools since the memory limit cannot be adjusted - It's better to default to the previous setting of disabling memory limits so that the performance profile doesn't change because of the memory limit. (cherry picked from commit a6a7516)
- There's a regression with the tools since the memory limit cannot be adjusted - It's better to default to the previous setting of disabling memory limits so that the performance profile doesn't change because of the memory limit. (cherry picked from commit a6a7516)
Motivation
There's a regression with the tools since the memory limit cannot be adjusted
so that the performance profile doesn't change because of the memory limit.
Memory limit is enabled by default in "PIP-120: Enable client memory limit by default", PIP-120: Enable client memory limit by default #13306 . PR was PIP-120 : Enable client memory limit controller by default #13344 .
Modifications
Set
.memoryLimit(0, SizeUnit.BYTES)
for PulsarClient builders in Cli tools.Additional context
#15723 restores the default
maxPendingMessages
&maxPendingMessagesAcrossPartitions
limits for producers to the defaults used before PIP-120 changes when the memory limit is disabled. That might be necessary in some cases for restoring the pre-PIP-120 behavior.