[improve][broker] Improve naming for delete topic error (#16965)

apache · Aug 14, 2022 · d3dd143 · d3dd143
1 parent 6fc48d1
commit d3dd143
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 7 deletions.
diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/PersistentTopics.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/PersistentTopics.java
@@ -381,7 +381,7 @@ public void deleteTopic(
                     Throwable t = FutureUtil.unwrapCompletionException(ex);
                     if (!force && (t instanceof BrokerServiceException.TopicBusyException)) {
                         ex = new RestException(Response.Status.PRECONDITION_FAILED,
-                                "Topic has active producers/subscriptions");
+                                t.getMessage());
                     }
                     if (isManagedLedgerNotFoundException(t)) {
                         ex = new RestException(Response.Status.NOT_FOUND,

diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java
@@ -1066,7 +1066,7 @@ public void deleteTopic(
                     Throwable t = FutureUtil.unwrapCompletionException(ex);
                     if (!force && (t instanceof BrokerServiceException.TopicBusyException)) {
                         ex = new RestException(Response.Status.PRECONDITION_FAILED,
-                                "Topic has active producers/subscriptions");
+                                t.getMessage());
                     }
                     if (isManagedLedgerNotFoundException(t)) {
                         ex = new RestException(Response.Status.NOT_FOUND,

diff --git a/...oker/src/main/java/org/apache/pulsar/broker/service/nonpersistent/NonPersistentTopic.java b/...oker/src/main/java/org/apache/pulsar/broker/service/nonpersistent/NonPersistentTopic.java
@@ -419,7 +419,8 @@ private CompletableFuture<Void> delete(boolean failIfHasSubscriptions, boolean c
                     if (failIfHasSubscriptions) {
                         if (!subscriptions.isEmpty()) {
                             isFenced = false;
-                            deleteFuture.completeExceptionally(new TopicBusyException("Topic has subscriptions"));
+                            deleteFuture.completeExceptionally(
+                                    new TopicBusyException("Topic has subscriptions:" + subscriptions.keys()));
                             return;
                         }
                     } else {

diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java
@@ -1130,9 +1130,15 @@ private CompletableFuture<Void> delete(boolean failIfHasSubscriptions,
                 log.warn("[{}] Topic is already being closed or deleted", topic);
                 return FutureUtil.failedFuture(new TopicFencedException("Topic is already fenced"));
             } else if (failIfHasSubscriptions && !subscriptions.isEmpty()) {
-                return FutureUtil.failedFuture(new TopicBusyException("Topic has subscriptions"));
+                return FutureUtil.failedFuture(
+                        new TopicBusyException("Topic has subscriptions: " + subscriptions.keys()));
             } else if (failIfHasBacklogs && hasBacklogs()) {
-                return FutureUtil.failedFuture(new TopicBusyException("Topic has subscriptions did not catch up"));
+                List<String> backlogSubs =
+                        subscriptions.values().stream()
+                                .filter(sub -> sub.getNumberOfEntriesInBacklog(false) > 0)
+                                .map(PersistentSubscription::getName).toList();
+                return FutureUtil.failedFuture(
+                        new TopicBusyException("Topic has subscriptions did not catch up: " + backlogSubs));
             }
 
             fenceTopicToCloseOrDelete(); // Avoid clients reconnections while deleting

diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/schema/SchemaTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/schema/SchemaTest.java
@@ -846,7 +846,7 @@ public void testDeleteTopicAndSchemaForV1() throws Exception {
         } catch (Exception e) {
             assertThat(e.getMessage())
                     .isNotNull()
-                    .startsWith("Topic has active producers/subscriptions");
+                    .startsWith("Topic has 2 connected producers/consumers");
         }
         assertEquals(this.getPulsar().getSchemaRegistryService()
                 .trimDeletedSchemaAndGetList(TopicName.get(topic1).getSchemaName()).get().size(), 2);
@@ -936,7 +936,7 @@ public void testDeleteTopicAndSchemaForV2() throws Exception {
             admin.topics().delete(topicOne, false);
             fail();
         } catch (Exception e) {
-            assertTrue(e.getMessage().startsWith("Topic has active producers/subscriptions"));
+            assertTrue(e.getMessage().startsWith("Topic has 2 connected producers/consumers"));
         }
         assertEquals(this.getPulsar().getSchemaRegistryService()
                 .trimDeletedSchemaAndGetList(TopicName.get(topicOne).getSchemaName()).get().size(), 2);