You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pulsar.apache.org by Asaf Mesika <as...@gmail.com> on 2023/05/01 06:12:50 UTC

Re: [Discuss] Should current `ServiceConfiguration` file split by category ?

The Config file size is the issue or the configuration pojo ?

On Sun, 30 Apr 2023 at 19:51 lifepuzzlefun <wj...@163.com> wrote:

> current file is 3470 line with all pulsar configuration.
> it is slow in my local ide.
>
>
> 59 config in loadbalancer.
> 17 config in metric.
> 64 config in policies.
> 41 config for bk client.
> 50 for managed ledger.
> 100+ for all other server cofig.
>
>
> if split by category maybe more clear
>
>
> class ServiceConfiguration {
>          LoadbalancerConfiguration
>          MetricConfiguration
>          BookkeeperClientConfiguration
>          ....
> }
>
>
> one problem is that if split in this way, most config should change access
> by conf.getLoadBalancerConfiguration().getXXXXX().
>
>
> so most of the code base will be changed.
>
>
> any ideas ?
>
>
>
>

Re: [Discuss] Should current `ServiceConfiguration` file split by category ?

Posted by Michael Marshall <mm...@apache.org>.
Perhaps another solution is to change the format of the configuration
file. The dev experience for configuring pulsar is already confusing
and adding more files would make it more confusing, not less, in my
opinion. Yunze proposed finding a way to simplify it in December [0].

yaml could give us a hierarchy and therefore a natural grouping of
like configurations.

As we look for an alternative, I think it would be valuable to provide
a way to make the configuration files immutable. That increases the
security of the environment. If we provided a way to merge
configuration files, a user could supply their config overrides via
their own file and then pulsar could merge the files with a defined
precedence. This design would be pretty similar to helm's
configuration design.

Another benefit of yaml: in kubernetes, you would create a config map
with the yaml overrides and then mount that as a file in the pulsar
component and start your broker. No need to run a special env to file
configuration script. That could move us one step closer to a
distroless docker image, as was recently discussed here [1].

Thanks,
Michael

[0] https://lists.apache.org/thread/j23ny19opmp8jww57gwk7g27b5dvl0ot
[1] https://github.com/apache/pulsar/discussions/20253


On Sun, May 7, 2023 at 8:58 AM Yunze Xu <yz...@streamnative.io.invalid> wrote:
>
> > To be clear, it's an improvement for dev experience instead of runtime performance, right?
>
> Yes. It should not affect runtime performance. And I never see the
> performance of a configuration class is a bottleneck.
>
> Thanks,
> Yunze
>
> On Sun, May 7, 2023 at 8:49 PM tison <wa...@gmail.com> wrote:
> >
> > > have a seamless performance improvement
> >
> > To be clear, it's an improvement for dev experience instead of runtime
> > performance, right?
> >
> > Best,
> > tison.
> >
> >
> > Yunze Xu <yz...@streamnative.io.invalid> 于2023年5月7日周日 20:25写道:
> >
> > > For now, I think we can use trivial getters and setters instead of
> > > lombok generated getters and setters to have a seamless performance
> > > improvement.
> > >
> > > Thanks,
> > > Yunze
> > >
> > > On Sun, May 7, 2023 at 7:49 PM Yunze Xu <yz...@streamnative.io> wrote:
> > > >
> > > > > creating a new class, and using it
> > > >
> > > > It's the commonly used way to handle breaking changes. Otherwise, we
> > > > don't need any "deprecated" annotation or something else in various
> > > > languages.
> > > >
> > > > > The biggest breakage would be the plugins no?
> > > >
> > > > Maybe. But any third party application that depends on the
> > > > pulsar-broker module and sets up its own mocked Pulsar service could
> > > > have a chance to modify the `ServiceConfiguration` via the setters.
> > > >
> > > > Thanks,
> > > > Yunze
> > > >
> > > > On Sun, May 7, 2023 at 5:39 PM Asaf Mesika <as...@gmail.com>
> > > wrote:
> > > > >
> > > > > I like doing the split.
> > > > > The biggest breakage would be the plugins no?
> > > > >
> > > > > I didn't understand creating a new class, and using it. Can you expand
> > > on
> > > > > it?
> > > > >
> > > > >
> > > > > On Fri, May 5, 2023 at 11:44 AM Yunze Xu <yzxu@streamnative.io.invalid
> > > >
> > > > > wrote:
> > > > >
> > > > > > Oh, I figured out the reason finally. It's caused by the lombok.
> > > After
> > > > > > I changed all getters and setters with the trivial getters and
> > > > > > setters, the completion time was reduced from 15 seconds to 2
> > > seconds.
> > > > > >
> > > > > > Thanks,
> > > > > > Yunze
> > > > > >
> > > > > > On Fri, May 5, 2023 at 4:30 PM Yunze Xu <yz...@streamnative.io>
> > > wrote:
> > > > > > >
> > > > > > > I think the configuration pojo is the issue. There are hundreds of
> > > > > > > getters and setters. I did an experiment in my local env. First,
> > > copy
> > > > > > > `ServiceConfiguration` to another class named
> > > > > > > `PulsarServiceConfiguration`. Then write the following code
> > > > > > >
> > > > > > > ```java
> > > > > > > PulsarServiceConfiguration conf = new PulsarServiceConfiguration();
> > > > > > > conf.
> > > > > > > ```
> > > > > > >
> > > > > > > and see how much time it would take between typing the dot symbol
> > > and
> > > > > > > the popup window being shown on my Intellij Idea.
> > > > > > >
> > > > > > > - Default: about 15 seconds
> > > > > > > - Remove the `@Getter`: about 5 seconds
> > > > > > > - Remove the `@Setter`: about 4 seconds
> > > > > > >
> > > > > > > > one problem is that if split in this way, most config should
> > > change
> > > > > > access by conf.getLoadBalancerConfiguration().getXXXXX().
> > > > > > >
> > > > > > > It's unacceptable because it's a big breaking change. My idea is to
> > > > > > > add another configuration class and mark `ServiceConfiguration` as
> > > > > > > deprecated. Once a new config was added, add it to the new class.
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Yunze
> > > > > > >
> > > > > > > On Mon, May 1, 2023 at 2:13 PM Asaf Mesika <as...@gmail.com>
> > > > > > wrote:
> > > > > > > >
> > > > > > > > The Config file size is the issue or the configuration pojo ?
> > > > > > > >
> > > > > > > > On Sun, 30 Apr 2023 at 19:51 lifepuzzlefun <wj...@163.com>
> > > wrote:
> > > > > > > >
> > > > > > > > > current file is 3470 line with all pulsar configuration.
> > > > > > > > > it is slow in my local ide.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > 59 config in loadbalancer.
> > > > > > > > > 17 config in metric.
> > > > > > > > > 64 config in policies.
> > > > > > > > > 41 config for bk client.
> > > > > > > > > 50 for managed ledger.
> > > > > > > > > 100+ for all other server cofig.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > if split by category maybe more clear
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > class ServiceConfiguration {
> > > > > > > > >          LoadbalancerConfiguration
> > > > > > > > >          MetricConfiguration
> > > > > > > > >          BookkeeperClientConfiguration
> > > > > > > > >          ....
> > > > > > > > > }
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > one problem is that if split in this way, most config should
> > > change
> > > > > > access
> > > > > > > > > by conf.getLoadBalancerConfiguration().getXXXXX().
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > so most of the code base will be changed.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > any ideas ?
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > >
> > >

Re: [Discuss] Should current `ServiceConfiguration` file split by category ?

Posted by Yunze Xu <yz...@streamnative.io.INVALID>.
> To be clear, it's an improvement for dev experience instead of runtime performance, right?

Yes. It should not affect runtime performance. And I never see the
performance of a configuration class is a bottleneck.

Thanks,
Yunze

On Sun, May 7, 2023 at 8:49 PM tison <wa...@gmail.com> wrote:
>
> > have a seamless performance improvement
>
> To be clear, it's an improvement for dev experience instead of runtime
> performance, right?
>
> Best,
> tison.
>
>
> Yunze Xu <yz...@streamnative.io.invalid> 于2023年5月7日周日 20:25写道:
>
> > For now, I think we can use trivial getters and setters instead of
> > lombok generated getters and setters to have a seamless performance
> > improvement.
> >
> > Thanks,
> > Yunze
> >
> > On Sun, May 7, 2023 at 7:49 PM Yunze Xu <yz...@streamnative.io> wrote:
> > >
> > > > creating a new class, and using it
> > >
> > > It's the commonly used way to handle breaking changes. Otherwise, we
> > > don't need any "deprecated" annotation or something else in various
> > > languages.
> > >
> > > > The biggest breakage would be the plugins no?
> > >
> > > Maybe. But any third party application that depends on the
> > > pulsar-broker module and sets up its own mocked Pulsar service could
> > > have a chance to modify the `ServiceConfiguration` via the setters.
> > >
> > > Thanks,
> > > Yunze
> > >
> > > On Sun, May 7, 2023 at 5:39 PM Asaf Mesika <as...@gmail.com>
> > wrote:
> > > >
> > > > I like doing the split.
> > > > The biggest breakage would be the plugins no?
> > > >
> > > > I didn't understand creating a new class, and using it. Can you expand
> > on
> > > > it?
> > > >
> > > >
> > > > On Fri, May 5, 2023 at 11:44 AM Yunze Xu <yzxu@streamnative.io.invalid
> > >
> > > > wrote:
> > > >
> > > > > Oh, I figured out the reason finally. It's caused by the lombok.
> > After
> > > > > I changed all getters and setters with the trivial getters and
> > > > > setters, the completion time was reduced from 15 seconds to 2
> > seconds.
> > > > >
> > > > > Thanks,
> > > > > Yunze
> > > > >
> > > > > On Fri, May 5, 2023 at 4:30 PM Yunze Xu <yz...@streamnative.io>
> > wrote:
> > > > > >
> > > > > > I think the configuration pojo is the issue. There are hundreds of
> > > > > > getters and setters. I did an experiment in my local env. First,
> > copy
> > > > > > `ServiceConfiguration` to another class named
> > > > > > `PulsarServiceConfiguration`. Then write the following code
> > > > > >
> > > > > > ```java
> > > > > > PulsarServiceConfiguration conf = new PulsarServiceConfiguration();
> > > > > > conf.
> > > > > > ```
> > > > > >
> > > > > > and see how much time it would take between typing the dot symbol
> > and
> > > > > > the popup window being shown on my Intellij Idea.
> > > > > >
> > > > > > - Default: about 15 seconds
> > > > > > - Remove the `@Getter`: about 5 seconds
> > > > > > - Remove the `@Setter`: about 4 seconds
> > > > > >
> > > > > > > one problem is that if split in this way, most config should
> > change
> > > > > access by conf.getLoadBalancerConfiguration().getXXXXX().
> > > > > >
> > > > > > It's unacceptable because it's a big breaking change. My idea is to
> > > > > > add another configuration class and mark `ServiceConfiguration` as
> > > > > > deprecated. Once a new config was added, add it to the new class.
> > > > > >
> > > > > > Thanks,
> > > > > > Yunze
> > > > > >
> > > > > > On Mon, May 1, 2023 at 2:13 PM Asaf Mesika <as...@gmail.com>
> > > > > wrote:
> > > > > > >
> > > > > > > The Config file size is the issue or the configuration pojo ?
> > > > > > >
> > > > > > > On Sun, 30 Apr 2023 at 19:51 lifepuzzlefun <wj...@163.com>
> > wrote:
> > > > > > >
> > > > > > > > current file is 3470 line with all pulsar configuration.
> > > > > > > > it is slow in my local ide.
> > > > > > > >
> > > > > > > >
> > > > > > > > 59 config in loadbalancer.
> > > > > > > > 17 config in metric.
> > > > > > > > 64 config in policies.
> > > > > > > > 41 config for bk client.
> > > > > > > > 50 for managed ledger.
> > > > > > > > 100+ for all other server cofig.
> > > > > > > >
> > > > > > > >
> > > > > > > > if split by category maybe more clear
> > > > > > > >
> > > > > > > >
> > > > > > > > class ServiceConfiguration {
> > > > > > > >          LoadbalancerConfiguration
> > > > > > > >          MetricConfiguration
> > > > > > > >          BookkeeperClientConfiguration
> > > > > > > >          ....
> > > > > > > > }
> > > > > > > >
> > > > > > > >
> > > > > > > > one problem is that if split in this way, most config should
> > change
> > > > > access
> > > > > > > > by conf.getLoadBalancerConfiguration().getXXXXX().
> > > > > > > >
> > > > > > > >
> > > > > > > > so most of the code base will be changed.
> > > > > > > >
> > > > > > > >
> > > > > > > > any ideas ?
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > >
> >

Re: [Discuss] Should current `ServiceConfiguration` file split by category ?

Posted by tison <wa...@gmail.com>.
> have a seamless performance improvement

To be clear, it's an improvement for dev experience instead of runtime
performance, right?

Best,
tison.


Yunze Xu <yz...@streamnative.io.invalid> 于2023年5月7日周日 20:25写道:

> For now, I think we can use trivial getters and setters instead of
> lombok generated getters and setters to have a seamless performance
> improvement.
>
> Thanks,
> Yunze
>
> On Sun, May 7, 2023 at 7:49 PM Yunze Xu <yz...@streamnative.io> wrote:
> >
> > > creating a new class, and using it
> >
> > It's the commonly used way to handle breaking changes. Otherwise, we
> > don't need any "deprecated" annotation or something else in various
> > languages.
> >
> > > The biggest breakage would be the plugins no?
> >
> > Maybe. But any third party application that depends on the
> > pulsar-broker module and sets up its own mocked Pulsar service could
> > have a chance to modify the `ServiceConfiguration` via the setters.
> >
> > Thanks,
> > Yunze
> >
> > On Sun, May 7, 2023 at 5:39 PM Asaf Mesika <as...@gmail.com>
> wrote:
> > >
> > > I like doing the split.
> > > The biggest breakage would be the plugins no?
> > >
> > > I didn't understand creating a new class, and using it. Can you expand
> on
> > > it?
> > >
> > >
> > > On Fri, May 5, 2023 at 11:44 AM Yunze Xu <yzxu@streamnative.io.invalid
> >
> > > wrote:
> > >
> > > > Oh, I figured out the reason finally. It's caused by the lombok.
> After
> > > > I changed all getters and setters with the trivial getters and
> > > > setters, the completion time was reduced from 15 seconds to 2
> seconds.
> > > >
> > > > Thanks,
> > > > Yunze
> > > >
> > > > On Fri, May 5, 2023 at 4:30 PM Yunze Xu <yz...@streamnative.io>
> wrote:
> > > > >
> > > > > I think the configuration pojo is the issue. There are hundreds of
> > > > > getters and setters. I did an experiment in my local env. First,
> copy
> > > > > `ServiceConfiguration` to another class named
> > > > > `PulsarServiceConfiguration`. Then write the following code
> > > > >
> > > > > ```java
> > > > > PulsarServiceConfiguration conf = new PulsarServiceConfiguration();
> > > > > conf.
> > > > > ```
> > > > >
> > > > > and see how much time it would take between typing the dot symbol
> and
> > > > > the popup window being shown on my Intellij Idea.
> > > > >
> > > > > - Default: about 15 seconds
> > > > > - Remove the `@Getter`: about 5 seconds
> > > > > - Remove the `@Setter`: about 4 seconds
> > > > >
> > > > > > one problem is that if split in this way, most config should
> change
> > > > access by conf.getLoadBalancerConfiguration().getXXXXX().
> > > > >
> > > > > It's unacceptable because it's a big breaking change. My idea is to
> > > > > add another configuration class and mark `ServiceConfiguration` as
> > > > > deprecated. Once a new config was added, add it to the new class.
> > > > >
> > > > > Thanks,
> > > > > Yunze
> > > > >
> > > > > On Mon, May 1, 2023 at 2:13 PM Asaf Mesika <as...@gmail.com>
> > > > wrote:
> > > > > >
> > > > > > The Config file size is the issue or the configuration pojo ?
> > > > > >
> > > > > > On Sun, 30 Apr 2023 at 19:51 lifepuzzlefun <wj...@163.com>
> wrote:
> > > > > >
> > > > > > > current file is 3470 line with all pulsar configuration.
> > > > > > > it is slow in my local ide.
> > > > > > >
> > > > > > >
> > > > > > > 59 config in loadbalancer.
> > > > > > > 17 config in metric.
> > > > > > > 64 config in policies.
> > > > > > > 41 config for bk client.
> > > > > > > 50 for managed ledger.
> > > > > > > 100+ for all other server cofig.
> > > > > > >
> > > > > > >
> > > > > > > if split by category maybe more clear
> > > > > > >
> > > > > > >
> > > > > > > class ServiceConfiguration {
> > > > > > >          LoadbalancerConfiguration
> > > > > > >          MetricConfiguration
> > > > > > >          BookkeeperClientConfiguration
> > > > > > >          ....
> > > > > > > }
> > > > > > >
> > > > > > >
> > > > > > > one problem is that if split in this way, most config should
> change
> > > > access
> > > > > > > by conf.getLoadBalancerConfiguration().getXXXXX().
> > > > > > >
> > > > > > >
> > > > > > > so most of the code base will be changed.
> > > > > > >
> > > > > > >
> > > > > > > any ideas ?
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > >
>

Re: [Discuss] Should current `ServiceConfiguration` file split by category ?

Posted by Yunze Xu <yz...@streamnative.io.INVALID>.
For now, I think we can use trivial getters and setters instead of
lombok generated getters and setters to have a seamless performance
improvement.

Thanks,
Yunze

On Sun, May 7, 2023 at 7:49 PM Yunze Xu <yz...@streamnative.io> wrote:
>
> > creating a new class, and using it
>
> It's the commonly used way to handle breaking changes. Otherwise, we
> don't need any "deprecated" annotation or something else in various
> languages.
>
> > The biggest breakage would be the plugins no?
>
> Maybe. But any third party application that depends on the
> pulsar-broker module and sets up its own mocked Pulsar service could
> have a chance to modify the `ServiceConfiguration` via the setters.
>
> Thanks,
> Yunze
>
> On Sun, May 7, 2023 at 5:39 PM Asaf Mesika <as...@gmail.com> wrote:
> >
> > I like doing the split.
> > The biggest breakage would be the plugins no?
> >
> > I didn't understand creating a new class, and using it. Can you expand on
> > it?
> >
> >
> > On Fri, May 5, 2023 at 11:44 AM Yunze Xu <yz...@streamnative.io.invalid>
> > wrote:
> >
> > > Oh, I figured out the reason finally. It's caused by the lombok. After
> > > I changed all getters and setters with the trivial getters and
> > > setters, the completion time was reduced from 15 seconds to 2 seconds.
> > >
> > > Thanks,
> > > Yunze
> > >
> > > On Fri, May 5, 2023 at 4:30 PM Yunze Xu <yz...@streamnative.io> wrote:
> > > >
> > > > I think the configuration pojo is the issue. There are hundreds of
> > > > getters and setters. I did an experiment in my local env. First, copy
> > > > `ServiceConfiguration` to another class named
> > > > `PulsarServiceConfiguration`. Then write the following code
> > > >
> > > > ```java
> > > > PulsarServiceConfiguration conf = new PulsarServiceConfiguration();
> > > > conf.
> > > > ```
> > > >
> > > > and see how much time it would take between typing the dot symbol and
> > > > the popup window being shown on my Intellij Idea.
> > > >
> > > > - Default: about 15 seconds
> > > > - Remove the `@Getter`: about 5 seconds
> > > > - Remove the `@Setter`: about 4 seconds
> > > >
> > > > > one problem is that if split in this way, most config should change
> > > access by conf.getLoadBalancerConfiguration().getXXXXX().
> > > >
> > > > It's unacceptable because it's a big breaking change. My idea is to
> > > > add another configuration class and mark `ServiceConfiguration` as
> > > > deprecated. Once a new config was added, add it to the new class.
> > > >
> > > > Thanks,
> > > > Yunze
> > > >
> > > > On Mon, May 1, 2023 at 2:13 PM Asaf Mesika <as...@gmail.com>
> > > wrote:
> > > > >
> > > > > The Config file size is the issue or the configuration pojo ?
> > > > >
> > > > > On Sun, 30 Apr 2023 at 19:51 lifepuzzlefun <wj...@163.com> wrote:
> > > > >
> > > > > > current file is 3470 line with all pulsar configuration.
> > > > > > it is slow in my local ide.
> > > > > >
> > > > > >
> > > > > > 59 config in loadbalancer.
> > > > > > 17 config in metric.
> > > > > > 64 config in policies.
> > > > > > 41 config for bk client.
> > > > > > 50 for managed ledger.
> > > > > > 100+ for all other server cofig.
> > > > > >
> > > > > >
> > > > > > if split by category maybe more clear
> > > > > >
> > > > > >
> > > > > > class ServiceConfiguration {
> > > > > >          LoadbalancerConfiguration
> > > > > >          MetricConfiguration
> > > > > >          BookkeeperClientConfiguration
> > > > > >          ....
> > > > > > }
> > > > > >
> > > > > >
> > > > > > one problem is that if split in this way, most config should change
> > > access
> > > > > > by conf.getLoadBalancerConfiguration().getXXXXX().
> > > > > >
> > > > > >
> > > > > > so most of the code base will be changed.
> > > > > >
> > > > > >
> > > > > > any ideas ?
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > >

Re: [Discuss] Should current `ServiceConfiguration` file split by category ?

Posted by Yunze Xu <yz...@streamnative.io.INVALID>.
> creating a new class, and using it

It's the commonly used way to handle breaking changes. Otherwise, we
don't need any "deprecated" annotation or something else in various
languages.

> The biggest breakage would be the plugins no?

Maybe. But any third party application that depends on the
pulsar-broker module and sets up its own mocked Pulsar service could
have a chance to modify the `ServiceConfiguration` via the setters.

Thanks,
Yunze

On Sun, May 7, 2023 at 5:39 PM Asaf Mesika <as...@gmail.com> wrote:
>
> I like doing the split.
> The biggest breakage would be the plugins no?
>
> I didn't understand creating a new class, and using it. Can you expand on
> it?
>
>
> On Fri, May 5, 2023 at 11:44 AM Yunze Xu <yz...@streamnative.io.invalid>
> wrote:
>
> > Oh, I figured out the reason finally. It's caused by the lombok. After
> > I changed all getters and setters with the trivial getters and
> > setters, the completion time was reduced from 15 seconds to 2 seconds.
> >
> > Thanks,
> > Yunze
> >
> > On Fri, May 5, 2023 at 4:30 PM Yunze Xu <yz...@streamnative.io> wrote:
> > >
> > > I think the configuration pojo is the issue. There are hundreds of
> > > getters and setters. I did an experiment in my local env. First, copy
> > > `ServiceConfiguration` to another class named
> > > `PulsarServiceConfiguration`. Then write the following code
> > >
> > > ```java
> > > PulsarServiceConfiguration conf = new PulsarServiceConfiguration();
> > > conf.
> > > ```
> > >
> > > and see how much time it would take between typing the dot symbol and
> > > the popup window being shown on my Intellij Idea.
> > >
> > > - Default: about 15 seconds
> > > - Remove the `@Getter`: about 5 seconds
> > > - Remove the `@Setter`: about 4 seconds
> > >
> > > > one problem is that if split in this way, most config should change
> > access by conf.getLoadBalancerConfiguration().getXXXXX().
> > >
> > > It's unacceptable because it's a big breaking change. My idea is to
> > > add another configuration class and mark `ServiceConfiguration` as
> > > deprecated. Once a new config was added, add it to the new class.
> > >
> > > Thanks,
> > > Yunze
> > >
> > > On Mon, May 1, 2023 at 2:13 PM Asaf Mesika <as...@gmail.com>
> > wrote:
> > > >
> > > > The Config file size is the issue or the configuration pojo ?
> > > >
> > > > On Sun, 30 Apr 2023 at 19:51 lifepuzzlefun <wj...@163.com> wrote:
> > > >
> > > > > current file is 3470 line with all pulsar configuration.
> > > > > it is slow in my local ide.
> > > > >
> > > > >
> > > > > 59 config in loadbalancer.
> > > > > 17 config in metric.
> > > > > 64 config in policies.
> > > > > 41 config for bk client.
> > > > > 50 for managed ledger.
> > > > > 100+ for all other server cofig.
> > > > >
> > > > >
> > > > > if split by category maybe more clear
> > > > >
> > > > >
> > > > > class ServiceConfiguration {
> > > > >          LoadbalancerConfiguration
> > > > >          MetricConfiguration
> > > > >          BookkeeperClientConfiguration
> > > > >          ....
> > > > > }
> > > > >
> > > > >
> > > > > one problem is that if split in this way, most config should change
> > access
> > > > > by conf.getLoadBalancerConfiguration().getXXXXX().
> > > > >
> > > > >
> > > > > so most of the code base will be changed.
> > > > >
> > > > >
> > > > > any ideas ?
> > > > >
> > > > >
> > > > >
> > > > >
> >

Re: [Discuss] Should current `ServiceConfiguration` file split by category ?

Posted by Asaf Mesika <as...@gmail.com>.
I like doing the split.
The biggest breakage would be the plugins no?

I didn't understand creating a new class, and using it. Can you expand on
it?


On Fri, May 5, 2023 at 11:44 AM Yunze Xu <yz...@streamnative.io.invalid>
wrote:

> Oh, I figured out the reason finally. It's caused by the lombok. After
> I changed all getters and setters with the trivial getters and
> setters, the completion time was reduced from 15 seconds to 2 seconds.
>
> Thanks,
> Yunze
>
> On Fri, May 5, 2023 at 4:30 PM Yunze Xu <yz...@streamnative.io> wrote:
> >
> > I think the configuration pojo is the issue. There are hundreds of
> > getters and setters. I did an experiment in my local env. First, copy
> > `ServiceConfiguration` to another class named
> > `PulsarServiceConfiguration`. Then write the following code
> >
> > ```java
> > PulsarServiceConfiguration conf = new PulsarServiceConfiguration();
> > conf.
> > ```
> >
> > and see how much time it would take between typing the dot symbol and
> > the popup window being shown on my Intellij Idea.
> >
> > - Default: about 15 seconds
> > - Remove the `@Getter`: about 5 seconds
> > - Remove the `@Setter`: about 4 seconds
> >
> > > one problem is that if split in this way, most config should change
> access by conf.getLoadBalancerConfiguration().getXXXXX().
> >
> > It's unacceptable because it's a big breaking change. My idea is to
> > add another configuration class and mark `ServiceConfiguration` as
> > deprecated. Once a new config was added, add it to the new class.
> >
> > Thanks,
> > Yunze
> >
> > On Mon, May 1, 2023 at 2:13 PM Asaf Mesika <as...@gmail.com>
> wrote:
> > >
> > > The Config file size is the issue or the configuration pojo ?
> > >
> > > On Sun, 30 Apr 2023 at 19:51 lifepuzzlefun <wj...@163.com> wrote:
> > >
> > > > current file is 3470 line with all pulsar configuration.
> > > > it is slow in my local ide.
> > > >
> > > >
> > > > 59 config in loadbalancer.
> > > > 17 config in metric.
> > > > 64 config in policies.
> > > > 41 config for bk client.
> > > > 50 for managed ledger.
> > > > 100+ for all other server cofig.
> > > >
> > > >
> > > > if split by category maybe more clear
> > > >
> > > >
> > > > class ServiceConfiguration {
> > > >          LoadbalancerConfiguration
> > > >          MetricConfiguration
> > > >          BookkeeperClientConfiguration
> > > >          ....
> > > > }
> > > >
> > > >
> > > > one problem is that if split in this way, most config should change
> access
> > > > by conf.getLoadBalancerConfiguration().getXXXXX().
> > > >
> > > >
> > > > so most of the code base will be changed.
> > > >
> > > >
> > > > any ideas ?
> > > >
> > > >
> > > >
> > > >
>

Re: [Discuss] Should current `ServiceConfiguration` file split by category ?

Posted by Yunze Xu <yz...@streamnative.io.INVALID>.
Oh, I figured out the reason finally. It's caused by the lombok. After
I changed all getters and setters with the trivial getters and
setters, the completion time was reduced from 15 seconds to 2 seconds.

Thanks,
Yunze

On Fri, May 5, 2023 at 4:30 PM Yunze Xu <yz...@streamnative.io> wrote:
>
> I think the configuration pojo is the issue. There are hundreds of
> getters and setters. I did an experiment in my local env. First, copy
> `ServiceConfiguration` to another class named
> `PulsarServiceConfiguration`. Then write the following code
>
> ```java
> PulsarServiceConfiguration conf = new PulsarServiceConfiguration();
> conf.
> ```
>
> and see how much time it would take between typing the dot symbol and
> the popup window being shown on my Intellij Idea.
>
> - Default: about 15 seconds
> - Remove the `@Getter`: about 5 seconds
> - Remove the `@Setter`: about 4 seconds
>
> > one problem is that if split in this way, most config should change access by conf.getLoadBalancerConfiguration().getXXXXX().
>
> It's unacceptable because it's a big breaking change. My idea is to
> add another configuration class and mark `ServiceConfiguration` as
> deprecated. Once a new config was added, add it to the new class.
>
> Thanks,
> Yunze
>
> On Mon, May 1, 2023 at 2:13 PM Asaf Mesika <as...@gmail.com> wrote:
> >
> > The Config file size is the issue or the configuration pojo ?
> >
> > On Sun, 30 Apr 2023 at 19:51 lifepuzzlefun <wj...@163.com> wrote:
> >
> > > current file is 3470 line with all pulsar configuration.
> > > it is slow in my local ide.
> > >
> > >
> > > 59 config in loadbalancer.
> > > 17 config in metric.
> > > 64 config in policies.
> > > 41 config for bk client.
> > > 50 for managed ledger.
> > > 100+ for all other server cofig.
> > >
> > >
> > > if split by category maybe more clear
> > >
> > >
> > > class ServiceConfiguration {
> > >          LoadbalancerConfiguration
> > >          MetricConfiguration
> > >          BookkeeperClientConfiguration
> > >          ....
> > > }
> > >
> > >
> > > one problem is that if split in this way, most config should change access
> > > by conf.getLoadBalancerConfiguration().getXXXXX().
> > >
> > >
> > > so most of the code base will be changed.
> > >
> > >
> > > any ideas ?
> > >
> > >
> > >
> > >

Re: [Discuss] Should current `ServiceConfiguration` file split by category ?

Posted by Yunze Xu <yz...@streamnative.io.INVALID>.
I think the configuration pojo is the issue. There are hundreds of
getters and setters. I did an experiment in my local env. First, copy
`ServiceConfiguration` to another class named
`PulsarServiceConfiguration`. Then write the following code

```java
PulsarServiceConfiguration conf = new PulsarServiceConfiguration();
conf.
```

and see how much time it would take between typing the dot symbol and
the popup window being shown on my Intellij Idea.

- Default: about 15 seconds
- Remove the `@Getter`: about 5 seconds
- Remove the `@Setter`: about 4 seconds

> one problem is that if split in this way, most config should change access by conf.getLoadBalancerConfiguration().getXXXXX().

It's unacceptable because it's a big breaking change. My idea is to
add another configuration class and mark `ServiceConfiguration` as
deprecated. Once a new config was added, add it to the new class.

Thanks,
Yunze

On Mon, May 1, 2023 at 2:13 PM Asaf Mesika <as...@gmail.com> wrote:
>
> The Config file size is the issue or the configuration pojo ?
>
> On Sun, 30 Apr 2023 at 19:51 lifepuzzlefun <wj...@163.com> wrote:
>
> > current file is 3470 line with all pulsar configuration.
> > it is slow in my local ide.
> >
> >
> > 59 config in loadbalancer.
> > 17 config in metric.
> > 64 config in policies.
> > 41 config for bk client.
> > 50 for managed ledger.
> > 100+ for all other server cofig.
> >
> >
> > if split by category maybe more clear
> >
> >
> > class ServiceConfiguration {
> >          LoadbalancerConfiguration
> >          MetricConfiguration
> >          BookkeeperClientConfiguration
> >          ....
> > }
> >
> >
> > one problem is that if split in this way, most config should change access
> > by conf.getLoadBalancerConfiguration().getXXXXX().
> >
> >
> > so most of the code base will be changed.
> >
> >
> > any ideas ?
> >
> >
> >
> >