-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[improve][broker] Tidy up the system topic. #15252
Conversation
@Technoboy- Why not add a method to check if a system topic is a transaction related topic? |
We have |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
pulsar-broker/src/main/java/org/apache/pulsar/broker/namespace/NamespaceService.java
Show resolved
Hide resolved
@@ -123,7 +125,7 @@ protected void setUpBase(int numBroker,int numPartitionsOfTC, String topic, int | |||
admin.tenants().createTenant(NamespaceName.SYSTEM_NAMESPACE.getTenant(), | |||
new TenantInfoImpl(Sets.newHashSet("appid1"), Sets.newHashSet(CLUSTER_NAME))); | |||
admin.namespaces().createNamespace(NamespaceName.SYSTEM_NAMESPACE.toString()); | |||
admin.topics().createPartitionedTopic(TopicName.TRANSACTION_COORDINATOR_ASSIGN.toString(), numPartitionsOfTC); | |||
createTransactionCoordinatorAssign(numPartitionsOfTC); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a question why don't use admin.topics().createPartitionedTopic(TopicName.TRANSACTION_COORDINATOR_ASSIGN.toString(), numPartitionsOfTC);
to create topic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, because TRANSACTION_COORDINATOR_ASSIGN is not a topic in fact. If we use the old way to create it like createPartitionedTopic
, we have to check the name on the broker side to let it pass(like not to create managed ledger or something).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your reply.
pulsar-broker/src/test/java/org/apache/pulsar/broker/service/BrokerTestBase.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
### Motivation Currently, topics/bundles in `pulsar/system` will be filter while doing shedding, which is introduced by mistake by pr #15252. But we need to unload topics/bundles in `pulsar/system` for load balancing. ### Modifications do not filter topics/bundles in `pulsar/system`.
### Motivation Currently, topics/bundles in `pulsar/system` will be filter while doing shedding, which is introduced by mistake by pr #15252. But we need to unload topics/bundles in `pulsar/system` for load balancing. ### Modifications do not filter topics/bundles in `pulsar/system`.
### Motivation Currently, topics/bundles in `pulsar/system` will be filter while doing shedding, which is introduced by mistake by pr #15252. But we need to unload topics/bundles in `pulsar/system` for load balancing. ### Modifications do not filter topics/bundles in `pulsar/system`. (cherry picked from commit 5c74d20)
### Motivation Currently, topics/bundles in `pulsar/system` will be filter while doing shedding, which is introduced by mistake by pr #15252. But we need to unload topics/bundles in `pulsar/system` for load balancing. ### Modifications do not filter topics/bundles in `pulsar/system`.
### Motivation Currently, topics/bundles in `pulsar/system` will be filter while doing shedding, which is introduced by mistake by pr apache#15252. But we need to unload topics/bundles in `pulsar/system` for load balancing. ### Modifications do not filter topics/bundles in `pulsar/system`.
Motivation
For #13520, #14643, #14949, we fix some issues related to system topic but result in checking the system topic in different method. So it's better to tidy up the system topic.
So put these system topic names into a new class called SystemTopicNames.
Documentation
no-need-doc