You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by yu...@apache.org on 2023/03/27 11:06:16 UTC

[pulsar] branch branch-2.9 updated (04d502946ca -> 4e2105ab69b)

This is an automated email from the ASF dual-hosted git repository.

yubiao pushed a change to branch branch-2.9
in repository https://gitbox.apache.org/repos/asf/pulsar.git


    from 04d502946ca [fix] [broker] Counter of pending send messages in Replicator incorrect if schema future not complete (#19242)
     new 0a23188c628 Revert "[fix] [admin] Make response code to 400 instead of 500 when delete topic fails due to enabled geo-replication (#19879)"
     new 4e2105ab69b [fix] [admin] Make response code to 400 instead of 500 when delete topic fails due to enabled geo-replication (#19879)

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../apache/pulsar/broker/admin/impl/PersistentTopicsBase.java    | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)


[pulsar] 01/02: Revert "[fix] [admin] Make response code to 400 instead of 500 when delete topic fails due to enabled geo-replication (#19879)"

Posted by yu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

yubiao pushed a commit to branch branch-2.9
in repository https://gitbox.apache.org/repos/asf/pulsar.git

commit 0a23188c62896ab61fc5695f670aa2b89e99d2a4
Author: fengyubiao <yu...@streamnative.io>
AuthorDate: Mon Mar 27 18:59:52 2023 +0800

    Revert "[fix] [admin] Make response code to 400 instead of 500 when delete topic fails due to enabled geo-replication (#19879)"
    
    This reverts commit a24659a7ded645692ba1bb4ded7c3a185b4790b4.
---
 .../pulsar/broker/service/ReplicatorTest.java      | 30 +---------------------
 1 file changed, 1 insertion(+), 29 deletions(-)

diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorTest.java
index e03a0c1acb9..90ecff200f7 100644
--- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorTest.java
+++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorTest.java
@@ -64,7 +64,6 @@ import org.apache.pulsar.broker.service.BrokerServiceException.NamingException;
 import org.apache.pulsar.broker.service.persistent.PersistentReplicator;
 import org.apache.pulsar.broker.service.persistent.PersistentTopic;
 import org.apache.pulsar.client.admin.PulsarAdmin;
-import org.apache.pulsar.client.admin.PulsarAdminException;
 import org.apache.pulsar.client.api.Consumer;
 import org.apache.pulsar.client.api.Message;
 import org.apache.pulsar.client.api.MessageId;
@@ -861,34 +860,7 @@ public class ReplicatorTest extends ReplicatorTestBase {
         assertNull(producer);
     }
 
-    @Test
-    public void testDeleteTopicFailure() throws Exception {
-        final String topicName = BrokerTestUtil.newUniqueName("persistent://pulsar/ns/tp_" + UUID.randomUUID());
-        admin1.topics().createNonPartitionedTopic(topicName);
-        try {
-            admin1.topics().delete(topicName);
-            fail("Delete topic should fail if enabled replicator");
-        } catch (Exception ex) {
-            assertTrue(ex instanceof PulsarAdminException);
-            assertEquals(((PulsarAdminException) ex).getStatusCode(), 422/* Unprocessable entity*/);
-        }
-    }
-
-    @Test
-    public void testDeletePartitionedTopicFailure() throws Exception {
-        final String topicName = BrokerTestUtil.newUniqueName("persistent://pulsar/ns/tp_" + UUID.randomUUID());
-        admin1.topics().createPartitionedTopic(topicName, 2);
-        admin1.topics().createSubscription(topicName, "sub1", MessageId.earliest);
-        try {
-            admin1.topics().deletePartitionedTopic(topicName);
-            fail("Delete topic should fail if enabled replicator");
-        } catch (Exception ex) {
-            assertTrue(ex instanceof PulsarAdminException);
-            assertEquals(((PulsarAdminException) ex).getStatusCode(), 422/* Unprocessable entity*/);
-        }
-    }
-
-    @Test(priority = 4, timeOut = 30000)
+    @Test(priority = 5, timeOut = 30000)
     public void testReplicatorProducerName() throws Exception {
         log.info("--- Starting ReplicatorTest::testReplicatorProducerName ---");
         final String topicName = BrokerTestUtil.newUniqueName("persistent://pulsar/ns/testReplicatorProducerName");


[pulsar] 02/02: [fix] [admin] Make response code to 400 instead of 500 when delete topic fails due to enabled geo-replication (#19879)

Posted by yu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

yubiao pushed a commit to branch branch-2.9
in repository https://gitbox.apache.org/repos/asf/pulsar.git

commit 4e2105ab69b5621e2553985a58364b17698733e5
Author: fengyubiao <yu...@streamnative.io>
AuthorDate: Wed Mar 22 16:49:13 2023 +0800

    [fix] [admin] Make response code to 400 instead of 500 when delete topic fails due to enabled geo-replication (#19879)
    
    Motivation: As expected, If geo-replication is enabled, a topic cannot be deleted. However deleting that topic returns a 500, and no further info.
    Modifications: Make response code to 400 instead of 500 when delete topic fails due to enabled geo-replication
    (cherry picked from commit a9037334a399af905fae94d2aefa5db339cbd5b1)
---
 .../broker/admin/impl/PersistentTopicsBase.java    |  9 +++++--
 .../pulsar/broker/service/ReplicatorTest.java      | 30 +++++++++++++++++++++-
 2 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
index e8ac4bb457d..a559ccb06d5 100644
--- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
+++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
@@ -314,7 +314,10 @@ public class PersistentTopicsBase extends AdminResource {
         try {
             pulsar().getBrokerService().deleteTopic(topicName.toString(), true, deleteSchema).get();
         } catch (Exception e) {
-            if (isManagedLedgerNotFoundException(e)) {
+            Throwable t = FutureUtil.unwrapCompletionException(e);
+            if (t instanceof IllegalStateException){
+                throw new RestException(422/* Unprocessable entity*/, t.getMessage());
+            } else if (isManagedLedgerNotFoundException(e)) {
                 log.info("[{}] Topic was already not existing {}", clientAppId(), topicName, e);
             } else {
                 log.error("[{}] Failed to delete topic forcefully {}", clientAppId(), topicName, e);
@@ -1040,7 +1043,9 @@ public class PersistentTopicsBase extends AdminResource {
         } catch (Exception e) {
             Throwable t = e.getCause();
             log.error("[{}] Failed to delete topic {}", clientAppId(), topicName, t);
-            if (t instanceof TopicBusyException) {
+            if (t instanceof IllegalStateException){
+                throw new RestException(422/* Unprocessable entity*/, t.getMessage());
+            } else if (t instanceof TopicBusyException) {
                 throw new RestException(Status.PRECONDITION_FAILED, t.getMessage());
             } else if (isManagedLedgerNotFoundException(e)) {
                 throw new RestException(Status.NOT_FOUND, "Topic not found");
diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorTest.java
index 90ecff200f7..e03a0c1acb9 100644
--- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorTest.java
+++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorTest.java
@@ -64,6 +64,7 @@ import org.apache.pulsar.broker.service.BrokerServiceException.NamingException;
 import org.apache.pulsar.broker.service.persistent.PersistentReplicator;
 import org.apache.pulsar.broker.service.persistent.PersistentTopic;
 import org.apache.pulsar.client.admin.PulsarAdmin;
+import org.apache.pulsar.client.admin.PulsarAdminException;
 import org.apache.pulsar.client.api.Consumer;
 import org.apache.pulsar.client.api.Message;
 import org.apache.pulsar.client.api.MessageId;
@@ -860,7 +861,34 @@ public class ReplicatorTest extends ReplicatorTestBase {
         assertNull(producer);
     }
 
-    @Test(priority = 5, timeOut = 30000)
+    @Test
+    public void testDeleteTopicFailure() throws Exception {
+        final String topicName = BrokerTestUtil.newUniqueName("persistent://pulsar/ns/tp_" + UUID.randomUUID());
+        admin1.topics().createNonPartitionedTopic(topicName);
+        try {
+            admin1.topics().delete(topicName);
+            fail("Delete topic should fail if enabled replicator");
+        } catch (Exception ex) {
+            assertTrue(ex instanceof PulsarAdminException);
+            assertEquals(((PulsarAdminException) ex).getStatusCode(), 422/* Unprocessable entity*/);
+        }
+    }
+
+    @Test
+    public void testDeletePartitionedTopicFailure() throws Exception {
+        final String topicName = BrokerTestUtil.newUniqueName("persistent://pulsar/ns/tp_" + UUID.randomUUID());
+        admin1.topics().createPartitionedTopic(topicName, 2);
+        admin1.topics().createSubscription(topicName, "sub1", MessageId.earliest);
+        try {
+            admin1.topics().deletePartitionedTopic(topicName);
+            fail("Delete topic should fail if enabled replicator");
+        } catch (Exception ex) {
+            assertTrue(ex instanceof PulsarAdminException);
+            assertEquals(((PulsarAdminException) ex).getStatusCode(), 422/* Unprocessable entity*/);
+        }
+    }
+
+    @Test(priority = 4, timeOut = 30000)
     public void testReplicatorProducerName() throws Exception {
         log.info("--- Starting ReplicatorTest::testReplicatorProducerName ---");
         final String topicName = BrokerTestUtil.newUniqueName("persistent://pulsar/ns/testReplicatorProducerName");