You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/05/24 08:24:40 UTC

[GitHub] [pulsar] lhotari opened a new pull request, #15748: [Cli tools] Disable Pulsar client memory limit by default

lhotari opened a new pull request, #15748:
URL: https://github.com/apache/pulsar/pull/15748

   ### Motivation
   
   - 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.
   
   - Memory limit is enabled by default in "PIP-120: Enable client memory limit by default", #13306 . PR was #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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar] lhotari commented on pull request #15748: [Cli tools] Disable Pulsar client memory limit by default

Posted by GitBox <gi...@apache.org>.
lhotari commented on PR #15748:
URL: https://github.com/apache/pulsar/pull/15748#issuecomment-1145288916

   > 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.
   
   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 `maxPendingMessages` and `maxPendingMessagesAcrossPartitions` settings (as well as `batchingMaxBytes`, `batchingMaxPublishDelayMicros` and `batchingMaxMessages` settings). For a testing tool, I think it's unexpected that the parameters wouldn't have an impact in the CLI tools such as `pulsar-perf`. That's why I think it's better to disable the memory limit until there's a configurable parameter.
   
   > I believe a better change here is to make the memory limit configurable for the perf producer.
   
   I agree. Until that is done, it's better to set the limit to 0 by default. I added #15912 for tracking this.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar] lhotari merged pull request #15748: [Cli tools] Disable Pulsar client memory limit by default

Posted by GitBox <gi...@apache.org>.
lhotari merged PR #15748:
URL: https://github.com/apache/pulsar/pull/15748


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar] lhotari closed pull request #15748: [Cli tools] Disable Pulsar client memory limit by default

Posted by GitBox <gi...@apache.org>.
lhotari closed pull request #15748: [Cli tools] Disable Pulsar client memory limit by default
URL: https://github.com/apache/pulsar/pull/15748


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar] merlimat commented on pull request #15748: [Cli tools] Disable Pulsar client memory limit by default

Posted by GitBox <gi...@apache.org>.
merlimat commented on PR #15748:
URL: https://github.com/apache/pulsar/pull/15748#issuecomment-1145013379

   > There's a regression with the tools since the memory limit cannot be adjusted
   
   @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.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org