fix consumer receives duplicate messages when enable retry letter topic

[aloyszhang]

Fixes #9742

Motivation

This PR fix Consumer receive extra messages when enableRetry and retryLetterTopic and deadLetterTopic are not set.

Modifications

makes sure that generated retryLetterTopic and deadLetterTopic contain both subscriptionName and topicName

[eolivelli]

How do we deal with upgrades?

If I upgrade my application, one instance at a time, the new instances will use the new default and the application may behave in an inconsistent way

[aloyszhang]

@eolivelli
Actually, the default generated deadLetterTopic are not consistent from ConsumerImpl

  .deadLetterTopic(String.format("%s-%s" + RetryMessageUtil.DLQ_GROUP_TOPIC_SUFFIX, topic, subscription))

and ConsumerBuilderImpl

String retryLetterTopic = topicFirst.getNamespace() + "/" + conf.getSubscriptionName() + RetryMessageUtil.RETRY_GROUP_TOPIC_SUFFIX;

So, IMO, it's better to unite the format of default retryLetterTopic and deadLetterTopic and fix the potential risk for consumer to receive extra message from topics it does not subscribe although we have to adjustment out application.

[aloyszhang]

/pulsarbot run-failure-checks

[aloyszhang]

/pulsarbot run-failure-checks

[eolivelli]

I totally agree with you.
If we were writing this code the first time I would go that way.

My question is about how can I upgrade an application that relies on the default value?
Especially in the client people are used to simply upgrade the dependencies and so this is why we should not change default behaviour.

So I would like to understand how the migration may work and how to help users to not fall into unexpected problems.

[aloyszhang]

@eolivelli
After this PR, if user want to use deadLetter policy and not custome the deadLetterTopic, the default deadLetterTopic is tenant/namespace/{topicName}-{subscriptionName}-DLQ.

• topicName is the base partitioned topic name which can be got by TopicName.getPartitionedTopicName()

For example, when enalbleRetry for a consumer with subscriptionName subscription-test and topic public/default/topic-test, if deadLetterTopic are not set, we should build a deadLetterTopic consumer to subscribe from the deadLetterTopic public/default/topic-test-subscription-test-DLQ instead of public/default/subscription-test-DLQ

[codelipenghui]

Topics might not belong to a partitioned topic since we can use .topic("persistent://my-tenant/my-ns/topic-a", "persistent://my-tenant/my-ns/topic-b", "persistent://my-tenant/my-ns/topic-c"). This is why the default retry topic name does not contains the topic(the topic name is written to the properties of the message when send it to the retry topic).

You can explicitly specify the retry topic name or the dead letter topic name through the dead letter policy.

[aloyszhang]

Thanks for your explain. I got that why the original default retry letter topic does not contain the topic. There is no doubt that the best way to avoid this problem is specify the retry letter topic and dead letter topic.

In our prduction environment, we maintain a public pulsar cluster and many users access it. There are some users just set enableRetry to true without specify retry letter topic and dead letter topic which may lead to problem discribed by #9742.

IMO, it's better we can find a way to fix this problem. Maybe we can handle default retry and dead letter topic for each topic.

Topics might not belong to a partitioned topic since we can use .topic("persistent://my-tenant/my-ns/topic-a", "persistent://my-tenant/my-ns/topic-b", "persistent://my-tenant/my-ns/topic-c").

when consumer subscribe multi topics, pulsar can set the default retry letter topic and dead letter topic for every topic seperatlly, e.g.
for persistent://my-tenant/my-ns/topic-a will have
default retry letter topic persistent://my-tenant/my-ns/topic-a-subscription-name-RETRY
and dead letter topic persistent://my-tenant/my-ns/topic-a-subscription-name-DLQ;
for persistent://my-tenant/my-ns/topic-b will have
default retry letter topic persistent://my-tenant/my-ns/topic-b-subscription-name-RETRY
and dead letter topic persistent://my-tenant/my-ns/topic-b-subscription-name-DLQ

Without specify the dead letter topic, user should subscribe all default dead letter topic when receive message from DLQ.

[codelipenghui]

Make sense to me. And I think after this change will break the users that uses the try letter topic due to the different topic name. So this should discuss it in the dev email channel and highlight this will be a breaking change, so that we can highlight it at the release note.

[aloyszhang]

closed by #10129