> If we want the maximum concurrency per second to be 1, and set the maximum concurrency per minute to 60, then the actual maximum concurrency per second could be up to 60, which is 60 times larger than our expected maximum concurrency.
I think the RateLimiter can handle it: https://github.com/apache/pulsar/blob/a1405ea006f175b1bd0b9d28b9444d592fb4a010/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java#L965-L968 > Secondly, we already have the `maxConcurrentUnloadPerSec` configuration, which is provided to the user in the CLI. Adding a similar `brokerShutdownMaxNumberOfGracefulBundleUnloadPerMinute` configuration might confuse users. `maxConcurrentUnloadPerSec ` is for the admin and CLI usage. This proposal is to add `brokerShutdownMaxNumberOfGracefulBundleUnloadPerMinute ` to the broker configuration. Overall, I'm +1 for this proposal. And I agree that we need a new PIP for this change. BR, Zike Yang On Mon, Sep 25, 2023 at 3:54 PM Xiangying Meng <xiangy...@apache.org> wrote: > > Hi Donglai, Heesung > > >brokerShutdownMaxNumberOfGracefulBundleUnloadPerMinute=60 is the same as > brokerShutdownMaxNumberOfGracefulBundleUnloadPerSec=1 So, the "per-min" > config can be more granular. > > I have some doubts about introducing the > `brokerShutdownMaxNumberOfGracefulBundleUnloadPerMinute` > configuration. > > Firstly, I also think that a minute is too long. If we want the > maximum concurrency per second to be 1, and set the maximum > concurrency per minute to 60, then the actual maximum concurrency per > second could be up to 60, which is 60 times larger than our expected > maximum concurrency. Moreover, if the unload requests are concentrated > in the last 10 seconds of the previous minute and the first 10 seconds > of the next minute, then the concurrency during this period will > exceed our configuration. Such fluctuations are inevitable, but the > larger the time span we set, the greater the distortion of the > configuration. > > Secondly, we already have the `maxConcurrentUnloadPerSec` > configuration, which is provided to the user in the CLI. Adding a > similar `brokerShutdownMaxNumberOfGracefulBundleUnloadPerMinute` > configuration might confuse users. When designing configuration > parameters, we should try to keep it simple and consistent, and avoid > introducing unnecessary complexity. > > Thanks, > Xiangying > > On Mon, Sep 25, 2023 at 12:14 PM Yubiao Feng > <yubiao.f...@streamnative.io.invalid> wrote: > > > > Hi Donglai, Mattison > > > > I agree with @Mattison > > > > Thanks > > Yubiao Feng > > > > On Mon, Aug 21, 2023 at 8:50 PM <mattisonc...@gmail.com> wrote: > > > > > > > > Hi, > > > > > > I agree with this change to improve the stability of the pulsar cluster. > > > > > > Just one concern. it's better to move this discussion to a new PIP. > > > because you wanna introduce a new broker configuration. > > > `brokerShutdownMaxNumberOfGracefulBundleUnloadPerMinute` > > > > > > FYI: https://github.com/apache/pulsar/blob/master/pip/README.md > > > > > > Looking forward this change and thanks for your contribution. :) > > > > > > Best, > > > Mattison > > > > > > > > > > > > On 7 Jul 2023 at 15:30 +0800, labuladong <labulad...@foxmail.com>, wrote: > > > > Thanks you guys. > > > > > > > > > > > > I agree that per-minute is better than per-second, which is more > > > flexible. > > > > > > > > > > > > I open an issue here: > > > > > > > > > > > > https://github.com/apache/pulsar/issues/20753 > > > > > > > > > > > > Regards, > > > > donglai > > >
