Agree，+1
Regards
Jiwei Guo (Tboy)
On Thu, Feb 9, 2023 at 7:07 PM Jiuming Tao
<jm...@streamnative.io.invalid> wrote:
>
>
> It makes sense
>
> Thanks,
> Tao Jiuming
>
>
> > 2023年2月9日 15:44，Baodi Shi <ba...@apache.org> 写道：
> >
> > 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
> >>
> >>>>>
> >>
> >>>>>
> >>
> >>>>>
> >>
> >>>
> >>
> >>
> >>
>
