[GitHub] [pulsar] congbobo184 commented on a change in pull request #10254: [Broker] Remove possible error case for system topic name checking

[GitBox]

congbobo184 commented on a change in pull request #10254:
URL: https://github.com/apache/pulsar/pull/10254#discussion_r615871300



##########
File path: 
pulsar-broker/src/main/java/org/apache/pulsar/broker/systopic/SystemTopicClient.java
##########
@@ -167,8 +167,11 @@
         SystemTopicClient<T> getSystemTopic();
     }
 
+    /**
+     * A topic is a system topic if and only if its local name has the defined 
prefix.
+     */
     static boolean isSystemTopic(TopicName topicName) {
-        return 
EventsTopicNames.NAMESPACE_EVENTS_LOCAL_NAME.equals(topicName.getLocalName());
+        return 
topicName.getLocalName().startsWith(EventsTopicNames.SYSTEM_TOPIC_LOCAL_NAME_PREFIX);

Review comment:
       i don't think `__` would be considered system topics is a good idea. It 
will cause problem when broker upgrade .  




-- 
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:
us...@infra.apache.org