Can we add another phase to this PIP - say on version x we add WARN and
override mode to non-durable, and on version x+1 we fail the client?

On Tue, Feb 14, 2023 at 9:48 AM Jiuming Tao <jm...@streamnative.io.invalid>
wrote:

> Hi Asaf,
>
> If we fail the client when users use Durable subscription to subscribe
> NonPersistentTopic, it will lead to break change.
>
> Users have to change their code, so we can change the `subscriptionMode`
> from `Durable` to `NonDurable` and print WARN log when users choose
> `Durable`, then, users don’t need to change their code and we will not
> introduce break change.
>
> The PIP will deprecate Durable subscriptions on NonPersistentTopic, all
> the subscriptions on NonPersistentTopic will be NonDurable.
>
>
> Thanks,
> Tao Jiuming
>
>
>
>
> > 2023年2月14日 03:24,Asaf Mesika <asaf.mes...@gmail.com> 写道:
> >
> > 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
> >>>>>>
> >>>>>>>>>
> >>>>>>
> >>>>>>>>>
> >>>>>>
> >>>>>>>>>
> >>>>>>
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>
> >>>
> >>
>
>

Reply via email to