lhotari commented on code in PR #22139:
URL: https://github.com/apache/pulsar/pull/22139#discussion_r1505444301
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentSubscription.java:
##########
@@ -157,7 +158,8 @@ public PersistentSubscription(PersistentTopic topic, String
subscriptionName, Ma
this.subscriptionProperties = MapUtils.isEmpty(subscriptionProperties)
? Collections.emptyMap() :
Collections.unmodifiableMap(subscriptionProperties);
if
(topic.getBrokerService().getPulsar().getConfig().isTransactionCoordinatorEnabled()
- && !isEventSystemTopic(TopicName.get(topicName))) {
+ && !isEventSystemTopic(TopicName.get(topicName))
+ && !ExtensibleLoadManagerImpl.isInternalTopic(topicName)) {
Review Comment:
It would be better to have this type of logic centralized. It's not a good
sign when more classes get coupled with ExtensibleLoadManagerImpl.
There have been discussions of coming up with a convention for Pulsar for
system topics, but no decisions came out of that (which was disappointing).
Trying to search for the discussion. Came across
https://lists.apache.org/thread/1lndgf1hx821fc12t0pc6j6zdrhkntht and
https://github.com/apache/pulsar/pull/20397 . Not that one.
It's this discussion:
https://lists.apache.org/thread/oz79m0f2nw059jctq4cmms74yq5n2l1m
That's just useful background knowledge to have. In this case, I'd look for
a practical solution. However, there is a need to cleanup messes when working
on small changes like this one.
In the Topic interface we already have `isSystemTopic()`:
https://github.com/apache/pulsar/blob/59782b3c1f63baa68b2f5f33a33f1b00212159d7/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/Topic.java#L322
This is used in many locations, but there's also other ways to detect system
topics.
We shouldn't be adding yet another way for ExtensibleLoadManagerImpl topics,
but instead find a way to cleanup the mess. I hope this makes sense.
--
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.
To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
