-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
[fix][broker][WIP] Don't check replication clusters for ownership of system topics #22597
base: master
Are you sure you want to change the base?
[fix][broker][WIP] Don't check replication clusters for ownership of system topics #22597
Conversation
1b91da8
to
648b5f0
Compare
@poorbarcode @heesung-sn Do you think that relaxing the checkLocalOrGetPeerReplicationCluster checks for system topics would be useful? |
I realized that Pulsar has some loopholes that unnecessarily try to replicate, migrate or do other operations on system topics. We need to check all system topics and define their behaviors for each feature. System namespace and system topics
I am checking the system topics from the new load balancer. |
To answer your questions, Im not sure. I assume all system topics should live in local cluster only,but I could be wrong. |
Same exception with current master branch version 93afd89
|
648b5f0
to
23d8b53
Compare
It seems that __change_events has special handling in replication: pulsar/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/SystemTopic.java Lines 69 to 74 in 93afd89
pulsar/pulsar-common/src/main/java/org/apache/pulsar/common/naming/SystemTopicNames.java Lines 80 to 85 in 93afd89
Therefore I'm not sure if the solution in this PR is correct or not. |
WIP, not ready for review. PR opened in draft mode for initial discussion and feedback.
Motivation
When investigating another issue and reproducing the problem using these instructions, #21948 (comment), I noticed this WARN log message:
Modifications
checkLocalOrGetPeerReplicationCluster
checks.Additional Context
This is related to #20304 changes since that made some relaxation on the checks.
Documentation
doc
doc-required
doc-not-needed
doc-complete