Clarifying: as Pengu wrote in the pip comment - if we change the default
subscription mode to be non-durable for non-persistent topics, then what
will happen to users who previously had durable subscriptions on that
topic? Are we ruining something if we create non-durable subscriptions
instead?
On Mon, Feb 13, 2023 at 9:16 PM Asaf Mesika <asaf.mes...@gmail.com> wrote:
> Can we fail the client if they choose NonDurable subscription mode on a
> non-persistent topic, instead of writing WARN log messages?
>
>
> On Mon, Feb 13, 2023 at 4:26 AM guo jiwei <techno...@apache.org> wrote:
>
>> 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
>> > >>
>> > >>>>>
>> > >>
>> > >>>>>
>> > >>
>> > >>>>>
>> > >>
>> > >>>
>> > >>
>> > >>
>> > >>
>> >
>>
>
