michaeljmarshall commented on code in PR #20566:
URL: https://github.com/apache/pulsar/pull/20566#discussion_r1235960291
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java:
##########
@@ -3304,10 +3305,19 @@ private CompletableFuture<Boolean>
isAllowAutoTopicCreationAsync(final TopicName
topicName.getNamespaceObject());
return CompletableFuture.completedFuture(false);
}
- //System topic can always be created automatically
+
+ // ServiceUnitStateChannelImpl.TOPIC expects to be a
non-partitioned-topic now.
+ // We don't allow the auto-creation here.
+ // ServiceUnitStateChannelImpl.start() is responsible to create the
topic.
+ if (ServiceUnitStateChannelImpl.TOPIC.equals(topicName.toString())) {
Review Comment:
Sorry, I was mixing up https://github.com/apache/pulsar/pull/20370 and
https://github.com/apache/pulsar/pull/20397.
My primary concern is that the solution in this PR creates a special case
instead of creating a framework to make it easier to build system topics in the
future. In my opinion, the load manager should be contained by the relevant
interfaces and should not be referenced here. There is some relevant discussion
about system topics in this PR too
https://github.com/apache/pulsar/pull/20514#issuecomment-1579937937. There is
also relevant discussion on the ML here
https://lists.apache.org/thread/f0q8n0hf1lgw9r2j53tm4yjjfdyr9kjd.
I think it is relevant that until now, we have always allowed system topics
to be auto created to prevent classes of failures.
If I were to guess, the problem this PR is trying to solve is actually more
generic than the system topics we're working with here. It is likely a
challenge for all pulsar users to know how to create their topics correctly.
That is one reason I want to push for thinking about a general solution.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
