Skip to content
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][admin] Remove duplicate topics name when deleteNamespace #20683

Merged
merged 4 commits into from
Jul 4, 2023

Conversation

Technoboy-
Copy link
Contributor

Motivation

When deleteNamespace, we will get all the topics, see:

private void internalRetryableDeleteNamespaceAsync0(boolean force, int retryTimes,
@Nonnull CompletableFuture<Void> callback) {
precheckWhenDeleteNamespace(namespaceName, force)
.thenCompose(policies -> {
final CompletableFuture<List<String>> topicsFuture;
if (policies == null || CollectionUtils.isEmpty(policies.replication_clusters)){
topicsFuture = pulsar().getNamespaceService().getListOfPersistentTopics(namespaceName);
} else {
topicsFuture = pulsar().getNamespaceService().getFullListOfTopics(namespaceName);
}
return topicsFuture.thenCompose(allTopics ->
pulsar().getNamespaceService().getFullListOfPartitionedTopic(namespaceName)
.thenCompose(allPartitionedTopics -> {
List<List<String>> topicsSum = new ArrayList<>(2);
topicsSum.add(allTopics);
topicsSum.add(allPartitionedTopics);
return CompletableFuture.completedFuture(topicsSum);
}))
.thenCompose(topics -> {

if the topic is partitioned-topic, eg : "persistent://public/default/__change_events" with 3 partitions,
then the topicsSum will contain:

persistent://public/default/__change_events-partition-0
persistent://public/default/__change_events-partition-1
persistent://public/default/__change_events-partition-2
persistent://public/default/__change_events

So cause topicPolicy with :

persistent://public/default/__change_events-partition-0
persistent://public/default/__change_events-partition-1
persistent://public/default/__change_events-partition-2

partitionedTopicPolicy with :

persistent://public/default/__change_events

return markDeleteFuture.thenCompose(__ ->
internalDeleteTopicsAsync(allUserCreatedTopics))
.thenCompose(ignore ->
internalDeletePartitionedTopicsAsync(allUserCreatedPartitionTopics))
.thenCompose(ignore ->
internalDeleteTopicsAsync(allSystemTopics))
.thenCompose(ignore ->
internalDeletePartitionedTopicsAsync(allPartitionedSystemTopics))
.thenCompose(ignore ->
internalDeleteTopicsAsync(topicPolicy))
.thenCompose(ignore ->
internalDeletePartitionedTopicsAsync(partitionedTopicPolicy));

So make the duplicate deleting.

image

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jun 29, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #20683 (a33e076) into master (2b01f83) will increase coverage by 39.56%.
The diff coverage is 68.62%.

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #20683       +/-   ##
=============================================
+ Coverage     33.58%   73.15%   +39.56%     
- Complexity    12127    32053    +19926     
=============================================
  Files          1613     1867      +254     
  Lines        126241   138764    +12523     
  Branches      13770    15263     +1493     
=============================================
+ Hits          42396   101508    +59112     
+ Misses        78331    29227    -49104     
- Partials       5514     8029     +2515     
Flag Coverage Δ
inttests 24.23% <16.66%> (+0.04%) ⬆️
systests 25.09% <15.68%> (?)
unittests 72.41% <68.62%> (+40.37%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...in/java/org/apache/pulsar/admin/cli/CmdTopics.java 78.94% <52.83%> (+78.94%) ⬆️
...ava/org/apache/pulsar/admin/cli/CmdNamespaces.java 76.25% <60.00%> (+76.25%) ⬆️
.../java/org/apache/pulsar/client/cli/CmdProduce.java 48.63% <63.63%> (+21.96%) ⬆️
...he/pulsar/client/impl/MultiTopicsConsumerImpl.java 78.16% <92.85%> (+45.79%) ⬆️
...pache/pulsar/broker/admin/impl/NamespacesBase.java 72.16% <100.00%> (+47.19%) ⬆️
...roker/service/persistent/MessageDeduplication.java 82.53% <100.00%> (+42.35%) ⬆️
.../pulsar/broker/stats/BrokerOperabilityMetrics.java 100.00% <100.00%> (ø)
...org/apache/pulsar/broker/stats/DimensionStats.java 87.27% <100.00%> (+1.81%) ⬆️
...metadata/bookkeeper/PulsarRegistrationManager.java 62.43% <100.00%> (+53.03%) ⬆️

... and 1514 files with indirect coverage changes

@Technoboy- Technoboy- merged commit a3bca68 into apache:master Jul 4, 2023
@Technoboy-
Copy link
Contributor Author

2.10 has been fixed by #20685

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants