Re: [DISCUSS] Cherry-pick PR-16059 to 2.10 to prevent infinite unloading
+1 for cherry-picking it to branch-2.10. We have a flag to control whether to enable or disable it. `removeMostServicingBrokersForNamespace ` is introduced by [1] to solve the problem that when all bundles in a particular namespace belong to 1 or few machines, customers owning that namespace will be heavily impacted if that broker goes down. Of course, this PR caused the infinite unloading issue and we need to fix it. > I agree with making it false for the next major version release by default. We'd better remove the config in the next version instead of change the default value to `false`, which will make Pulsar's configuration keep increasing. Thanks, Hang [1] https://github.com/apache/pulsar/pull/388 PengHui Li 于2023年7月1日周六 09:38写道： > > +1 for cherry-pick to branch-2.10 since users don't have a workaround > for this issue, and the change is well-understand, low risk. > > I agree with making it false for the next major version release by default. > > Thanks, > Penghui > > On Sat, Jul 1, 2023 at 9:26 AM Heesung Sohn > wrote: > > > Hi dev, > > > > I realized that `removeMostServicingBrokersForNamespace` func in the broker > > selection logic can cause infinite unloading. > > > > Suppose an overloaded broker unloaded a bundle and only has the minimum > > number of bundles(in that namespace) among brokers. In that case, the > > selection logic (`removeMostServicingBrokersForNamespace`) will filter out > > other brokers and always reassign the bundle to the previous broker. This > > will cause infinite unloading(like a boomerang). > > > > To mitigate this issue, we need to cherry-pick this PR to disable this > > logic by the config. > > https://github.com/apache/pulsar/pull/16059 > > > > And we probably want to disable this > > `removeMostServicingBrokersForNamespace` logic by default. > > > > Regards, > > Heesung > >
RE: [DISCUSS] PIP-280 : Refactor CLI Argument Parsing Logic for Measurement Units using JCommander's custom converter
Hi, community With regards to the refactor work, I made a PR [1] that shows how Command classes will be improved. Any feedback on the PR or the PIP [2] itself will be much appreciated. Links [1] PR : https://github.com/JooHyukKim/pulsar/pull/15 [2] PIP : https://github.com/apache/pulsar/pull/20691 Best, joo hyuk (vince) On 2023/06/30 07:20:11 Joo Hyuk Kim wrote: > Hi, community > > > I am proposing to refactor `Cmd*` classes’s measurement parsing logic. > > > ## Background > > > As it stands, the parsing logic for measurement units such as time and > bytes in various CLI classes is implemented verbosely, contained in the Cmd > converter themselves. Leading to a lack of code reuse and possible > inconsistencies. > > > ## Proposal > > > The idea is to refactor all `Cmd` classes to utilize the converter > functionality of JCommander (link[1]). This will allow us to isolate the > measurement-specific parsing logic and streamline the code, thereby > improving maintainability and reusability. See the concrete example in this > PIP for a more in-depth explanation. > > > There is ongoing PR to provide concept in (link [2]) > > > ## Concerns > > > 1. Will this create too much work or introduce confusion? > > > By working class-by-class or inner-class basis, we can gradually refactor > the codebase without causing disruption. The changes are expected to > increase readability and maintainability, making future modifications > easier and less error-prone. > > > 2. Will it affect existing functionality? > > > The proposed changes are meant to streamline and standardize the code > without altering functionality. > > > ## Implementation > > > The initial focus will be on the following classes: > > > - `CmdNamespaces.java` > > - `CmdTopics.java` > > - `CmdTopicPolicies.java` > > > More classes may be added as the refactoring process continues. > > > Let me know what you think. > > > Best, > > JooHyuk > > > PIP : https://github.com/apache/pulsar/pull/20691 > > > Links : > > [1] https://jcommander.org/#_custom_types_converters_and_splitters > > [2] https://github.com/apache/pulsar/pull/20663 >
