Thanks for your review Michael. On Tue, Dec 21, 2021 at 5:48 AM Michael Marshall <mmarsh...@apache.org> wrote: > > Thanks for creating this PIP Zike Yang. This sounds like an important > improvement. I have a couple thoughts. > > 1. Does the `retryLetterTopic` have the same problem with immediate > message expiration? Based on a quick glance, I don't see anything > creating a subscription for the retry topic. > Yes. I think we also need to handle the retryLetterTopic. I will update the PIP.
> 2. Your prototype exposes a lacking in our existing client > implementation: the non-admin client cannot directly create > subscriptions. IIUC, this implementation does not work if > `allowAutoSubscriptionCreation` is false. IMO, we won't create the init subscription if `allowAutoSubscriptionCreation` is false. Otherwise, it will confuse the user. Users can explicitly create that subscription to handle this case. > Further, it is inefficient > to create and immediately close a consumer just to create a > subscription. Perhaps we should just update the non-admin client so > that it can create subscriptions? It could be private so that we don't > create confusion with the non-admin and the admin clients. Do you mean to create a private method called `createSubscription` in the non-admin client? But we don't have a protocol command for creating subscription. So in that method, we can send the `CommandSubscribe` and `CommandUnsubscribe` to the broker to simulate the creation and closure of a consumer. But I think it is not straightforward to create a subscription using two commands. Or the other way is to create a new command `CommandSubscription` in the protocol to handle this. But I wonder if there are more use cases for this command that would make it worthwhile to use this solution. What do you think about these two ways? > > Thanks, > Michael > > > On Mon, Dec 20, 2021 at 4:18 AM Zike Yang > <zky...@streamnative.io.invalid> wrote: > > > > https://github.com/apache/pulsar/issues/13408 > > > > Pasted below for quoting convenience. > > > > —— > > > > > > ## Motivation > > > > If we enable the DLQ when consuming messages. For some messages that can't > > be processed successfully, the messages will be moved to the DLQ, but if we > > do not specify the data retention for the namespace or create a > > subscription for the DLQ to retain the data, the data of the DLQ topic will > > be removed automatically. Therefore, we need to create the initial > > subscription before sending messages to the DLQ. > > > > ## Goal > > > > Users can set the initial subscription name in the DeadLetterPolicy. The > > consumer will create the initial subscription before sending messages to > > the DLQ. At this point, subsequent messages produced to the DLQ are not > > automatically deleted unexpectedly. > > > > This PIP needs to be compatible with the previous behavior. The initial > > subscription name in the DeadLetterPolicy is optional. Default value is the > > subscription name of the consumer. > > > > ## API Changes > > > > Add `initSubscriptionName` to the `DeadLetterPolicy` > > > > ```java > > /** > > * Name of the initial subscription name of the dead letter topic. > > * The default value is the subscription name of the consumer. > > */ > > private String initSubscriptionName; > > ``` > > > > ## Implementation > > > > Before the deadLetterProducer is initialized, the consumer first tries to > > create a deadLetterConsumer using the initial subscription name in the > > DeadLetterPolicy. When this subscription already exists, the ConsumerBusy > > exception will occur. In this case, we can ignore that exception and create > > the deadLetterProducer. > > > > Prototype implementation PR: https://github.com/apache/pulsar/pull/13355 > > > > > > > > Thanks, > > Zike Yang > > Best regards, Zike Yang
