> `brokerShutdownMaxNumberOfGracefulBundleUnloadPerSec=1` So, the "per-min" config can be more granular.
Oh, okay. I thought we would end up only using one of `brokerShutdownMaxNumberOfGracefulBundleUnloadPerMinute` `brokerShutdownMaxNumberOfGracefulBundleUnloadPerSec` ... these two. If there is per-sec configuration, we might simply have default per-second config value 60 (1minute). Thanks, JooHyukKim(vince) On Fri, Jul 7, 2023 at 2:09 PM Heesung Sohn <heesung.s...@streamnative.io.invalid> wrote: > > but we should also consider per-minute could be too slow for some. > Could you clarify this part? > `brokerShutdownMaxNumberOfGracefulBundleUnloadPerMinute=60` is the same as > `brokerShutdownMaxNumberOfGracefulBundleUnloadPerSec=1` So, the "per-min" > config can be more granular. > > Heesung > > On Thu, Jul 6, 2023 at 10:04 PM Joo Hyuk Kim <beansk...@gmail.com> wrote: > > > Hi, > > > > I agree that unloading per-sec might too fast, but we should also > consider > > per-minute could be too slow for some. > > So we might as well add configurability for unload rate (in seconds).But > I > > am concerend that we might end up having too many parameters 🤔 > > > > Thanks, > > JooHyukKim(vince) > > > > On Fri, Jul 7, 2023 at 1:57 PM Heesung Sohn > > <heesung.s...@streamnative.io.invalid> wrote: > > > > > Hi, > > > > > > I agree with configuring the > `shutdown.unloadNamespaceBundlesGracefully` > > > behavior by a dynamic config. > > > > > > Also, I wonder if the better config is > > > `brokerShutdownMaxNumberOfGracefulBundleUnloadPerMinute` to support a > > > slower rate. > > > One bundle unloading per sec could be too fast if there are many > > > topics(thousands) per bundle. > > > > > > Regards, > > > Heesung > > > > > > > > > On Thu, Jul 6, 2023 at 8:32 PM labuladong <labulad...@foxmail.com> > > wrote: > > > > > > > Hi JooHyukKim, > > > > > > > > > > > > That is what I want to do. Just use this function: > > > > > > > > > > > > > > > > > > > > > > https://github.com/apache/pulsar/blob/7636e8989f4d3fc24fce69a356d54e1c550945ed/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java#L557 > > > > > > > > > > > > to replace this function in closeAsync: > > > > > > > > > > > > > > > > > > > > > > https://github.com/apache/pulsar/blob/7636e8989f4d3fc24fce69a356d54e1c550945ed/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java#L790-L791 > > > > > > > > > > > > Now admin API support to limit the unload concurrency, but > > graceful > > > > shutdown doesn't support, it's easy to improve. > > > > > > > > > > > > In addition, I suggest storing the `maxConcurrentUnloadPerSec` as a > > > broker > > > > dynamic config, because users may want to change this value in > > different > > > > situation. What do you think, static config or dynamic config? > > > > > On Fri, Jul 7, 2023 at 2:09 PM Heesung Sohn <heesung.s...@streamnative.io.invalid> wrote: > > but we should also consider per-minute could be too slow for some. > Could you clarify this part? > `brokerShutdownMaxNumberOfGracefulBundleUnloadPerMinute=60` is the same as > `brokerShutdownMaxNumberOfGracefulBundleUnloadPerSec=1` So, the "per-min" > config can be more granular. > > Heesung > > On Thu, Jul 6, 2023 at 10:04 PM Joo Hyuk Kim <beansk...@gmail.com> wrote: > > > Hi, > > > > I agree that unloading per-sec might too fast, but we should also > consider > > per-minute could be too slow for some. > > So we might as well add configurability for unload rate (in seconds).But > I > > am concerend that we might end up having too many parameters 🤔 > > > > Thanks, > > JooHyukKim(vince) > > > > On Fri, Jul 7, 2023 at 1:57 PM Heesung Sohn > > <heesung.s...@streamnative.io.invalid> wrote: > > > > > Hi, > > > > > > I agree with configuring the > `shutdown.unloadNamespaceBundlesGracefully` > > > behavior by a dynamic config. > > > > > > Also, I wonder if the better config is > > > `brokerShutdownMaxNumberOfGracefulBundleUnloadPerMinute` to support a > > > slower rate. > > > One bundle unloading per sec could be too fast if there are many > > > topics(thousands) per bundle. > > > > > > Regards, > > > Heesung > > > > > > > > > On Thu, Jul 6, 2023 at 8:32 PM labuladong <labulad...@foxmail.com> > > wrote: > > > > > > > Hi JooHyukKim, > > > > > > > > > > > > That is what I want to do. Just use this function: > > > > > > > > > > > > > > > > > > > > > > https://github.com/apache/pulsar/blob/7636e8989f4d3fc24fce69a356d54e1c550945ed/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java#L557 > > > > > > > > > > > > to replace this function in closeAsync: > > > > > > > > > > > > > > > > > > > > > > https://github.com/apache/pulsar/blob/7636e8989f4d3fc24fce69a356d54e1c550945ed/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java#L790-L791 > > > > > > > > > > > > Now admin API support to limit the unload concurrency, but > > graceful > > > > shutdown doesn't support, it's easy to improve. > > > > > > > > > > > > In addition, I suggest storing the `maxConcurrentUnloadPerSec` as a > > > broker > > > > dynamic config, because users may want to change this value in > > different > > > > situation. What do you think, static config or dynamic config? > > > > > >
