You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/08/29 15:14:08 UTC
[GitHub] [pulsar] poorbarcode opened a new pull request, #17338: [fix][broker]delete namespace fail when has partitioned system topic
poorbarcode opened a new pull request, #17338:
URL: https://github.com/apache/pulsar/pull/17338
### Motivation
When a namespace containing partitioned topics is deleted, will throw the topic not found exception.
<strong>(High light)</strong> Should use API `deletePartitionedTopicAsync` to delete partitioned system topic.
### Modifications
- Use `cmd-deletePartitionedTopicAsync` to delete partitioned system topic.
- Add tests: delete namespace.
### Documentation
- [ ] `doc-required`
- [x] `doc-not-needed`
- [ ] `doc`
- [ ] `doc-complete`
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#issuecomment-1240184904
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] dlg99 commented on pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
dlg99 commented on PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#issuecomment-1231860351
I think CI failure is fixed by https://github.com/apache/pulsar/pull/17339
I'll rebase this PR and let Ci rerun
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#issuecomment-1233411683
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on code in PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#discussion_r960331150
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -264,13 +265,19 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth
}
boolean isEmpty;
- List<String> topics;
+ Set<String> partitionedTopicNames = new HashSet<>();
+ Set<String> topicNames = new HashSet<>();
try {
- topics = pulsar().getNamespaceService().getListOfPersistentTopics(namespaceName)
- .get(config().getMetadataStoreOperationTimeoutSeconds(), TimeUnit.SECONDS);
- topics.addAll(getPartitionedTopicList(TopicDomain.persistent));
- topics.addAll(getPartitionedTopicList(TopicDomain.non_persistent));
- isEmpty = topics.isEmpty();
+ if (policies == null || CollectionUtils.isEmpty(policies.replication_clusters)){
+ partitionedTopicNames.addAll(pulsar().getNamespaceService().getListOfPersistentTopics(namespaceName)
+ .get(config().getMetadataStoreOperationTimeoutSeconds(), TimeUnit.SECONDS));
+ } else {
+ partitionedTopicNames.addAll(pulsar().getNamespaceService().getFullListOfTopics(namespaceName)
+ .get(config().getMetadataStoreOperationTimeoutSeconds(), TimeUnit.SECONDS));
+ }
+ topicNames.addAll(getPartitionedTopicList(TopicDomain.persistent));
Review Comment:
Yes.
`getFullListOfTopics()` includes the persistent and non-persistent topics, and `getPartitionedTopicList()` check the partitioned-topic by metadata., see also https://github.com/apache/pulsar/pull/17338#discussion_r960188195
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on code in PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#discussion_r960370138
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -264,13 +265,19 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth
}
boolean isEmpty;
- List<String> topics;
+ Set<String> partitionedTopicNames = new HashSet<>();
+ Set<String> topicNames = new HashSet<>();
try {
- topics = pulsar().getNamespaceService().getListOfPersistentTopics(namespaceName)
- .get(config().getMetadataStoreOperationTimeoutSeconds(), TimeUnit.SECONDS);
- topics.addAll(getPartitionedTopicList(TopicDomain.persistent));
- topics.addAll(getPartitionedTopicList(TopicDomain.non_persistent));
- isEmpty = topics.isEmpty();
+ if (policies == null || CollectionUtils.isEmpty(policies.replication_clusters)){
Review Comment:
Hi @nodece
The cause has been identified:
1. Firstly: creating `non persistent topic` will deny when not set replication_clusters.
2. The namespace created in V1 must manually set the `replicator` policy to use `non persistent topic`
3. The namespace created in V2 will automatically set `replicator` policy
So this code is correct.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on a diff in pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on code in PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#discussion_r971420417
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -307,20 +312,33 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth
}
// remove from owned namespace map and ephemeral node from ZK
- final List<CompletableFuture<Void>> futures = Lists.newArrayList();
+ final List<CompletableFuture<Void>> deletePartitionedTopicFuture = Lists.newArrayList();
Review Comment:
```suggestion
final List<CompletableFuture<Void>> deleteNonPartitionedTopicFuture = Lists.newArrayList();
```
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -307,20 +312,33 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth
}
// remove from owned namespace map and ephemeral node from ZK
- final List<CompletableFuture<Void>> futures = Lists.newArrayList();
+ final List<CompletableFuture<Void>> deletePartitionedTopicFuture = Lists.newArrayList();
// remove system topics first.
- if (!topics.isEmpty()) {
- for (String topic : topics) {
+ if (!nonPartitionedTopicNames.isEmpty()) {
+ for (String topic : nonPartitionedTopicNames) {
try {
- futures.add(pulsar().getAdminClient().topics().deleteAsync(topic, true, true));
+ deletePartitionedTopicFuture.add(pulsar().getAdminClient().topics().deleteAsync(topic, true));
} catch (Exception ex) {
- log.error("[{}] Failed to delete system topic {}", clientAppId(), topic, ex);
+ log.error("[{}] Failed to delete partitioned system topic {}", clientAppId(), topic, ex);
Review Comment:
```suggestion
log.error("[{}] Failed to delete non-partitioned system topic {}", clientAppId(), topic, ex);
```
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#issuecomment-1235035899
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#issuecomment-1233540994
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#issuecomment-1233669284
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nodece commented on a diff in pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#discussion_r960188195
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -307,20 +314,33 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth
}
// remove from owned namespace map and ephemeral node from ZK
- final List<CompletableFuture<Void>> futures = Lists.newArrayList();
+ final List<CompletableFuture<Void>> deletePartitionedTopicFuture = Lists.newArrayList();
// remove system topics first.
- if (!topics.isEmpty()) {
- for (String topic : topics) {
+ if (!partitionedTopicNames.isEmpty()) {
+ for (String topic : partitionedTopicNames) {
try {
- futures.add(pulsar().getAdminClient().topics().deleteAsync(topic, true, true));
Review Comment:
Oh, I see, if so, I suggest we need to check the topic metadata.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nodece commented on a diff in pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#discussion_r960187424
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -282,7 +289,13 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth
log.debug("Found topics on namespace {}", namespaceName);
}
boolean hasNonSystemTopic = false;
- for (String topic : topics) {
+ for (String topic : partitionedTopicNames) {
+ if (!pulsar().getBrokerService().isSystemTopic(TopicName.get(topic))) {
+ hasNonSystemTopic = true;
+ break;
+ }
+ }
+ for (String topic : topicNames) {
Review Comment:
If `hasNonSystemTopic` is `true`, it is unnecessary here, please add a check.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#issuecomment-1246154649
@Technoboy- could this PR merge (^_^)
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#issuecomment-1248368029
#17230 already fixed this problem, so this PR keeps only the additional test cases
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on code in PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#discussion_r971471989
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -307,20 +312,33 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth
}
// remove from owned namespace map and ephemeral node from ZK
- final List<CompletableFuture<Void>> futures = Lists.newArrayList();
+ final List<CompletableFuture<Void>> deletePartitionedTopicFuture = Lists.newArrayList();
// remove system topics first.
- if (!topics.isEmpty()) {
- for (String topic : topics) {
+ if (!nonPartitionedTopicNames.isEmpty()) {
+ for (String topic : nonPartitionedTopicNames) {
try {
- futures.add(pulsar().getAdminClient().topics().deleteAsync(topic, true, true));
+ deletePartitionedTopicFuture.add(pulsar().getAdminClient().topics().deleteAsync(topic, true));
} catch (Exception ex) {
- log.error("[{}] Failed to delete system topic {}", clientAppId(), topic, ex);
+ log.error("[{}] Failed to delete partitioned system topic {}", clientAppId(), topic, ex);
Review Comment:
already fixed
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -307,20 +312,33 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth
}
// remove from owned namespace map and ephemeral node from ZK
- final List<CompletableFuture<Void>> futures = Lists.newArrayList();
+ final List<CompletableFuture<Void>> deletePartitionedTopicFuture = Lists.newArrayList();
Review Comment:
already fixed
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on code in PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#discussion_r960332153
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -307,20 +314,33 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth
}
// remove from owned namespace map and ephemeral node from ZK
- final List<CompletableFuture<Void>> futures = Lists.newArrayList();
+ final List<CompletableFuture<Void>> deletePartitionedTopicFuture = Lists.newArrayList();
// remove system topics first.
- if (!topics.isEmpty()) {
- for (String topic : topics) {
+ if (!partitionedTopicNames.isEmpty()) {
+ for (String topic : partitionedTopicNames) {
try {
- futures.add(pulsar().getAdminClient().topics().deleteAsync(topic, true, true));
Review Comment:
yes. `getPartitionedTopicList` did it
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#issuecomment-1233225670
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#issuecomment-1236391159
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] michaeljmarshall commented on pull request #17338: [test][admin]add test case: delete namespace when has partitioned system topic
Posted by "michaeljmarshall (via GitHub)" <gi...@apache.org>.
michaeljmarshall commented on PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#issuecomment-1610272072
As discussed on the mailing list https://lists.apache.org/thread/w4jzk27qhtosgsz7l9bmhf1t7o9mxjhp, there is no plan to release 2.9.6, so I am going to remove the release/2.9.6 label
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nodece commented on a diff in pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#discussion_r959152289
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -282,7 +283,13 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth
log.debug("Found topics on namespace {}", namespaceName);
}
boolean hasNonSystemTopic = false;
- for (String topic : topics) {
+ for (String topic : partitionedTopicNames) {
+ if (!pulsar().getBrokerService().isSystemTopic(TopicName.get(topic))) {
+ hasNonSystemTopic = true;
+ break;
+ }
+ }
+ for (String topic : topicNames) {
Review Comment:
Optimize:
```suggestion
if (hasNonSystemTopic == false) {
for (String topic : topicNames) {
```
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on a diff in pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on code in PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#discussion_r959482478
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -282,7 +283,13 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth
log.debug("Found topics on namespace {}", namespaceName);
}
boolean hasNonSystemTopic = false;
- for (String topic : topics) {
+ for (String topic : partitionedTopicNames) {
+ if (!pulsar().getBrokerService().isSystemTopic(TopicName.get(topic))) {
+ hasNonSystemTopic = true;
+ break;
+ }
+ }
+ for (String topic : topicNames) {
Review Comment:
We don't need go through partitionedTopicNames here.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on code in PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#discussion_r960324181
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -264,13 +265,19 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth
}
boolean isEmpty;
- List<String> topics;
+ Set<String> partitionedTopicNames = new HashSet<>();
+ Set<String> topicNames = new HashSet<>();
try {
- topics = pulsar().getNamespaceService().getListOfPersistentTopics(namespaceName)
- .get(config().getMetadataStoreOperationTimeoutSeconds(), TimeUnit.SECONDS);
- topics.addAll(getPartitionedTopicList(TopicDomain.persistent));
- topics.addAll(getPartitionedTopicList(TopicDomain.non_persistent));
- isEmpty = topics.isEmpty();
+ if (policies == null || CollectionUtils.isEmpty(policies.replication_clusters)){
Review Comment:
Now, create `non persistent topic` will denied when not set `replication_clusters`.
https://github.com/apache/pulsar/blob/d4a66a48b008ee39a78a45fed7d724dde7148710/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/NonPersistentTopics.java#L157-L158
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#issuecomment-1231364342
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#issuecomment-1233518420
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#issuecomment-1236172393
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on code in PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#discussion_r968633448
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -281,16 +288,17 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth
if (log.isDebugEnabled()) {
log.debug("Found topics on namespace {}", namespaceName);
}
- boolean hasNonSystemTopic = false;
- for (String topic : topics) {
+ for (String topic : partitionedTopicNames) {
if (!pulsar().getBrokerService().isSystemTopic(TopicName.get(topic))) {
- hasNonSystemTopic = true;
- break;
+ asyncResponse.resume(new RestException(Status.CONFLICT, "Cannot delete non empty namespace"));
Review Comment:
Good idea. Already add log line
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#issuecomment-1243990748
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] dlg99 commented on pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
dlg99 commented on PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#issuecomment-1231859644
Related PRs, please take a look: https://github.com/apache/pulsar/pull/15141
and https://github.com/apache/pulsar/pull/17308
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nodece commented on a diff in pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#discussion_r960186966
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -264,13 +265,19 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth
}
boolean isEmpty;
- List<String> topics;
+ Set<String> partitionedTopicNames = new HashSet<>();
+ Set<String> topicNames = new HashSet<>();
try {
- topics = pulsar().getNamespaceService().getListOfPersistentTopics(namespaceName)
- .get(config().getMetadataStoreOperationTimeoutSeconds(), TimeUnit.SECONDS);
- topics.addAll(getPartitionedTopicList(TopicDomain.persistent));
- topics.addAll(getPartitionedTopicList(TopicDomain.non_persistent));
- isEmpty = topics.isEmpty();
+ if (policies == null || CollectionUtils.isEmpty(policies.replication_clusters)){
+ partitionedTopicNames.addAll(pulsar().getNamespaceService().getListOfPersistentTopics(namespaceName)
+ .get(config().getMetadataStoreOperationTimeoutSeconds(), TimeUnit.SECONDS));
+ } else {
+ partitionedTopicNames.addAll(pulsar().getNamespaceService().getFullListOfTopics(namespaceName)
+ .get(config().getMetadataStoreOperationTimeoutSeconds(), TimeUnit.SECONDS));
+ }
+ topicNames.addAll(getPartitionedTopicList(TopicDomain.persistent));
Review Comment:
`getFullListOfTopics()` includes the persistent and non-persistent topics.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nodece commented on a diff in pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#discussion_r960186762
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -264,13 +265,19 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth
}
boolean isEmpty;
- List<String> topics;
+ Set<String> partitionedTopicNames = new HashSet<>();
+ Set<String> topicNames = new HashSet<>();
try {
- topics = pulsar().getNamespaceService().getListOfPersistentTopics(namespaceName)
- .get(config().getMetadataStoreOperationTimeoutSeconds(), TimeUnit.SECONDS);
- topics.addAll(getPartitionedTopicList(TopicDomain.persistent));
- topics.addAll(getPartitionedTopicList(TopicDomain.non_persistent));
- isEmpty = topics.isEmpty();
+ if (policies == null || CollectionUtils.isEmpty(policies.replication_clusters)){
Review Comment:
Could you explain this line?
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on code in PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#discussion_r960350048
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -264,13 +265,19 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth
}
boolean isEmpty;
- List<String> topics;
+ Set<String> partitionedTopicNames = new HashSet<>();
+ Set<String> topicNames = new HashSet<>();
try {
- topics = pulsar().getNamespaceService().getListOfPersistentTopics(namespaceName)
- .get(config().getMetadataStoreOperationTimeoutSeconds(), TimeUnit.SECONDS);
- topics.addAll(getPartitionedTopicList(TopicDomain.persistent));
- topics.addAll(getPartitionedTopicList(TopicDomain.non_persistent));
- isEmpty = topics.isEmpty();
+ if (policies == null || CollectionUtils.isEmpty(policies.replication_clusters)){
Review Comment:
I think this is a problem left over from history. I am finding the reason
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on code in PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#discussion_r960329079
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -282,7 +289,13 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth
log.debug("Found topics on namespace {}", namespaceName);
}
boolean hasNonSystemTopic = false;
- for (String topic : topics) {
+ for (String topic : partitionedTopicNames) {
+ if (!pulsar().getBrokerService().isSystemTopic(TopicName.get(topic))) {
+ hasNonSystemTopic = true;
+ break;
+ }
+ }
+ for (String topic : topicNames) {
Review Comment:
Already fixed
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on code in PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#discussion_r959640019
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -307,20 +314,33 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth
}
// remove from owned namespace map and ephemeral node from ZK
- final List<CompletableFuture<Void>> futures = Lists.newArrayList();
+ final List<CompletableFuture<Void>> deletePartitionedTopicFuture = Lists.newArrayList();
// remove system topics first.
- if (!topics.isEmpty()) {
- for (String topic : topics) {
+ if (!partitionedTopicNames.isEmpty()) {
+ for (String topic : partitionedTopicNames) {
try {
- futures.add(pulsar().getAdminClient().topics().deleteAsync(topic, true, true));
Review Comment:
This is A good suggestion, but 'topicname.get(topic name).ispartitioned()' just uses the naming rule to figure out if it's a `partitioned topic`, which isn't exact. When enabled the feature `auto create topic`, users can create a non-partitioned topic name with `persistent://public/default/special-partition-0`
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -264,13 +264,14 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth
}
boolean isEmpty;
- List<String> topics;
+ Set<String> partitionedTopicNames = new HashSet<>();
+ Set<String> topicNames = new HashSet<>();
try {
- topics = pulsar().getNamespaceService().getListOfPersistentTopics(namespaceName)
- .get(config().getMetadataStoreOperationTimeoutSeconds(), TimeUnit.SECONDS);
- topics.addAll(getPartitionedTopicList(TopicDomain.persistent));
- topics.addAll(getPartitionedTopicList(TopicDomain.non_persistent));
- isEmpty = topics.isEmpty();
+ partitionedTopicNames.addAll(pulsar().getNamespaceService().getListOfPersistentTopics(namespaceName)
Review Comment:
Already fixed
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on code in PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#discussion_r959648030
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -282,7 +283,13 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth
log.debug("Found topics on namespace {}", namespaceName);
}
boolean hasNonSystemTopic = false;
- for (String topic : topics) {
+ for (String topic : partitionedTopicNames) {
+ if (!pulsar().getBrokerService().isSystemTopic(TopicName.get(topic))) {
+ hasNonSystemTopic = true;
+ break;
+ }
+ }
+ for (String topic : topicNames) {
Review Comment:
We need this check because users possibly creates and delete topics like this:
```
1. create-partitioned-topic persistent://my-tenant/my-namespace/my-topic --partitions 2
2. delete persistent://my-tenant/my-namespace/my-topic-partition-0
3. delete persistent://my-tenant/my-namespace/my-topic-partition-1
```
Now, the topic is not actually removed.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on a diff in pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on code in PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#discussion_r970955250
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -264,13 +265,19 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth
}
boolean isEmpty;
- List<String> topics;
+ Set<String> partitionedTopicNames = new HashSet<>();
+ Set<String> topicNames = new HashSet<>();
try {
- topics = pulsar().getNamespaceService().getListOfPersistentTopics(namespaceName)
- .get(config().getMetadataStoreOperationTimeoutSeconds(), TimeUnit.SECONDS);
- topics.addAll(getPartitionedTopicList(TopicDomain.persistent));
- topics.addAll(getPartitionedTopicList(TopicDomain.non_persistent));
- isEmpty = topics.isEmpty();
+ if (policies == null || CollectionUtils.isEmpty(policies.replication_clusters)){
+ partitionedTopicNames.addAll(pulsar().getNamespaceService().getListOfPersistentTopics(namespaceName)
Review Comment:
Why we add non-partitioned topics to `partitionedTopicNames`
but add partitioned topics to `topicNames`
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -281,16 +288,19 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth
if (log.isDebugEnabled()) {
log.debug("Found topics on namespace {}", namespaceName);
}
- boolean hasNonSystemTopic = false;
- for (String topic : topics) {
+ for (String topic : partitionedTopicNames) {
Review Comment:
Can we use one loop?
It looks basically the same
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#issuecomment-1242609512
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on a diff in pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
eolivelli commented on code in PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#discussion_r967667290
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -281,16 +288,17 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth
if (log.isDebugEnabled()) {
log.debug("Found topics on namespace {}", namespaceName);
}
- boolean hasNonSystemTopic = false;
- for (String topic : topics) {
+ for (String topic : partitionedTopicNames) {
if (!pulsar().getBrokerService().isSystemTopic(TopicName.get(topic))) {
- hasNonSystemTopic = true;
- break;
+ asyncResponse.resume(new RestException(Status.CONFLICT, "Cannot delete non empty namespace"));
Review Comment:
What about logging a line that reports the topic that is preventing the deletion, here and below?
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#issuecomment-1231919250
Hi @dlg99
Thanks.
> Was it a problem if the topic autocreation disabled? (e.g. see https://github.com/apache/pulsar/pull/17308 )
If it was not, maybe namespace deletion (at least the forced one) should disable topic autocreation before attempting the deletion.
#17308 try to fix https://github.com/datastax/pulsar/issues/112, that is a really difficult problem to solve, I left a comment at #17308
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nodece commented on a diff in pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#discussion_r959152289
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -282,7 +283,13 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth
log.debug("Found topics on namespace {}", namespaceName);
}
boolean hasNonSystemTopic = false;
- for (String topic : topics) {
+ for (String topic : partitionedTopicNames) {
+ if (!pulsar().getBrokerService().isSystemTopic(TopicName.get(topic))) {
+ hasNonSystemTopic = true;
+ break;
+ }
+ }
+ for (String topic : topicNames) {
Review Comment:
Optimize:
```suggestion
if (hasNonSystemTopic == false) {
for (String topic : topicNames) {
```
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#issuecomment-1236383120
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#issuecomment-1236376139
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#issuecomment-1236548529
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on code in PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#discussion_r971141233
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -264,13 +265,19 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth
}
boolean isEmpty;
- List<String> topics;
+ Set<String> partitionedTopicNames = new HashSet<>();
+ Set<String> topicNames = new HashSet<>();
try {
- topics = pulsar().getNamespaceService().getListOfPersistentTopics(namespaceName)
- .get(config().getMetadataStoreOperationTimeoutSeconds(), TimeUnit.SECONDS);
- topics.addAll(getPartitionedTopicList(TopicDomain.persistent));
- topics.addAll(getPartitionedTopicList(TopicDomain.non_persistent));
- isEmpty = topics.isEmpty();
+ if (policies == null || CollectionUtils.isEmpty(policies.replication_clusters)){
+ partitionedTopicNames.addAll(pulsar().getNamespaceService().getListOfPersistentTopics(namespaceName)
Review Comment:
Already fixed
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -281,16 +288,19 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth
if (log.isDebugEnabled()) {
log.debug("Found topics on namespace {}", namespaceName);
}
- boolean hasNonSystemTopic = false;
- for (String topic : topics) {
+ for (String topic : partitionedTopicNames) {
Review Comment:
Already fixed
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- merged pull request #17338: [test][admin]add test case: delete namespace when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
Technoboy- merged PR #17338:
URL: https://github.com/apache/pulsar/pull/17338
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on a diff in pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on code in PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#discussion_r959463610
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -264,13 +264,14 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth
}
boolean isEmpty;
- List<String> topics;
+ Set<String> partitionedTopicNames = new HashSet<>();
+ Set<String> topicNames = new HashSet<>();
try {
- topics = pulsar().getNamespaceService().getListOfPersistentTopics(namespaceName)
- .get(config().getMetadataStoreOperationTimeoutSeconds(), TimeUnit.SECONDS);
- topics.addAll(getPartitionedTopicList(TopicDomain.persistent));
- topics.addAll(getPartitionedTopicList(TopicDomain.non_persistent));
- isEmpty = topics.isEmpty();
+ partitionedTopicNames.addAll(pulsar().getNamespaceService().getListOfPersistentTopics(namespaceName)
Review Comment:
Do we miss non persistent topics here?
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nodece commented on a diff in pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#discussion_r959159720
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -307,20 +314,33 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth
}
// remove from owned namespace map and ephemeral node from ZK
- final List<CompletableFuture<Void>> futures = Lists.newArrayList();
+ final List<CompletableFuture<Void>> deletePartitionedTopicFuture = Lists.newArrayList();
// remove system topics first.
- if (!topics.isEmpty()) {
- for (String topic : topics) {
+ if (!partitionedTopicNames.isEmpty()) {
+ for (String topic : partitionedTopicNames) {
try {
- futures.add(pulsar().getAdminClient().topics().deleteAsync(topic, true, true));
Review Comment:
You only need to distinct partitionedTopicName and nonPartitionedTopicName here.
```java
TopicName topicName = TopicName.get(topic);
if (topicName.isPartitioned()) {
futures.add(pulsar().getAdminClient().topics().deletePartitionedTopicAsync(topic, true));
} else {
futures.add(pulsar().getAdminClient().topics().deleteAsync(topic, true));
}
```
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on code in PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#discussion_r960348271
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -264,13 +265,19 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth
}
boolean isEmpty;
- List<String> topics;
+ Set<String> partitionedTopicNames = new HashSet<>();
+ Set<String> topicNames = new HashSet<>();
try {
- topics = pulsar().getNamespaceService().getListOfPersistentTopics(namespaceName)
- .get(config().getMetadataStoreOperationTimeoutSeconds(), TimeUnit.SECONDS);
- topics.addAll(getPartitionedTopicList(TopicDomain.persistent));
- topics.addAll(getPartitionedTopicList(TopicDomain.non_persistent));
- isEmpty = topics.isEmpty();
+ if (policies == null || CollectionUtils.isEmpty(policies.replication_clusters)){
Review Comment:
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#issuecomment-1236379304
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#issuecomment-1236394965
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#issuecomment-1237906843
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17338: [fix][admin]delete namespace fail when has partitioned system topic
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17338:
URL: https://github.com/apache/pulsar/pull/17338#issuecomment-1246077042
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org