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 2021/05/20 04:27:11 UTC

[GitHub] [pulsar] merlimat opened a new pull request #10649: Allow to configure the number of BK client worker threads

merlimat opened a new pull request #10649:
URL: https://github.com/apache/pulsar/pull/10649


   ### Motivation
   
   In broker, we should be allowed to configure the number of threads to be be used for the BK worker pool.


-- 
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.

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



[GitHub] [pulsar] sijie edited a comment on pull request #10649: Allow to configure the number of BK client worker threads

Posted by GitBox <gi...@apache.org>.
sijie edited a comment on pull request #10649:
URL: https://github.com/apache/pulsar/pull/10649#issuecomment-845561904


   @eolivelli 
   
   `bookkeeper_` prefix is designed for allowing configuring bookkeeper client settings if they are not available in the broker configuration. But if there are settings critical for configuring bookkeeper client, we should make new settings for them, make them explicit in the broker configuration and follow the naming convention of the other bookkeeper client settings. 


-- 
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.

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



[GitHub] [pulsar] eolivelli commented on pull request #10649: Allow to configure the number of BK client worker threads

Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #10649:
URL: https://github.com/apache/pulsar/pull/10649#issuecomment-844776021


   > I think that having that in `broker.conf` is the most appropriate form to advertise the configurability of such parameter.
   
   Yes. Can add the line to broker.conf.
   Nothing more
   


-- 
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.

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



[GitHub] [pulsar] merlimat commented on pull request #10649: Allow to configure the number of BK client worker threads

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


   I think that having that in `broker.conf` is the most appropriate form to advertise the configurability of such parameter. 


-- 
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.

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



[GitHub] [pulsar] eolivelli edited a comment on pull request #10649: Allow to configure the number of BK client worker threads

Posted by GitBox <gi...@apache.org>.
eolivelli edited a comment on pull request #10649:
URL: https://github.com/apache/pulsar/pull/10649#issuecomment-844776021


   > I think that having that in `broker.conf` is the most appropriate form to advertise the configurability of such parameter.
   
   Yes. We could add the line to broker.conf.
   Nothing more
   


-- 
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.

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



[GitHub] [pulsar] merlimat commented on pull request #10649: Allow to configure the number of BK client worker threads

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


   We need to have this documented and visible though. 
   


-- 
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.

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



[GitHub] [pulsar] sijie commented on pull request #10649: Allow to configure the number of BK client worker threads

Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #10649:
URL: https://github.com/apache/pulsar/pull/10649#issuecomment-845561904


   `bookkeeper_` prefix is designed for allowing configuring bookkeeper client settings if they are not available in the broker configuration. But if there are settings critical for configuring bookkeeper client, we should make new settings for them, make them explicit in the broker configuration and follow the naming convention of the other bookkeeper client settings. 


-- 
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.

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



[GitHub] [pulsar] eolivelli merged pull request #10649: Allow to configure the number of BK client worker threads

Posted by GitBox <gi...@apache.org>.
eolivelli merged pull request #10649:
URL: https://github.com/apache/pulsar/pull/10649


   


-- 
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.

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



[GitHub] [pulsar] sijie commented on pull request #10649: Allow to configure the number of BK client worker threads

Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #10649:
URL: https://github.com/apache/pulsar/pull/10649#issuecomment-845559792


   Having this setting available can emphasize its importance. Because it is very relevant to the performance. +1 on adding this setting.


-- 
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.

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



[GitHub] [pulsar] merlimat commented on pull request #10649: Allow to configure the number of BK client worker threads

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


   Having the `bookkeeper_` prefix is good for settings that we don't expect to have to change in broker (while retaining the ability to do so), but for settings that are exposed directly in broker, we should follow the existing convention and naming scheme.
   
   For this setting, I believe that it is very relevant to the performance of brokers and we should have it in same consistent way as the other BK client settings within broker.conf.
   
   


-- 
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.

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



[GitHub] [pulsar] eolivelli commented on pull request #10649: Allow to configure the number of BK client worker threads

Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #10649:
URL: https://github.com/apache/pulsar/pull/10649#issuecomment-844698523


   Sorry I amended my comment. 
   I wanted to say that we just have to document this property.
   


-- 
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.

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