[GitHub] merlimat commented on a change in pull request #3553: [pulsar-broker][replicator] add remote cluster name in producer name

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

merlimat commented on a change in pull request #3553: [pulsar-broker][replicator] add remote cluster name in producer name
URL: https://github.com/apache/pulsar/pull/3553#discussion_r255241165
 
 

 ##########
 File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractReplicator.java
 ##########
 @@ -46,6 +46,7 @@
     protected final PulsarClientImpl client;
 
     protected volatile ProducerImpl producer;
+    public static final String producerNamePadding = "####@@@@!!!!padding";
 
 Review comment:
   Not really a big fan of having these random chars in the log :) 
   
   Given that the current parsing logic is based on splitting at the `.`, I see 3 options here: 
   
    1. Currently we use `pulsar.repl.SOURCE` . Change into something like `pulsar.repl.SOURCE.-->.TARGET` so that the padding is used to indicate the data direction.
    2. Fix the parsing logic to expect a better format and do a 2 steps rollout 
    3. Just change the format and live with stats inconsistencies during the upgrade 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services