[fix][broker]The partitions of the topic was wrongly created even though this cluster was not allowed to access it#25443
Open
poorbarcode wants to merge 3 commits intoapache:masterfrom
Conversation
…ugh this cluster was not allowed to access it
Comment on lines
+2019
to
+2053
| public CompletableFuture<Boolean> isAllowedCurrentClusterAccess(@NonNull TopicName topicName) { | ||
| final String cluster = getPulsar().getConfig().getClusterName(); | ||
| return getCombinedTopicPolicies(topicName).thenApply(triple -> { | ||
| Optional<TopicPolicies> topicP = triple.getRight(); | ||
| Optional<TopicPolicies> globalTopicP = triple.getMiddle(); | ||
| Optional<Policies> nsPolicies = triple.getLeft(); | ||
| // Disabled a cluster for a namespace manually. | ||
| if (nsPolicies.isPresent() && !nsPolicies.get().allowed_clusters.isEmpty() | ||
| && !nsPolicies.get().allowed_clusters.contains(cluster)) { | ||
| return false; | ||
| } | ||
| // Manually enabled topic-level replication, which can skip to set a namespace-level replication. | ||
| if (topicP.isPresent() && CollectionUtils.isNotEmpty(topicP.get().getReplicationClusters())) { | ||
| if (topicP.get().getReplicationClusters().contains(cluster)) { | ||
| return true; | ||
| } else { | ||
| return false; | ||
| } | ||
| } | ||
| if (globalTopicP.isPresent() && CollectionUtils.isNotEmpty(globalTopicP.get().getReplicationClusters())) { | ||
| if (globalTopicP.get().getReplicationClusters().contains(cluster)) { | ||
| return true; | ||
| } else { | ||
| return false; | ||
| } | ||
| } | ||
| // No settings for replication/allowed_clusters. | ||
| if (nsPolicies.isEmpty()) { | ||
| return true; | ||
| } | ||
| // Namespace level settings. | ||
| return nsPolicies.get().replication_clusters.isEmpty() | ||
| || nsPolicies.get().replication_clusters.contains(cluster); | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
Don't we already have a mechanism for retrieving the applied policies for a topic? We should use it to avoid writing duplicate code.
Contributor
Author
There was a problem hiding this comment.
Don't we already have a mechanism for retrieving the applied policies for a topic? We should use it to avoid writing duplicate code.
No, we don't have such a method. There are currently two methods
- Persistent topic has a
HierarchyTopicPoliciesof merged policies, and this result is relied upon for inspection. - Check when the namespace modifies replication/allowed-cluster, see also
BrokerService.isCurrentClusterAllowed(Namespace)
The newly added method has maximised the reuse of code (by merging the namespace check method BrokerService.isCurrentClusterAllowed(Namespace).
BTW, the method newly added will become the method you mentioned: a mechanism for retrieving the applied policies for a topic
Contributor
Author
|
/pulsarbot rerun-failure-checks |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Env:
cluster-1andcluster-2share configuration metadata storepublic/default/tp1/admin/partitioned-topics/{topic}will be created on the shared configuration metadata store.cluster-1andcluster-2cluster-2from the topic-levelreplication policiespublic/default/tp1-partition-0will be deleted automatically.Issue: call
pulsar-admin topics create-missed-partitions <topic>, the partitionpublic/default/tp1-partition-0was loaded up again.Modifications
Fix the issue
Documentation
docdoc-requireddoc-not-neededdoc-completeMatching PR in forked repository
PR in forked repository: x