[GitHub] [pulsar] aloyszhang commented on a change in pull request #9743: fix consumer receives duplicate messages when enable retry letter topic

[GitBox <gi...@apache.org>]

aloyszhang commented on a change in pull request #9743:
URL: https://github.com/apache/pulsar/pull/9743#discussion_r585188393



##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerBuilderImpl.java
##########
@@ -119,10 +119,12 @@ public ConsumerBuilderImpl(PulsarClientImpl client, Schema<T> schema) {
             return FutureUtil.failedFuture(
                     new InvalidConfigurationException("KeySharedPolicy must set with KeyShared subscription"));
         }
-        if(conf.isRetryEnable() && conf.getTopicNames().size() > 0 ) {
-            TopicName topicFirst = TopicName.get(conf.getTopicNames().iterator().next());
-            String retryLetterTopic = topicFirst.getNamespace() + "/" + conf.getSubscriptionName() + RetryMessageUtil.RETRY_GROUP_TOPIC_SUFFIX;
-            String deadLetterTopic = topicFirst.getNamespace() + "/" + conf.getSubscriptionName() + RetryMessageUtil.DLQ_GROUP_TOPIC_SUFFIX;
+        if (conf.isRetryEnable() && conf.getTopicNames().size() > 0 ) {
+            String topicName = TopicName.get(conf.getTopicNames().iterator().next()).getPartitionedTopicName();
+            String retryLetterTopic = topicName + "-" + conf.getSubscriptionName()
+                    + RetryMessageUtil.RETRY_GROUP_TOPIC_SUFFIX;
+            String deadLetterTopic = topicName + "-" + conf.getSubscriptionName()
+                    + RetryMessageUtil.DLQ_GROUP_TOPIC_SUFFIX;

Review comment:
       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 out 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, i.e. 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`
   
   
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org