You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pulsar.apache.org by houxiaoyu <an...@gmail.com> on 2023/06/06 05:55:54 UTC

[DISCUSS] PIP-275: Introduce numWorkerThreadsForPersistentTopic to deprecate numWorkerThreadsForNonPersistentTopic in configuration

Hi Pulsar Community,

I am writing to start the discussion on PIP-275: Introduce
numWorkerThreadsForPersistentTopic to deprecate
numWorkerThreadsForNonPersistentTopic in configuration

PR with PIP contents: https://github.com/apache/pulsar/pull/20507

# Motivation

Introduce `numWorkerThreadsForPersistentTopic` to deprecate
`numWorkerThreadsForNonPersistentTopic`.

The `numWorkerThreadsForNonPersistentTopic` is used to specify for
PersistentTopic, not NonPersistentTopic. So I propose change the config
item from `numWorkerThreadsForNonPersistentTopic` to
`numWorkerThreadsForPersistentTopic`:

Thanks,
Xiaoyu Hou

Re: [DISCUSS] PIP-275: Introduce numWorkerThreadsForPersistentTopic to deprecate numWorkerThreadsForNonPersistentTopic in configuration

Posted by houxiaoyu <an...@gmail.com>.
Updated the new Introduced name as `topicOrderedExecutorThreadNum`.

Is there any other suggestions? Or I will start the VOTE later. :)

Thanks
Xiaoyu Hou

houxiaoyu <an...@gmail.com> 于2023年6月8日周四 13:07写道:

> Hi Enrico
>
> `numWorkersTopicOrderedExecutor` is OK. I have updated the pip
>
> Thanks,
> Xiaoyu Hou
>
> Enrico Olivelli <eo...@gmail.com> 于2023年6月7日周三 15:44写道:
>
>> I would call it numWorkersTopicOrderedExecutor and remove
>> "persistent/non-persistent" from the name.
>>
>> Or alternatively we could introduce a topicOrderedExecutor for
>> persistent topics.
>>
>> Unfortunately the topicOrderedExecutor is now used for many things (I
>> also used it the wrong way when I introduced some features, sorry
>> about that).
>> Initially it was meant only for non-persistent topics, but now it is
>> used for anything that needs to be done under strict order for a
>> topic, like processing Subscriptions even for a persistent topic.
>>
>> Enrico
>>
>> Il giorno mer 7 giu 2023 alle ore 07:28 houxiaoyu
>> <an...@gmail.com> ha scritto:
>> >
>> > Hi Rajan,
>> > Thanks for your reply.
>> >
>> > The `numWorkerThreadsForNonPersistentTopic` will specify the thread num
>> of
>> > `BrokerService#topicOrderedExecutor` [0].
>> > However, the `topicOrderedExecutor` are both used for `persistent` and
>> > `non-persistent` topics, not just for `non-persistent`:
>> > * There is only one place invoke `topicOrderedExecutor` for
>> > `non-persistent` topics. [1]
>> > * Other places will invoke `topicOrderedExecutor` for `persistent`
>> topic,
>> > e.g., [2][3][4][5]
>> >
>> > In short, `numWorkerThreadsForNonPersistentTopic` is not the `Number of
>> > worker threads to serve non-persistent topic` only.  So how about change
>> > the name to `numWorkerThreadsTopic`
>> >
>> >
>> > [0]
>> >
>> https://github.com/apache/pulsar/blob/50b9a93e42e412d9f17b1637287d1a4c7c7ab148/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java#L317C14-L320
>> > [1]
>> >
>> https://github.com/apache/pulsar/blob/50b9a93e42e412d9f17b1637287d1a4c7c7ab148/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java#L1705-L1709
>> > [2]
>> >
>> https://github.com/apache/pulsar/blob/50b9a93e42e412d9f17b1637287d1a4c7c7ab148/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumers.java#L134-L142
>> > [3]
>> >
>> https://github.com/apache/pulsar/blob/50b9a93e42e412d9f17b1637287d1a4c7c7ab148/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java#L279-L281
>> > [4]
>> >
>> https://github.com/apache/pulsar/blob/50b9a93e42e412d9f17b1637287d1a4c7c7ab148/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherSingleActiveConsumer.java#L77-L83
>> > [5]
>> >
>> https://github.com/apache/pulsar/blob/50b9a93e42e412d9f17b1637287d1a4c7c7ab148/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentStickyKeyDispatcherMultipleConsumers.java#L392-L396
>> >
>> > Thanks
>> >
>> >
>> >
>> >
>> > Rajan Dhabalia <rd...@apache.org> 于2023年6月6日周二 14:57写道:
>> >
>> > > Hi,
>> > >
>> > > We already have a default number of threads for persistent topics but
>> we
>> > > have added a feature non-persistent topics and to isolate that path we
>> > > introduced a number of worker threads which we can reduce or tune
>> based on
>> > > resources we would like to allocate for non-persistent topics. So, it
>> > > really doesn't make sense and I don't see any clear reason in this
>> PIP why
>> > > we would like to take away control to tune thread resources to
>> > > non-persistent topics.
>> > >
>> > > Thanks,
>> > > Rajan
>> > >
>> > > On Mon, Jun 5, 2023 at 10:56 PM houxiaoyu <an...@gmail.com>
>> wrote:
>> > >
>> > > > Hi Pulsar Community,
>> > > >
>> > > > I am writing to start the discussion on PIP-275: Introduce
>> > > > numWorkerThreadsForPersistentTopic to deprecate
>> > > > numWorkerThreadsForNonPersistentTopic in configuration
>> > > >
>> > > > PR with PIP contents: https://github.com/apache/pulsar/pull/20507
>> > > >
>> > > > # Motivation
>> > > >
>> > > > Introduce `numWorkerThreadsForPersistentTopic` to deprecate
>> > > > `numWorkerThreadsForNonPersistentTopic`.
>> > > >
>> > > > The `numWorkerThreadsForNonPersistentTopic` is used to specify for
>> > > > PersistentTopic, not NonPersistentTopic. So I propose change the
>> config
>> > > > item from `numWorkerThreadsForNonPersistentTopic` to
>> > > > `numWorkerThreadsForPersistentTopic`:
>> > > >
>> > > > Thanks,
>> > > > Xiaoyu Hou
>> > > >
>> > >
>>
>

Re: [DISCUSS] PIP-275: Introduce numWorkerThreadsForPersistentTopic to deprecate numWorkerThreadsForNonPersistentTopic in configuration

Posted by houxiaoyu <an...@gmail.com>.
Hi Enrico

`numWorkersTopicOrderedExecutor` is OK. I have updated the pip

Thanks,
Xiaoyu Hou

Enrico Olivelli <eo...@gmail.com> 于2023年6月7日周三 15:44写道:

> I would call it numWorkersTopicOrderedExecutor and remove
> "persistent/non-persistent" from the name.
>
> Or alternatively we could introduce a topicOrderedExecutor for
> persistent topics.
>
> Unfortunately the topicOrderedExecutor is now used for many things (I
> also used it the wrong way when I introduced some features, sorry
> about that).
> Initially it was meant only for non-persistent topics, but now it is
> used for anything that needs to be done under strict order for a
> topic, like processing Subscriptions even for a persistent topic.
>
> Enrico
>
> Il giorno mer 7 giu 2023 alle ore 07:28 houxiaoyu
> <an...@gmail.com> ha scritto:
> >
> > Hi Rajan,
> > Thanks for your reply.
> >
> > The `numWorkerThreadsForNonPersistentTopic` will specify the thread num
> of
> > `BrokerService#topicOrderedExecutor` [0].
> > However, the `topicOrderedExecutor` are both used for `persistent` and
> > `non-persistent` topics, not just for `non-persistent`:
> > * There is only one place invoke `topicOrderedExecutor` for
> > `non-persistent` topics. [1]
> > * Other places will invoke `topicOrderedExecutor` for `persistent` topic,
> > e.g., [2][3][4][5]
> >
> > In short, `numWorkerThreadsForNonPersistentTopic` is not the `Number of
> > worker threads to serve non-persistent topic` only.  So how about change
> > the name to `numWorkerThreadsTopic`
> >
> >
> > [0]
> >
> https://github.com/apache/pulsar/blob/50b9a93e42e412d9f17b1637287d1a4c7c7ab148/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java#L317C14-L320
> > [1]
> >
> https://github.com/apache/pulsar/blob/50b9a93e42e412d9f17b1637287d1a4c7c7ab148/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java#L1705-L1709
> > [2]
> >
> https://github.com/apache/pulsar/blob/50b9a93e42e412d9f17b1637287d1a4c7c7ab148/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumers.java#L134-L142
> > [3]
> >
> https://github.com/apache/pulsar/blob/50b9a93e42e412d9f17b1637287d1a4c7c7ab148/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java#L279-L281
> > [4]
> >
> https://github.com/apache/pulsar/blob/50b9a93e42e412d9f17b1637287d1a4c7c7ab148/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherSingleActiveConsumer.java#L77-L83
> > [5]
> >
> https://github.com/apache/pulsar/blob/50b9a93e42e412d9f17b1637287d1a4c7c7ab148/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentStickyKeyDispatcherMultipleConsumers.java#L392-L396
> >
> > Thanks
> >
> >
> >
> >
> > Rajan Dhabalia <rd...@apache.org> 于2023年6月6日周二 14:57写道:
> >
> > > Hi,
> > >
> > > We already have a default number of threads for persistent topics but
> we
> > > have added a feature non-persistent topics and to isolate that path we
> > > introduced a number of worker threads which we can reduce or tune
> based on
> > > resources we would like to allocate for non-persistent topics. So, it
> > > really doesn't make sense and I don't see any clear reason in this PIP
> why
> > > we would like to take away control to tune thread resources to
> > > non-persistent topics.
> > >
> > > Thanks,
> > > Rajan
> > >
> > > On Mon, Jun 5, 2023 at 10:56 PM houxiaoyu <an...@gmail.com> wrote:
> > >
> > > > Hi Pulsar Community,
> > > >
> > > > I am writing to start the discussion on PIP-275: Introduce
> > > > numWorkerThreadsForPersistentTopic to deprecate
> > > > numWorkerThreadsForNonPersistentTopic in configuration
> > > >
> > > > PR with PIP contents: https://github.com/apache/pulsar/pull/20507
> > > >
> > > > # Motivation
> > > >
> > > > Introduce `numWorkerThreadsForPersistentTopic` to deprecate
> > > > `numWorkerThreadsForNonPersistentTopic`.
> > > >
> > > > The `numWorkerThreadsForNonPersistentTopic` is used to specify for
> > > > PersistentTopic, not NonPersistentTopic. So I propose change the
> config
> > > > item from `numWorkerThreadsForNonPersistentTopic` to
> > > > `numWorkerThreadsForPersistentTopic`:
> > > >
> > > > Thanks,
> > > > Xiaoyu Hou
> > > >
> > >
>

Re: [DISCUSS] PIP-275: Introduce numWorkerThreadsForPersistentTopic to deprecate numWorkerThreadsForNonPersistentTopic in configuration

Posted by Enrico Olivelli <eo...@gmail.com>.
I would call it numWorkersTopicOrderedExecutor and remove
"persistent/non-persistent" from the name.

Or alternatively we could introduce a topicOrderedExecutor for
persistent topics.

Unfortunately the topicOrderedExecutor is now used for many things (I
also used it the wrong way when I introduced some features, sorry
about that).
Initially it was meant only for non-persistent topics, but now it is
used for anything that needs to be done under strict order for a
topic, like processing Subscriptions even for a persistent topic.

Enrico

Il giorno mer 7 giu 2023 alle ore 07:28 houxiaoyu
<an...@gmail.com> ha scritto:
>
> Hi Rajan,
> Thanks for your reply.
>
> The `numWorkerThreadsForNonPersistentTopic` will specify the thread num of
> `BrokerService#topicOrderedExecutor` [0].
> However, the `topicOrderedExecutor` are both used for `persistent` and
> `non-persistent` topics, not just for `non-persistent`:
> * There is only one place invoke `topicOrderedExecutor` for
> `non-persistent` topics. [1]
> * Other places will invoke `topicOrderedExecutor` for `persistent` topic,
> e.g., [2][3][4][5]
>
> In short, `numWorkerThreadsForNonPersistentTopic` is not the `Number of
> worker threads to serve non-persistent topic` only.  So how about change
> the name to `numWorkerThreadsTopic`
>
>
> [0]
> https://github.com/apache/pulsar/blob/50b9a93e42e412d9f17b1637287d1a4c7c7ab148/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java#L317C14-L320
> [1]
> https://github.com/apache/pulsar/blob/50b9a93e42e412d9f17b1637287d1a4c7c7ab148/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java#L1705-L1709
> [2]
> https://github.com/apache/pulsar/blob/50b9a93e42e412d9f17b1637287d1a4c7c7ab148/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumers.java#L134-L142
> [3]
> https://github.com/apache/pulsar/blob/50b9a93e42e412d9f17b1637287d1a4c7c7ab148/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java#L279-L281
> [4]
> https://github.com/apache/pulsar/blob/50b9a93e42e412d9f17b1637287d1a4c7c7ab148/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherSingleActiveConsumer.java#L77-L83
> [5]
> https://github.com/apache/pulsar/blob/50b9a93e42e412d9f17b1637287d1a4c7c7ab148/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentStickyKeyDispatcherMultipleConsumers.java#L392-L396
>
> Thanks
>
>
>
>
> Rajan Dhabalia <rd...@apache.org> 于2023年6月6日周二 14:57写道:
>
> > Hi,
> >
> > We already have a default number of threads for persistent topics but we
> > have added a feature non-persistent topics and to isolate that path we
> > introduced a number of worker threads which we can reduce or tune based on
> > resources we would like to allocate for non-persistent topics. So, it
> > really doesn't make sense and I don't see any clear reason in this PIP why
> > we would like to take away control to tune thread resources to
> > non-persistent topics.
> >
> > Thanks,
> > Rajan
> >
> > On Mon, Jun 5, 2023 at 10:56 PM houxiaoyu <an...@gmail.com> wrote:
> >
> > > Hi Pulsar Community,
> > >
> > > I am writing to start the discussion on PIP-275: Introduce
> > > numWorkerThreadsForPersistentTopic to deprecate
> > > numWorkerThreadsForNonPersistentTopic in configuration
> > >
> > > PR with PIP contents: https://github.com/apache/pulsar/pull/20507
> > >
> > > # Motivation
> > >
> > > Introduce `numWorkerThreadsForPersistentTopic` to deprecate
> > > `numWorkerThreadsForNonPersistentTopic`.
> > >
> > > The `numWorkerThreadsForNonPersistentTopic` is used to specify for
> > > PersistentTopic, not NonPersistentTopic. So I propose change the config
> > > item from `numWorkerThreadsForNonPersistentTopic` to
> > > `numWorkerThreadsForPersistentTopic`:
> > >
> > > Thanks,
> > > Xiaoyu Hou
> > >
> >

Re: [DISCUSS] PIP-275: Introduce numWorkerThreadsForPersistentTopic to deprecate numWorkerThreadsForNonPersistentTopic in configuration

Posted by houxiaoyu <an...@gmail.com>.
Hi Rajan,
Thanks for your reply.

The `numWorkerThreadsForNonPersistentTopic` will specify the thread num of
`BrokerService#topicOrderedExecutor` [0].
However, the `topicOrderedExecutor` are both used for `persistent` and
`non-persistent` topics, not just for `non-persistent`:
* There is only one place invoke `topicOrderedExecutor` for
`non-persistent` topics. [1]
* Other places will invoke `topicOrderedExecutor` for `persistent` topic,
e.g., [2][3][4][5]

In short, `numWorkerThreadsForNonPersistentTopic` is not the `Number of
worker threads to serve non-persistent topic` only.  So how about change
the name to `numWorkerThreadsTopic`


[0]
https://github.com/apache/pulsar/blob/50b9a93e42e412d9f17b1637287d1a4c7c7ab148/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java#L317C14-L320
[1]
https://github.com/apache/pulsar/blob/50b9a93e42e412d9f17b1637287d1a4c7c7ab148/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java#L1705-L1709
[2]
https://github.com/apache/pulsar/blob/50b9a93e42e412d9f17b1637287d1a4c7c7ab148/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumers.java#L134-L142
[3]
https://github.com/apache/pulsar/blob/50b9a93e42e412d9f17b1637287d1a4c7c7ab148/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java#L279-L281
[4]
https://github.com/apache/pulsar/blob/50b9a93e42e412d9f17b1637287d1a4c7c7ab148/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherSingleActiveConsumer.java#L77-L83
[5]
https://github.com/apache/pulsar/blob/50b9a93e42e412d9f17b1637287d1a4c7c7ab148/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentStickyKeyDispatcherMultipleConsumers.java#L392-L396

Thanks




Rajan Dhabalia <rd...@apache.org> 于2023年6月6日周二 14:57写道:

> Hi,
>
> We already have a default number of threads for persistent topics but we
> have added a feature non-persistent topics and to isolate that path we
> introduced a number of worker threads which we can reduce or tune based on
> resources we would like to allocate for non-persistent topics. So, it
> really doesn't make sense and I don't see any clear reason in this PIP why
> we would like to take away control to tune thread resources to
> non-persistent topics.
>
> Thanks,
> Rajan
>
> On Mon, Jun 5, 2023 at 10:56 PM houxiaoyu <an...@gmail.com> wrote:
>
> > Hi Pulsar Community,
> >
> > I am writing to start the discussion on PIP-275: Introduce
> > numWorkerThreadsForPersistentTopic to deprecate
> > numWorkerThreadsForNonPersistentTopic in configuration
> >
> > PR with PIP contents: https://github.com/apache/pulsar/pull/20507
> >
> > # Motivation
> >
> > Introduce `numWorkerThreadsForPersistentTopic` to deprecate
> > `numWorkerThreadsForNonPersistentTopic`.
> >
> > The `numWorkerThreadsForNonPersistentTopic` is used to specify for
> > PersistentTopic, not NonPersistentTopic. So I propose change the config
> > item from `numWorkerThreadsForNonPersistentTopic` to
> > `numWorkerThreadsForPersistentTopic`:
> >
> > Thanks,
> > Xiaoyu Hou
> >
>

Re: [DISCUSS] PIP-275: Introduce numWorkerThreadsForPersistentTopic to deprecate numWorkerThreadsForNonPersistentTopic in configuration

Posted by Rajan Dhabalia <rd...@apache.org>.
Hi,

We already have a default number of threads for persistent topics but we
have added a feature non-persistent topics and to isolate that path we
introduced a number of worker threads which we can reduce or tune based on
resources we would like to allocate for non-persistent topics. So, it
really doesn't make sense and I don't see any clear reason in this PIP why
we would like to take away control to tune thread resources to
non-persistent topics.

Thanks,
Rajan

On Mon, Jun 5, 2023 at 10:56 PM houxiaoyu <an...@gmail.com> wrote:

> Hi Pulsar Community,
>
> I am writing to start the discussion on PIP-275: Introduce
> numWorkerThreadsForPersistentTopic to deprecate
> numWorkerThreadsForNonPersistentTopic in configuration
>
> PR with PIP contents: https://github.com/apache/pulsar/pull/20507
>
> # Motivation
>
> Introduce `numWorkerThreadsForPersistentTopic` to deprecate
> `numWorkerThreadsForNonPersistentTopic`.
>
> The `numWorkerThreadsForNonPersistentTopic` is used to specify for
> PersistentTopic, not NonPersistentTopic. So I propose change the config
> item from `numWorkerThreadsForNonPersistentTopic` to
> `numWorkerThreadsForPersistentTopic`:
>
> Thanks,
> Xiaoyu Hou
>