From 0ba4b20d2cc9e8f86cb2bc17ba7b4857692fb04d Mon Sep 17 00:00:00 2001 From: rajan Date: Tue, 12 Apr 2022 15:01:53 -0700 Subject: [PATCH] [pulsar-broker] Fix delete empty namespace with partitioned-topic metadata --- .../broker/admin/impl/NamespacesBase.java | 6 +---- .../pulsar/broker/admin/AdminApi2Test.java | 23 +++++++++++++++++++ .../pulsar/broker/admin/NamespacesTest.java | 16 ++++++------- 3 files changed, 31 insertions(+), 14 deletions(-) diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java index 49a221946ee35..a8af983dfdf1f 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java @@ -68,7 +68,6 @@ import org.apache.pulsar.common.naming.NamespaceBundleSplitAlgorithm; import org.apache.pulsar.common.naming.NamespaceBundles; import org.apache.pulsar.common.naming.NamespaceName; -import org.apache.pulsar.common.naming.TopicDomain; import org.apache.pulsar.common.naming.TopicName; import org.apache.pulsar.common.policies.data.AuthAction; import org.apache.pulsar.common.policies.data.AutoSubscriptionCreationOverride; @@ -225,12 +224,9 @@ protected void internalDeleteNamespace(AsyncResponse asyncResponse, boolean auth boolean isEmpty; List topics; try { - topics = pulsar().getNamespaceService().getListOfPersistentTopics(namespaceName) + topics = pulsar().getNamespaceService().getFullListOfTopics(namespaceName) .get(config().getMetadataStoreOperationTimeoutSeconds(), TimeUnit.SECONDS); - topics.addAll(getPartitionedTopicList(TopicDomain.persistent)); - topics.addAll(getPartitionedTopicList(TopicDomain.non_persistent)); isEmpty = topics.isEmpty(); - } catch (Exception e) { asyncResponse.resume(new RestException(e)); return; diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApi2Test.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApi2Test.java index a6b89112634de..dbf4a42ca2253 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApi2Test.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApi2Test.java @@ -2431,4 +2431,27 @@ public void testPartitionedStatsAggregationByProducerNamePerPartition(String top assertEquals(topicStats.getPublishers().size(), 2); topicStats.getPublishers().forEach(p -> assertTrue(p.isSupportsPartialProducer())); } + + @Test + public void testDeleteNamespaceWithDeletedPartitionedTopic() throws Exception { + final String namespace = "prop-xyz/ns-delete"; + admin.namespaces().createNamespace(namespace); + final String topicName = "persistent://" + namespace + "/delete-ns-topic"; + admin.topics().createPartitionedTopic(topicName, 10); + List topics = pulsar.getPulsarResources().getTopicResources() + .listPersistentTopicsAsync(NamespaceName.get(namespace)).get(); + topics.forEach(topic -> { + try { + admin.topics().delete(topic); + } catch (PulsarAdminException e) { + fail("should not fail to delete"); + } + }); + admin.namespaces().deleteNamespace(namespace); + try { + admin.namespaces().getPolicies(namespace); + } catch (PulsarAdminException.NotFoundException e) { + // Ok.. + } + } } diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java index a334fceed766b..05fb35ea6df15 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java @@ -729,20 +729,19 @@ public void testDeleteNamespaces() throws Exception { response = mock(AsyncResponse.class); namespaces.deleteNamespace(response, testNs.getTenant(), testNs.getCluster(), testNs.getLocalName(), false, false); - errorCaptor = ArgumentCaptor.forClass(RestException.class); // Ok, namespace not empty - verify(response, timeout(5000).times(1)).resume(errorCaptor.capture()); - assertEquals(errorCaptor.getValue().getResponse().getStatus(), Status.CONFLICT.getStatusCode()); - - mockZooKeeperGlobal.delete("/admin/partitioned-topics/" + topicName.getPersistenceNamingEncoding(), -1); + ArgumentCaptor responseCaptor = ArgumentCaptor.forClass(Response.class); + verify(response, timeout(5000).times(1)).resume(responseCaptor.capture()); + assertEquals(responseCaptor.getValue().getStatus(), Status.NO_CONTENT.getStatusCode()); testNs = this.testGlobalNamespaces.get(0); + namespaces.setNamespaceReplicationClusters(testNs.getTenant(), testNs.getCluster(), testNs.getLocalName(), Lists.newArrayList("use")); // setup ownership to localhost doReturn(Optional.of(localWebServiceUrl)).when(nsSvc).getWebServiceUrl(testNs, options); doReturn(true).when(nsSvc).isServiceUnitOwned(testNs); response = mock(AsyncResponse.class); namespaces.deleteNamespace(response, testNs.getTenant(), testNs.getCluster(), testNs.getLocalName(), false, false); - ArgumentCaptor responseCaptor = ArgumentCaptor.forClass(Response.class); + responseCaptor = ArgumentCaptor.forClass(Response.class); verify(response, timeout(5000).times(1)).resume(responseCaptor.capture()); assertEquals(responseCaptor.getValue().getStatus(), Status.NO_CONTENT.getStatusCode()); @@ -755,12 +754,11 @@ public void testDeleteNamespaces() throws Exception { responseCaptor = ArgumentCaptor.forClass(Response.class); verify(response, timeout(5000).times(1)).resume(responseCaptor.capture()); assertEquals(responseCaptor.getValue().getStatus(), Status.NO_CONTENT.getStatusCode()); - List nsList = Lists.newArrayList(this.testLocalNamespaces.get(1).toString(), - this.testLocalNamespaces.get(2).toString()); + List nsList = Lists.newArrayList(this.testLocalNamespaces.get(2).toString()); nsList.sort(null); assertEquals(namespaces.getTenantNamespaces(this.testTenant), nsList); - testNs = this.testLocalNamespaces.get(1); + testNs = this.testLocalNamespaces.get(2); // setup ownership to localhost doReturn(Optional.of(localWebServiceUrl)).when(nsSvc).getWebServiceUrl(testNs, options); doReturn(true).when(nsSvc).isServiceUnitOwned(testNs);