> How about set `isDurable = false` when create `NonPersistentSubscription`?
We could directly delete the `isDurable` in this constructor. There
will be no durable subscriptions on the non-persistent topic anymore.
> When the topic is non-persistent and uses durable to subscribe, print warns
> logs: “non-persistent not support durable subscribe will use non-durable to
> subscribe.”
Printing warn log makes sense to me.
Thanks,
Zike Yang
On Thu, Feb 9, 2023 at 3:44 PM Baodi Shi <ba...@apache.org> wrote:
>
> Agree, and we can add verification on the client side. When the topic is
> non-persistent and uses durable to subscribe, print warns logs:
> “non-persistent not support durable subscribe will use non-durable to
> subscribe.”
>
>
>
> Thanks,
> Baodi Shi
>
> 在 2023年2月9日 15:10:12 上，Jiuming Tao <jm...@streamnative.io.invalid> 写道：
>
> > Agree. But we need to take care of the compatibility. How do we handle
> >
> > this compatibility?
> >
> >
> > How about set `isDurable = false` when create `NonPersistentSubscription`?
> >
> > In `NonPersistentSubscription`, we can change the constructor from
> > ```
> > public NonPersistentSubscription(NonPersistentTopic topic, String
> > subscriptionName, boolean isDurable,
> > Map<String, String> properties) {
> > this.topic = topic;
> > this.topicName = topic.getName();
> > this.subName = subscriptionName;
> > this.fullName = MoreObjects.toStringHelper(this).add("topic",
> > topicName).add("name", subName).toString();
> > IS_FENCED_UPDATER.set(this, FALSE);
> > this.lastActive = System.currentTimeMillis();
> > this.isDurable = isDurable;
> > this.subscriptionProperties = properties != null
> > ? Collections.unmodifiableMap(properties) :
> > Collections.emptyMap();
> > }
> >
> > ```
> >
> > to:
> >
> > ```
> > public NonPersistentSubscription(NonPersistentTopic topic, String
> > subscriptionName,
> > Map<String, String> properties) {
> > this.topic = topic;
> > this.topicName = topic.getName();
> > this.subName = subscriptionName;
> > this.fullName = MoreObjects.toStringHelper(this).add("topic",
> > topicName).add("name", subName).toString();
> > IS_FENCED_UPDATER.set(this, FALSE);
> > this.lastActive = System.currentTimeMillis();
> > this.isDurable = false;
> > this.subscriptionProperties = properties != null
> > ? Collections.unmodifiableMap(properties) :
> > Collections.emptyMap();
> > }
> >
> > ```
> >
> > In the broker side, for non-persistent topic, there is no durable
> > subscriptions.
> > In the client side, users don’t need to change their code.
> >
> > Thanks,
> > Tao Jiuming
> >
> > 2023年2月9日 15:01，Zike Yang <z...@apache.org> 写道：
> >
> >
> > > It makes sense, for non-persistent topic, it should not create a durable
> > subscription.
> >
> >
> > Agree. But we need to take care of the compatibility. How do we handle
> >
> > this compatibility?
> >
> >
> > BR,
> >
> > Zike Yang
> >
> >
> > On Thu, Feb 9, 2023 at 2:43 PM Jiuming Tao
> >
> > <jm...@streamnative.io.invalid> wrote:
> >
> > >
> >
> > >> We can add validation to inform the user that a durable subscription on
> > a
> >
> > >> non-persistent topic is invalid.
> >
> > >
> >
> > > It makes sense, for non-persistent topic, it should not create a durable
> > subscription.
> >
> > >
> >
> > > Thanks,
> >
> > > Tao Jiuming
> >
> > >
> >
> > >> 2023年2月8日 23:06，Baodi Shi <ba...@apache.org> 写道：
> >
> > >>
> >
> > >>>
> >
> > >>> But it seems that currently all Durable subscriptions on
> >
> > >>>
> >
> > >> non-persistent topics are NonDurable, and I wonder if that confuses
> >
> > >>> users.
> >
> > >>>
> >
> > >>
> >
> > >> We can add validation to inform the user that a durable subscription on
> > a
> >
> > >> non-persistent topic is invalid.
> >
> > >>
> >
> > >>
> >
> > >> Thanks,
> >
> > >> Baodi Shi
> >
> > >>
> >
> > >>
> >
> > >> 在 2023年2月8日 10:48:32 上，Zike Yang <z...@apache.org> 写道：
> >
> > >>
> >
> > >>> I agree to remove all subscriptions when there're no consumers on the
> >
> > >>> non-persistent topic. The configuration
> >
> > >>> `subscriptionExpirationTimeMinutes ` should not take effect on the
> >
> > >>> non-persistent topic.
> >
> > >>>
> >
> > >>> But it seems that currently all Durable subscriptions on
> >
> > >>> non-persistent topics are NonDurable, and I wonder if that confuses
> >
> > >>> users.
> >
> > >>>
> >
> > >>>> But for `NonPersistentDispatcherMultipleConsumers`,
> >
> > >>> `NonPersistentStickyKeyDispatcherMultipleConsumers`, I’m not sure that
> >
> > >>> if it matters when we auto delete the subscriptions which has no
> >
> > >>> consumers connected.
> >
> > >>>
> >
> > >>> It doesn't matter IMO.
> >
> > >>>
> >
> > >>> Thanks,
> >
> > >>> Zike Yang
> >
> > >>>
> >
> > >>>
> >
> > >>>
> >
> > >>> Zike Yang
> >
> > >>>
> >
> > >>> On Tue, Feb 7, 2023 at 9:22 PM Jiuming Tao
> >
> > >>> <jm...@streamnative.io.invalid> wrote:
> >
> > >>>
> >
> > >>>
> >
> > >>>> I think we are talking about the durable subscription of the
> >
> > >>> non-persistent
> >
> > >>>
> >
> > >>>> topic, right?
> >
> > >>>
> >
> > >>>
> >
> > >>> Yes, non-durable subscriptions created by Reader, if the reader
> >
> > >>> disconnected, the subscription will be deleted automatically.
> >
> > >>>
> >
> > >>>
> >
> > >>>
> >
> > >>> For `NonPersistentDispatcher`, it has the following implementations:
> >
> > >>>
> >
> > >>> 1. NonPersistentDispatcherMultipleConsumers
> >
> > >>>
> >
> > >>> 2. NonPersistentDispatcherSingleActiveConsumer
> >
> > >>>
> >
> > >>> 3. NonPersistentStickyKeyDispatcherMultipleConsumers
> >
> > >>>
> >
> > >>>
> >
> > >>> For `NonPersistentDispatcherSingleActiveConsumer`, auto delete the
> >
> > >>> subscription when there isn’t consumer connected affect nothing.
> >
> > >>>
> >
> > >>>
> >
> > >>> But for `NonPersistentDispatcherMultipleConsumers`,
> >
> > >>> `NonPersistentStickyKeyDispatcherMultipleConsumers`, I’m not sure that
> > if
> >
> > >>> it matters when we auto delete the subscriptions which has no
> > consumers
> >
> > >>> connected.
> >
> > >>>
> >
> > >>>
> >
> > >>> If it doesn’t matter, I agree with you.
> >
> > >>>
> >
> > >>>
> >
> > >>> Auto delete the durable subscriptions when there aren’t consumers
> >
> > >>> connected to the non-persistent topic.
> >
> > >>>
> >
> > >>>
> >
> > >>>
> >
> > >>> Thanks,
> >
> > >>>
> >
> > >>> Tao Jiuming
> >
> > >>>
> >
> > >>>
> >
> > >>>
> >
> > >>>> 2023年2月7日 19:58，PengHui Li <peng...@apache.org <mailto:
> >
> > >>> peng...@apache.org>> 写道：
> >
> > >>>
> >
> > >>>>
> >
> > >>>
> >
> > >>>> I think we are talking about the durable subscription of the
> >
> > >>> non-persistent
> >
> > >>>
> >
> > >>>> topic, right? For the Reader API (non-durable subscription), the
> >
> > >>>
> >
> > >>>> subscription will be
> >
> > >>>
> >
> > >>>> removed after the reader disconnects. If not, it should be BUG.
> >
> > >>>
> >
> > >>>>
> >
> > >>>
> >
> > >>>> IMO, we should remove the subscription automatically if all the
> > consumers
> >
> > >>>
> >
> > >>>> are disconnected for the non-persistent topic. The consumers can only
> > get
> >
> > >>>
> >
> > >>>> the new incoming messages after they connect to the non-persistent
> > topic.
> >
> > >>>
> >
> > >>>> It looks like even if we leave the subscription there; it will
> >
> > >>>
> >
> > >>>> help nothing.
> >
> > >>>
> >
> > >>>> The behavior should be the same as creating a new subscription with
> > the
> >
> > >>>
> >
> > >>>> same subscription name.
> >
> > >>>
> >
> > >>>>
> >
> > >>>
> >
> > >>>> For subscription management, users are not able to disable the
> >
> > >>> subscription
> >
> > >>>
> >
> > >>>> auto-creation of the non-persistent topic? We don't have any metadata
> >
> > >>>
> >
> > >>>> persist
> >
> > >>>
> >
> > >>>> in the metadata store. After the broker restarts, we will lose
> >
> > >>> everything.
> >
> > >>>
> >
> > >>>>
> >
> > >>>
> >
> > >>>> So I think we don't need to introduce a new configuration. Instead, we
> >
> > >>>
> >
> > >>>> should
> >
> > >>>
> >
> > >>>> figure out if there is a strong reason to leave the durable
> > subscription
> >
> > >>>
> >
> > >>>> after
> >
> > >>>
> >
> > >>>> all the consumers are disconnected. If there is no strong reason, I
> > tend
> >
> > >>> to
> >
> > >>>
> >
> > >>>> fix
> >
> > >>>
> >
> > >>>> incorrect behavior directly.
> >
> > >>>
> >
> > >>>>
> >
> > >>>
> >
> > >>>> Best,
> >
> > >>>
> >
> > >>>> Penghui
> >
> > >>>
> >
> > >>>>
> >
> > >>>
> >
> > >>>>
> >
> > >>>
> >
> > >>>>
> >
> > >>>
> >
> > >>>>
> >
> > >>>
> >
> > >>>> On Tue, Feb 7, 2023 at 4:14 PM Jiuming Tao
> > <jm...@streamnative.io.invalid
> >
> > >>> <mailto:jm...@streamnative.io.invalid>>
> >
> > >>>
> >
> > >>>> wrote:
> >
> > >>>
> >
> > >>>>
> >
> > >>>
> >
> > >>>>> Hi,
> >
> > >>>
> >
> > >>>>>
> >
> > >>>
> >
> > >>>>> I’ve opened a new PIP to discuss:
> >
> > >>>
> >
> > >>>>>
> >
> > >>>
> >
> > >>>>> Currently, in Pulsar, we have a configuration named
> >
> > >>>
> >
> > >>>>> `subscriptionExpirationTimeMinutes` to manage the subscription
> >
> > >>> expiration
> >
> > >>>
> >
> > >>>>> of `PersistentTopic` and `NonPersistentTopic`.
> >
> > >>>
> >
> > >>>>>
> >
> > >>>
> >
> > >>>>> When we set a value which is greater than 0 to
> >
> > >>>
> >
> > >>>>> `subscriptionExpirationTimeMinutes`, it will affect both
> >
> > >>> `PersistentTopic`
> >
> > >>>
> >
> > >>>>> and `NonPersistentTopic`. Their inactive subscriptions will get
> > expired
> >
> > >>> and
> >
> > >>>
> >
> > >>>>> will clean automatically.
> >
> > >>>
> >
> > >>>>>
> >
> > >>>
> >
> > >>>>> For `NonPersistentTopic`, its subscriptions can be clean because we
> >
> > >>> don't
> >
> > >>>
> >
> > >>>>> guarantee its data integrity. But for `PersistentTopic`, if we clean
> > its
> >
> > >>>
> >
> > >>>>> subscriptions automatically may lead to data loss.
> >
> > >>>
> >
> > >>>>>
> >
> > >>>
> >
> > >>>>> However, their subscription expiration is managed by the same
> >
> > >>>
> >
> > >>>>> configuration(`subscriptionExpirationTimeMinutes`), we can't manage
> >
> > >>> their
> >
> > >>>
> >
> > >>>>> subscription expiration independently.
> >
> > >>>
> >
> > >>>>>
> >
> > >>>
> >
> > >>>>> So I want to introduce a new configuration named
> >
> > >>>
> >
> > >>>>> `nonPersistentSubscriptionExpirationTimeMinutes` to manage
> >
> > >>>
> >
> > >>>>> `NonPersistentTopic`'s subscription expiration.
> >
> > >>>
> >
> > >>>>>
> >
> > >>>
> >
> > >>>>>
> >
> > >>>
> >
> > >>>>> Please feel free to leave your comments, the PIP link:
> >
> > >>>
> >
> > >>>>> https://github.com/apache/pulsar/issues/19448 <
> >
> > >>> https://github.com/apache/pulsar/issues/19448> <
> >
> > >>>
> >
> > >>>>> https://github.com/apache/pulsar/issues/19448 <
> >
> > >>> https://github.com/apache/pulsar/issues/19448>>
> >
> > >>>
> >
> > >>>>>
> >
> > >>>
> >
> > >>>>>
> >
> > >>>
> >
> > >>>>> Thanks,
> >
> > >>>
> >
> > >>>>> Tao Jiuming
> >
> > >>>
> >
> > >>>
> >
> > >>>
> >
> > >
> >
> >
> >
