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 > >>>>>> > >>>>>>>>> > >>>>>> > >>>>>>>>> > >>>>>> > >>>>>>>>> > >>>>>> > >>>>>>> > >>>>>> > >>>>>> > >>>>>> > >>>> > >>> > >> > >