RE: Re: [DISCUSS] Replace stale bot with ping-pong workflow
Coming from https://github.com/apache/pulsar/pull/21549. Agreed on Stale-Bot creating too much noise. Another suggestion : just disable commenting functionality. There is consensus where noise is the problem and not the having Stale label itself. All we need to do is remove field "stale-issue-message” and "stale-pr-message”. I am making we should also account that labeling issues and PRs do serve us well (e.g. anything other than “Stale” label). PS : Sorry for late reaction btw Thanks, JooHyukKim On 2023/11/09 14:29:10 tison wrote: > Thank you! > > I saw a few other complains for this "noise" from the issue author recently > also. > > Since the real-world solution is handling issues effectively, I support > disable this action. Keep the PR open for a while to accept comments. > > I'll merge this patch in the next monday if no more objection. > > Best, > tison. > > > Asaf Mesika 于2023年11月9日周四 21:21写道： > > > Submitted a PR to disable it: https://github.com/apache/pulsar/pull/21549 > > > > On Tue, Nov 7, 2023 at 3:58 PM Asaf Mesika wrote: > > > > > Tison let's start as you suggested by disabling it > > > > > > > > > On Tue, May 16, 2023 at 5:13 AM Yunze Xu wrote: > > > > > >> +1 to me > > >> > > >> Thanks, > > >> Yunze > > >> > > >> On Sun, May 14, 2023 at 9:28 PM Dave Fisher > > >> wrote: > > >> > > > >> > Hi - > > >> > > > >> > I have not looked at all your links but I think this is a great idea. > > >> This will help everyone pay attention better. > > >> > > > >> > Best, > > >> > Dave > > >> > > > >> > Sent from my iPhone > > >> > > > >> > > On May 14, 2023, at 12:33 AM, tison wrote: > > >> > > > > >> > > Of course, changing the workflow cannot magically increase the > > >> bandwidth to > > >> > > handle stale issues. That is what the triage guide wants to > > encourage > > >> > > committers to practice. But such a move can reduce the frustrating > > >> > > experience and explicitly express who is responsible for taking the > > >> next > > >> > > action to nudge the conversation. > > >> > > > > >> > > Best, > > >> > > tison. > > >> > > > > >> > > > > >> > > tison 于2023年5月14日周日 15:28写道： > > >> > > > > >> > >> Hi devs, > > >> > >> > > >> > >> Recently, I have handled a large number of stale issues and noticed > > >> that > > >> > >> periodically notifying users that "the issue is stale" without any > > >> human > > >> > >> reaction can be a frustrating experience, e.g., ISSUE-13925[1]. > > >> > >> > > >> > >> Learning from the INFRA JIRA project experience, I propose we > > >> replace the > > >> > >> stale bot with a ping-pong workflow. That is - > > >> > >> > > >> > >> ping - Labeling waiting-for-reviewer on issue created and commented > > >> by > > >> > >> non-committers > > >> > >> pong - Labeling waiting-for-user on issue responded by committers > > >> > >> > > >> > >> Here is a demo implementation[2] you can refer to and you can try > > the > > >> > >> workflow in my fork[3]. > > >> > >> > > >> > >> Previous references - > > >> > >> > > >> > >> * The triage guide[4] > > >> > >> * [DISCUSS] Does stale bot make value for you?[5] > > >> > >> * [COMMITTER ATTENTION] You can close stale issues as not planned > > [6] > > >> > >> > > >> > >> Looking forward to your feedback :D > > >> > >> > > >> > >> Best, > > >> > >> tison. > > >> > >> > > >> > >> [1] https://github.com/apache/pulsar/issues/13925 > > >> > >> [2] https://github.com/apache/pulsar/pull/20319 > > >> > >> [3] https://github.com/tisonkun/pulsar > > >> > >> [4] https://pulsar.apache.org/contribute/develop-triage > > >> > >> [5] > > https://lists.apache.org/thread/tv774jqohdpx8x0dymsskrd90xwwfvgp > > >> > >> [6] > > https://lists.apache.org/thread/x2c7xod8y0wvh14nsb6bknf0dq3r9gls > > >> > >> > > >> > >> > > >> > > > >> > > > > > >
RE: Re: [DISCUSS] Seek for maintainers of pulsar-manager
NOTE: Earlier mail was sent in such a messy state, so I am sending my reply again. IMO, Pulsar-manager[1] really could benefit from more tests. Fortunately, I am familiar with the tech stack tho might need some recollection on front-end :)) If you plan on continuing the work, Tison. I will also start to look around and put in some work. Thanks, JooHyukKim (vince) [1] https://github.com/apache/pulsar-manager On 2023/07/23 07:44:05 Joo Hyuk Kim wrote: > > However, the tech stack is a bit old to take up and it seems we have > fewer contributors maintain this project. I tried to resolve some issues > but encountered troubles in both lack of time and lack of collaborators. > > > It really seems that the Pulsar-manager project lacks contributions and > could benefit from more tests. > > > > Thus, I started this thread to seek maintainers (or collaborators, I'm > spending some time on this project, but it's a bit frustrating to me to > recap the design alone (of course, the project needs a recap)) of > pulsar-manager. > > > Fortunately, I am familiar with the tech stack, especially with the backend > in Spring. Although the frontend might need some recollection :)). > > > If you plan on continuing the work, Tison. I will also start to look around > and put in some work. > > > Thanks, > > JooHyukKim (vince) > > On Wed, Jul 19, 2023 at 6:40 PM tison wrote: > > > Hi devs, > > > > The pulsar-manager project[1], while it's still under 0.x version series, > > has had low traffic during the past years. > > > > I recently drove the release of pulsar-manager 0.4.0[2] and saw an > > increasing issue traffics around the community. > > > > However, the tech stack is a bit old to take up and it seems we have fewer > > contributors maintain this project. I tried to resolve some issues but > > encountered troubles in both lack of time and lack of collaborators. > > > > I remember a thread talking about Pulsar UI[3]. But no maintainer of > > pulsar-manager showed up in that thread. The pulsar-manager project looks > > half-implemented and stalls. > > > > Thus, I started this thread to seek maintainers (or collaborators, I'm > > spending some time on this project, but it's a bit frustrating to me to > > recap the design alone (of course, the project needs a recap)) of > > pulsar-manager. > > > > Also cc @users, let me know how you use this project, what's your > > expectations of this project, and if you are will to share your > > requirements as well as patches among the community. > > > > Looking forward to your feedback! > > > > Best, > > tison. > > > > [1] https://github.com/apache/pulsar-manager > > [2] https://lists.apache.org/thread/03h1yvqx0lkj3htxd55zj6wfk95bcvlo > > [3] https://lists.apache.org/thread/y20bkzwtkqtky517g560cdoq7g95ttkx > > >
Re: [DISCUSS] Seek for maintainers of pulsar-manager
> However, the tech stack is a bit old to take up and it seems we have fewer contributors maintain this project. I tried to resolve some issues but encountered troubles in both lack of time and lack of collaborators. It really seems that the Pulsar-manager project lacks contributions and could benefit from more tests. > Thus, I started this thread to seek maintainers (or collaborators, I'm spending some time on this project, but it's a bit frustrating to me to recap the design alone (of course, the project needs a recap)) of pulsar-manager. Fortunately, I am familiar with the tech stack, especially with the backend in Spring. Although the frontend might need some recollection :)). If you plan on continuing the work, Tison. I will also start to look around and put in some work. Thanks, JooHyukKim (vince) On Wed, Jul 19, 2023 at 6:40 PM tison wrote: > Hi devs, > > The pulsar-manager project[1], while it's still under 0.x version series, > has had low traffic during the past years. > > I recently drove the release of pulsar-manager 0.4.0[2] and saw an > increasing issue traffics around the community. > > However, the tech stack is a bit old to take up and it seems we have fewer > contributors maintain this project. I tried to resolve some issues but > encountered troubles in both lack of time and lack of collaborators. > > I remember a thread talking about Pulsar UI[3]. But no maintainer of > pulsar-manager showed up in that thread. The pulsar-manager project looks > half-implemented and stalls. > > Thus, I started this thread to seek maintainers (or collaborators, I'm > spending some time on this project, but it's a bit frustrating to me to > recap the design alone (of course, the project needs a recap)) of > pulsar-manager. > > Also cc @users, let me know how you use this project, what's your > expectations of this project, and if you are will to share your > requirements as well as patches among the community. > > Looking forward to your feedback! > > Best, > tison. > > [1] https://github.com/apache/pulsar-manager > [2] https://lists.apache.org/thread/03h1yvqx0lkj3htxd55zj6wfk95bcvlo > [3] https://lists.apache.org/thread/y20bkzwtkqtky517g560cdoq7g95ttkx >
Re: [ANNOUNCE] Zili Chen (tison) as new PMC member in Apache Pulsar
Totally earned it. Congrats! Thanks, JooHyukKim. 2023년 7월 22일 (토) 오후 1:00, SiNan Liu 님이 작성: > Congratulations > > Thanks, > sinan > > > Yu 于2023年7月22日周六 11:53写道： > > > Congrats! > > > > On Sat, Jul 22, 2023 at 9:53 AM ZhangJian He wrote: > > > > > Congratulations > > > > > > Thanks > > > ZhangJian He > > > > > > > > > On Sat, 22 Jul 2023 at 01:19, Lari Hotari wrote: > > > > > > > Congratulations, Tison! > > > > > > > > -Lari > > > > > > > > On Fri, 21 Jul 2023, 20.05 Michael Marshall, > > > wrote: > > > > > > > > > Hi Pulsar Community, > > > > > > > > > > The Apache Pulsar Project Management Committee (PMC) has invited > Zili > > > > Chen > > > > > (https://github.com/tisonkun) as a member of the PMC and we are > > > > > pleased to announce that tison has accepted. > > > > > > > > > > tison is very active in the community by contributing and reviewing > > > > > many PRs, actively engaging on the mailing list, triaging GitHub > > > > > Issues, and helping out with the website. > > > > > > > > > > On behalf of the Pulsar PMC, welcome and congratulations to tison! > > > > > > > > > > Best, > > > > > Michael > > > > > > > > > > > > > > >
Re: [VOTE] PIP-280 : Refactor CLI Argument Parsing Logic for Measurement Units using JCommander's custom converter
Hi community, Thank you all for your participation. We may close the vote with the following result. 1 of (+1 non-binding) - Zili Chen 3 of (+1 binding) - Yunze Xu - Mattison (Qiang Zhao) - Nicolò Boschi Best regards JooHyukKim (Vince) On Tue, Jul 11, 2023 at 7:04 PM Joo Hyuk Kim wrote: > > As long as we don't introduce any breaking change and the new parameters > > are covered by unit test > > Made a commit(Link [1]) to improve considerations > > Link [1] > https://github.com/apache/pulsar/pull/20691/commits/96515b85e97a56133a512165c981361fab939ec1 > > Thanks, > JooHyukKim (Vince) > > On Tue, Jul 11, 2023 at 6:58 PM Joo Hyuk Kim wrote: > >> > As long as we don't introduce any breaking change and the new >> parameters are covered by unit test >> >> Hello, thank you for your feedback. >> I agree and will add following to PIP file. >> "The refactored parameters should maintain coverage. " >> >> On Tue, Jul 11, 2023 at 6:34 PM Nicolò Boschi >> wrote: >> >>> +1 binding >>> As long as we don't introduce any breaking change and the new parameters >>> are covered by unit test >>> >>> Thanks, >>> Nicolò Boschi >>> >>> >>> Il giorno mar 11 lug 2023 alle ore 05:00 Qiang Zhao < >>> mattisonc...@apache.org> >>> ha scritto: >>> >>> > +1(binding) >>> > >>> > Best, >>> > Mattison >>> > >>> > On 2023/07/07 09:25:22 Joo Hyuk Kim wrote: >>> > > Hi community, >>> > > >>> > > This PIP has received a couple of approvals in github PR link [1] >>> > > So I thought it's time to vote. >>> > > >>> > > ## Motivation >>> > > >>> > > In the current Pulsar codebase, the logic to parse CLI arguments for >>> > > measurement units like time and bytes is >>> > > >>> > > scattered across various CLI classes. Each value read has its >>> distinct >>> > > parsing implementation, leading to a lack of code >>> > > >>> > > reuse. >>> > > >>> > > >>> > > ## Goals >>> > > >>> > > >>> > > This PIP is to refactor the argument parsing logic to leverage the >>> > > `@Parameter.converter` >>> > > >>> > > functionality provided by JCommander [link 3]. This will isolate the >>> > > measurement-specific parsing logic and increase >>> > > >>> > > code >>> > > >>> > > reusability. >>> > > >>> > > >>> > > ### In Scope >>> > > >>> > > >>> > > - Refactor all `Cmd` classes to utilize the converter functionality >>> of >>> > > JCommander. This will streamline the parsing >>> > > >>> > > logic and simplify the codebase. >>> > > >>> > > - Refer to bottom section "Concrete Example", before "Links" >>> > > >>> > > - Or on-going PR with small use case in >>> > > https://github.com/apache/pulsar/pull/20663 >>> > > >>> > > >>> > > ## links >>> > > >>> > > >>> > > [1] PR : https://github.com/apache/pulsar/pull/20691 >>> > > >>> > > >>> > > >>> > > Best regards, >>> > > >>> > > JooHyukKim (Vince) >>> > > >>> > >>> >>
Re: [VOTE] PIP-280 : Refactor CLI Argument Parsing Logic for Measurement Units using JCommander's custom converter
> As long as we don't introduce any breaking change and the new parameters > are covered by unit test Made a commit(Link [1]) to improve considerations Link [1] https://github.com/apache/pulsar/pull/20691/commits/96515b85e97a56133a512165c981361fab939ec1 Thanks, JooHyukKim (Vince) On Tue, Jul 11, 2023 at 6:58 PM Joo Hyuk Kim wrote: > > As long as we don't introduce any breaking change and the new parameters > are covered by unit test > > Hello, thank you for your feedback. > I agree and will add following to PIP file. > "The refactored parameters should maintain coverage. " > > On Tue, Jul 11, 2023 at 6:34 PM Nicolò Boschi > wrote: > >> +1 binding >> As long as we don't introduce any breaking change and the new parameters >> are covered by unit test >> >> Thanks, >> Nicolò Boschi >> >> >> Il giorno mar 11 lug 2023 alle ore 05:00 Qiang Zhao < >> mattisonc...@apache.org> >> ha scritto: >> >> > +1(binding) >> > >> > Best, >> > Mattison >> > >> > On 2023/07/07 09:25:22 Joo Hyuk Kim wrote: >> > > Hi community, >> > > >> > > This PIP has received a couple of approvals in github PR link [1] >> > > So I thought it's time to vote. >> > > >> > > ## Motivation >> > > >> > > In the current Pulsar codebase, the logic to parse CLI arguments for >> > > measurement units like time and bytes is >> > > >> > > scattered across various CLI classes. Each value read has its distinct >> > > parsing implementation, leading to a lack of code >> > > >> > > reuse. >> > > >> > > >> > > ## Goals >> > > >> > > >> > > This PIP is to refactor the argument parsing logic to leverage the >> > > `@Parameter.converter` >> > > >> > > functionality provided by JCommander [link 3]. This will isolate the >> > > measurement-specific parsing logic and increase >> > > >> > > code >> > > >> > > reusability. >> > > >> > > >> > > ### In Scope >> > > >> > > >> > > - Refactor all `Cmd` classes to utilize the converter functionality of >> > > JCommander. This will streamline the parsing >> > > >> > > logic and simplify the codebase. >> > > >> > > - Refer to bottom section "Concrete Example", before "Links" >> > > >> > > - Or on-going PR with small use case in >> > > https://github.com/apache/pulsar/pull/20663 >> > > >> > > >> > > ## links >> > > >> > > >> > > [1] PR : https://github.com/apache/pulsar/pull/20691 >> > > >> > > >> > > >> > > Best regards, >> > > >> > > JooHyukKim (Vince) >> > > >> > >> >
Re: [VOTE] PIP-280 : Refactor CLI Argument Parsing Logic for Measurement Units using JCommander's custom converter
> As long as we don't introduce any breaking change and the new parameters are covered by unit test Hello, thank you for your feedback. I agree and will add following to PIP file. "The refactored parameters should maintain coverage. " On Tue, Jul 11, 2023 at 6:34 PM Nicolò Boschi wrote: > +1 binding > As long as we don't introduce any breaking change and the new parameters > are covered by unit test > > Thanks, > Nicolò Boschi > > > Il giorno mar 11 lug 2023 alle ore 05:00 Qiang Zhao < > mattisonc...@apache.org> > ha scritto: > > > +1(binding) > > > > Best, > > Mattison > > > > On 2023/07/07 09:25:22 Joo Hyuk Kim wrote: > > > Hi community, > > > > > > This PIP has received a couple of approvals in github PR link [1] > > > So I thought it's time to vote. > > > > > > ## Motivation > > > > > > In the current Pulsar codebase, the logic to parse CLI arguments for > > > measurement units like time and bytes is > > > > > > scattered across various CLI classes. Each value read has its distinct > > > parsing implementation, leading to a lack of code > > > > > > reuse. > > > > > > > > > ## Goals > > > > > > > > > This PIP is to refactor the argument parsing logic to leverage the > > > `@Parameter.converter` > > > > > > functionality provided by JCommander [link 3]. This will isolate the > > > measurement-specific parsing logic and increase > > > > > > code > > > > > > reusability. > > > > > > > > > ### In Scope > > > > > > > > > - Refactor all `Cmd` classes to utilize the converter functionality of > > > JCommander. This will streamline the parsing > > > > > > logic and simplify the codebase. > > > > > > - Refer to bottom section "Concrete Example", before "Links" > > > > > > - Or on-going PR with small use case in > > > https://github.com/apache/pulsar/pull/20663 > > > > > > > > > ## links > > > > > > > > > [1] PR : https://github.com/apache/pulsar/pull/20691 > > > > > > > > > > > > Best regards, > > > > > > JooHyukKim (Vince) > > > > > >
RE: Re: [VOTE] PIP-280 : Refactor CLI Argument Parsing Logic for Measurement Units using JCommander's custom converter
Thanks, JooHyuk. On 2023/07/10 00:24:20 Zili Chen wrote: > +1 (non-binding) > > On 2023/07/07 09:25:22 Joo Hyuk Kim wrote: > > Hi community, > > > > This PIP has received a couple of approvals in github PR link [1] > > So I thought it's time to vote. > > > > ## Motivation > > > > In the current Pulsar codebase, the logic to parse CLI arguments for > > measurement units like time and bytes is > > > > scattered across various CLI classes. Each value read has its distinct > > parsing implementation, leading to a lack of code > > > > reuse. > > > > > > ## Goals > > > > > > This PIP is to refactor the argument parsing logic to leverage the > > `@Parameter.converter` > > > > functionality provided by JCommander [link 3]. This will isolate the > > measurement-specific parsing logic and increase > > > > code > > > > reusability. > > > > > > ### In Scope > > > > > > - Refactor all `Cmd` classes to utilize the converter functionality of > > JCommander. This will streamline the parsing > > > > logic and simplify the codebase. > > > > - Refer to bottom section "Concrete Example", before "Links" > > > > - Or on-going PR with small use case in > > https://github.com/apache/pulsar/pull/20663 > > > > > > ## links > > > > > > [1] PR : https://github.com/apache/pulsar/pull/20691 > > > > > > > > Best regards, > > > > JooHyukKim (Vince) > > >
[VOTE] PIP-280 : Refactor CLI Argument Parsing Logic for Measurement Units using JCommander's custom converter
Hi community, This PIP has received a couple of approvals in github PR link [1] So I thought it's time to vote. ## Motivation In the current Pulsar codebase, the logic to parse CLI arguments for measurement units like time and bytes is scattered across various CLI classes. Each value read has its distinct parsing implementation, leading to a lack of code reuse. ## Goals This PIP is to refactor the argument parsing logic to leverage the `@Parameter.converter` functionality provided by JCommander [link 3]. This will isolate the measurement-specific parsing logic and increase code reusability. ### In Scope - Refactor all `Cmd` classes to utilize the converter functionality of JCommander. This will streamline the parsing logic and simplify the codebase. - Refer to bottom section "Concrete Example", before "Links" - Or on-going PR with small use case in https://github.com/apache/pulsar/pull/20663 ## links [1] PR : https://github.com/apache/pulsar/pull/20691 Best regards, JooHyukKim (Vince)
Re: [DISCUSS] Unload Rate Limiting during Graceful Shutdown of Pulsar
> `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 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 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 > > 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 > > wrote: > > > > > > > HiJooHyukKim, > > > > > > > > > > > > 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 unloadconcurrency, 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 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 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 > > 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 > > wrote: > > > > > > &g
Re: [DISCUSS] Unload Rate Limiting during Graceful Shutdown of Pulsar
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 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 wrote: > > > HiJooHyukKim, > > > > > > 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 unloadconcurrency, 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? >
Re: [DISCUSS] Unload Rate Limiting during Graceful Shutdown of Pulsar
Thanks, JooHyukKim (vince) On Fri, Jul 7, 2023 at 11:59 AM Joo Hyuk Kim wrote: > > When Pulsar undergoes a graceful shutdown, it unloads all topics, but > there is no limit on the rate of unloading > > Is it possible that following code might already cover what we need here? > > > https://github.com/apache/pulsar/blob/7636e8989f4d3fc24fce69a356d54e1c550945ed/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java#L557 > > On Fri, Jul 7, 2023 at 11:06 AM Dave Fisher wrote: > >> Hi - >> >> Interesting. I’m curious to know what Heesung, Lari, and others think. >> >> Best, >> Dave >> >> Sent from my iPhone >> >> > On Jul 6, 2023, at 7:01 PM, labuladong wrote: >> > >> > Dear Apache Pulsar community, >> > >> > >> > When Pulsar undergoes a graceful shutdown, it unloads all topics, but >> there is no limit on the rate of unloading. As a result, during a rolling >> upgrade, the broker cluster might generate a large volume of unload >> requests in a short period, which could affect the service of the broker. >> Implementing a limit on the unload rate would make the upgrade process >> smoother. >> > >> > >> > This issue has been previously raised in a pull request and a solution >> was proposed by adding an unload interface with concurrency control: >> > >> > >> > https://github.com/apache/pulsar/pull/14114 >> > >> > >> > However, the current broker graceful shutdown doesn't use this >> interface. Hence, my proposal is pretty straightforward: >> > >> > >> > 1. Add a dynamic config to the Pulsar broker to control the concurrency >> of unloading. >> > >> > >> > 2. Modify the following code to use the function with the concurrency >> parameter for unloading. >> > >> > >> > >> https://github.com/apache/pulsar/blob/7636e8989f4d3fc24fce69a356d54e1c550945ed/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java#L790-L791 >> > >> > >> > I look forward to your thoughts on this. >> > >> > >> > >> > Best regards, donglai >> >>
Re: [DISCUSS] Unload Rate Limiting during Graceful Shutdown of Pulsar
> When Pulsar undergoes a graceful shutdown, it unloads all topics, but there is no limit on the rate of unloading Is it possible that following code might already cover what we need here? https://github.com/apache/pulsar/blob/7636e8989f4d3fc24fce69a356d54e1c550945ed/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java#L557 On Fri, Jul 7, 2023 at 11:06 AM Dave Fisher wrote: > Hi - > > Interesting. I’m curious to know what Heesung, Lari, and others think. > > Best, > Dave > > Sent from my iPhone > > > On Jul 6, 2023, at 7:01 PM, labuladong wrote: > > > > Dear Apache Pulsar community, > > > > > > When Pulsar undergoes a graceful shutdown, it unloads all topics, but > there is no limit on the rate of unloading. As a result, during a rolling > upgrade, the broker cluster might generate a large volume of unload > requests in a short period, which could affect the service of the broker. > Implementing a limit on the unload rate would make the upgrade process > smoother. > > > > > > This issue has been previously raised in a pull request and a solution > was proposed by adding an unload interface with concurrency control: > > > > > > https://github.com/apache/pulsar/pull/14114 > > > > > > However, the current broker graceful shutdown doesn't use this > interface. Hence, my proposal is pretty straightforward: > > > > > > 1. Add a dynamic config to the Pulsar broker to control the concurrency > of unloading. > > > > > > 2. Modify the following code to use the function with the concurrency > parameter for unloading. > > > > > > > https://github.com/apache/pulsar/blob/7636e8989f4d3fc24fce69a356d54e1c550945ed/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java#L790-L791 > > > > > > I look forward to your thoughts on this. > > > > > > > > Best regards, donglai > >
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 >
[DISCUSS] PIP-280 : Refactor CLI Argument Parsing Logic for Measurement Units using JCommander's custom converter
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
Re: [DISCUSS] Consistent code style (esp. ws/indent) and autotools
> Spotless has an IDEA plugin, but it's for Gradle task only. I saw this one also 沈. > [1] can be used Nice, IMO having at least one will suffice . Thanks, joohyuk On Fri, Jun 30, 2023 at 11:43 AM tison wrote: > > IDE > > If using the palantir style, [1] can be used. Spotless has an IDEA plugin > but it's for Gradle task only. > > Best, > tison. > > [1] https://plugins.jetbrains.com/plugin/13180-palantir-java-format > > > Joo Hyuk Kim 于2023年6月30日周五 10:38写道： > > > Sounds great. > > > > Another thing is about IDE. > > Pulsar site sort of directs contributors to use IntelliJ as shown in > > https://pulsar.apache.org/contribute/setup-ide/. > > It would be nice if there was IDE support (auto-format, or shortkey-base) > > and we can add it to the setup-ide guide also. > > Do you know any? Or I can do some research if you don't. > > > > Thanks, > > joohyuk > > > > On Fri, Jun 30, 2023 at 10:58 AM tison wrote: > > > > > Hi, > > > > > > I'd like to propose applying a consistent code style (especially > > whitespace > > > and indent) with an autotool Spotless. > > > > > > // Background > > > > > > Over and over we argue contributors reformat their patch manually for > > > checkstyle violations, or even whitespace changes that are not detected > > by > > > checkstyle. [1] > > > > > > A common reason is that such style-only changes increase the burden to > do > > > cherry-pick if a later bug fix is made around the code while we often > do > > > not pick the style change barely or even it's coupled with an > unpickable > > > commit. > > > > > > CheckStyle helps in some cases while we don't have a strong rule set > nor > > > even apply it to tests (porting bug fix actually include the test). > > > > > > Manually fixing style violations can be boring and even annoying. > That's > > > why it's effectively "suppressed" in mind. > > > > > > An autotool to do the formatting work can help and here comes > > Spotless[2]. > > > Flink and Curator use it and Flink actually migrated from CheckStyle to > > > Spotless for its more strict consistent rule set and oneliner format. > See > > > their original discussion for the context[3]. > > > > > > // Proprosal > > > > > > Using Spotless as the auto-formatting tool in all around apache/pulsar. > > > Since it's an auto tool I can cover the task solely. > > > > > > // Concerns > > > > > > 1. Won't it be yet another noise to the codebase? > > > > > > Yes and no. I suggest we pick this change to all maintained versions so > > > that all of them align with the consistent style. Then from now on, no > > more > > > style conflict. > > > > > > Flink used the same strategy[4] and even we can do it starting from > 2.10 > > as > > > Lari proposed to apply commits from the least recent maintained > version. > > I > > > understand the maintained versions are: 2.10, 2.11, 3.0, and master. > > > > > > 2. Can it cover all the rule sets we use in CheckStyle now? > > > > > > Let's say we almost don't care what the style is but it's consistent > and > > > "not awful". > > > > > > The default Google style takes line length = 80 which can be a > > > disappointment so in Curator I use the palantir style[5] which takes > line > > > length = 120 which should keep consistent with current settings. > > > > > > A downside is that Spotless Maven plugin cannot detect "forbidden > > imports" > > > where we can still use CheckStyle[6] - this is a low-traffic path and > it > > > should be fine. > > > > > > // Implementation > > > > > > 1. Introduce Spotless in the project and apply it around the codebase, > > for > > > all maintained versions. > > > 2. Remove the then redundant CheckStyle rules, while retaining the > > > forbidden imports part as it's still useful. > > > > > > Looking forward to your feedback! > > > > > > Best, > > > tison. > > > > > > [1] > > > > > > > > > https://github.com/apache/pulsar/pull/20642/files#diff-cb9b09b67f54fccdd5155dbbcedd50970ee93d9ee85ade1e6b6984cab64dab5d > > > [2] https://github.com/diffplug/spotless > > > [3] https://lists.apache.org/thread/3kjkcz9gj6f8j477d1t3gnbkl61hsb7z > > > [4] https://lists.apache.org/thread/nxdm0pg84q913w0kxszm502myqcg3db0 > > > [5] https://github.com/palantir/palantir-java-format > > > [6] https://issues.apache.org/jira/browse/FLINK-32154 > > > > > >
Re: [DISCUSS] Consistent code style (esp. ws/indent) and autotools
Sounds great. Another thing is about IDE. Pulsar site sort of directs contributors to use IntelliJ as shown in https://pulsar.apache.org/contribute/setup-ide/. It would be nice if there was IDE support (auto-format, or shortkey-base) and we can add it to the setup-ide guide also. Do you know any? Or I can do some research if you don't. Thanks, joohyuk On Fri, Jun 30, 2023 at 10:58 AM tison wrote: > Hi, > > I'd like to propose applying a consistent code style (especially whitespace > and indent) with an autotool Spotless. > > // Background > > Over and over we argue contributors reformat their patch manually for > checkstyle violations, or even whitespace changes that are not detected by > checkstyle. [1] > > A common reason is that such style-only changes increase the burden to do > cherry-pick if a later bug fix is made around the code while we often do > not pick the style change barely or even it's coupled with an unpickable > commit. > > CheckStyle helps in some cases while we don't have a strong rule set nor > even apply it to tests (porting bug fix actually include the test). > > Manually fixing style violations can be boring and even annoying. That's > why it's effectively "suppressed" in mind. > > An autotool to do the formatting work can help and here comes Spotless[2]. > Flink and Curator use it and Flink actually migrated from CheckStyle to > Spotless for its more strict consistent rule set and oneliner format. See > their original discussion for the context[3]. > > // Proprosal > > Using Spotless as the auto-formatting tool in all around apache/pulsar. > Since it's an auto tool I can cover the task solely. > > // Concerns > > 1. Won't it be yet another noise to the codebase? > > Yes and no. I suggest we pick this change to all maintained versions so > that all of them align with the consistent style. Then from now on, no more > style conflict. > > Flink used the same strategy[4] and even we can do it starting from 2.10 as > Lari proposed to apply commits from the least recent maintained version. I > understand the maintained versions are: 2.10, 2.11, 3.0, and master. > > 2. Can it cover all the rule sets we use in CheckStyle now? > > Let's say we almost don't care what the style is but it's consistent and > "not awful". > > The default Google style takes line length = 80 which can be a > disappointment so in Curator I use the palantir style[5] which takes line > length = 120 which should keep consistent with current settings. > > A downside is that Spotless Maven plugin cannot detect "forbidden imports" > where we can still use CheckStyle[6] - this is a low-traffic path and it > should be fine. > > // Implementation > > 1. Introduce Spotless in the project and apply it around the codebase, for > all maintained versions. > 2. Remove the then redundant CheckStyle rules, while retaining the > forbidden imports part as it's still useful. > > Looking forward to your feedback! > > Best, > tison. > > [1] > > https://github.com/apache/pulsar/pull/20642/files#diff-cb9b09b67f54fccdd5155dbbcedd50970ee93d9ee85ade1e6b6984cab64dab5d > [2] https://github.com/diffplug/spotless > [3] https://lists.apache.org/thread/3kjkcz9gj6f8j477d1t3gnbkl61hsb7z > [4] https://lists.apache.org/thread/nxdm0pg84q913w0kxszm502myqcg3db0 > [5] https://github.com/palantir/palantir-java-format > [6] https://issues.apache.org/jira/browse/FLINK-32154 >
RE: [DISCUSS] PIP-277: Clusters list API return clusters with local flag
May I make a proposal : adding `remote` flag also? #Concern : It seems like we are sort of allowing users to assume what the ones without `local` flag are (are called by the community). #Possible Benfit : Users will benefit from not having to think about what ` non-local` clusters are called. # Use-case : when someone automates counting how many remotes and clusters are there in list via shell-script, or simple String parser where the user will benefit more when grouping by `local` vs `remote`, rather than `local` vs `“”`. # Original Github Comment : https://github.com/apache/pulsar/pull/20614#issuecomment-1599025705 On 2023/06/20 06:00:29 guo jiwei wrote: > Hi community: > After configuring the geo-replication on Pulsar clusters, the `clusters > list` API will return multiple clusters, including the local Pulsar cluster > and remote clusters like > > ``` > bin/pulsar-admin clusters list > us-west > us-east > us-cent > ``` > But in this return, you can't distinguish the local and the remote cluster. > When you need to remove the geo-replication configuration, it will be hard > to decide which cluster should be removed on replicated tenants and > namespaces unless you record the cluster information. > > > ### Modification > Add `local` flag to distinguish clusters > ``` > bin/pulsar-admin clusters list > us-west(local) > us-east > us-cent > ``` > > PIP: https://github.com/apache/pulsar/pull/20614 > > > Regards > Jiwei Guo (Tboy) >
[DISCUSS] Provide consistent concept around partitioned topic and topic partition/internal topic
Hi community: Currently, it seems like there is no good definition of what "partitioned topic" and "non-partitioned topic" mean and some places topic partition and internal topic interchangeably. I would like to propose to vote (or discuss) to come up with solid definition of what composes "partitioned topic", then apply to pulsar-site, javadoc, and TopicName API accordingly. Let me know what you think # ISSUE https://github.com/apache/pulsar/issues/20622 Regards Joo Hyuk, Kim (Vince)
RE: [DISCUSS] PIP-277: Clusters list API return clusters with local flag
May I make a proposal : adding `remote` flag also? #Concern : It seems like we are sort of allowing users to assume what the ones without `local` flag are (are called by the community). #Possible Benfit : Users will benefit from not having to think about what ` non-local` clusters are called. # Use-case : when someone automates counting how many remotes and clusters are there in list via shell-script, or simple String parser where the user will benefit more when grouping by `local` vs `remote`, rather than `local` vs `“”`. # Original Github Comment : https://github.com/apache/pulsar/pull/20614#issuecomment-1599025705 On 2023/06/20 06:00:29 guo jiwei wrote: > Hi community: > After configuring the geo-replication on Pulsar clusters, the `clusters > list` API will return multiple clusters, including the local Pulsar cluster > and remote clusters like > > ``` > bin/pulsar-admin clusters list > us-west > us-east > us-cent > ``` > But in this return, you can't distinguish the local and the remote cluster. > When you need to remove the geo-replication configuration, it will be hard > to decide which cluster should be removed on replicated tenants and > namespaces unless you record the cluster information. > > > ### Modification > Add `local` flag to distinguish clusters > ``` > bin/pulsar-admin clusters list > us-west(local) > us-east > us-cent > ``` > > PIP: https://github.com/apache/pulsar/pull/20614 > > > Regards > Jiwei Guo (Tboy) >
