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:19:59 UTC
[pulsar] branch branch-2.9 updated (4e2105ab69b -> dacd8ec46e9)
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 4e2105ab69b [fix] [admin] Make response code to 400 instead of 500 when delete topic fails due to enabled geo-replication (#19879)
new c2d24ba0ebf Revert "[fix] [admin] Make response code to 400 instead of 500 when delete topic fails due to enabled geo-replication (#19879)"
new dacd8ec46e9 [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:
.../src/test/java/org/apache/pulsar/broker/service/ReplicatorTest.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[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 c2d24ba0ebfd9a8453405c0895a9991f9c3d0e74
Author: fengyubiao <yu...@streamnative.io>
AuthorDate: Mon Mar 27 19:10:58 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 4e2105ab69b5621e2553985a58364b17698733e5.
---
.../broker/admin/impl/PersistentTopicsBase.java | 9 ++-----
.../pulsar/broker/service/ReplicatorTest.java | 30 +---------------------
2 files changed, 3 insertions(+), 36 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 a559ccb06d5..e8ac4bb457d 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,10 +314,7 @@ public class PersistentTopicsBase extends AdminResource {
try {
pulsar().getBrokerService().deleteTopic(topicName.toString(), true, deleteSchema).get();
} catch (Exception e) {
- Throwable t = FutureUtil.unwrapCompletionException(e);
- if (t instanceof IllegalStateException){
- throw new RestException(422/* Unprocessable entity*/, t.getMessage());
- } else if (isManagedLedgerNotFoundException(e)) {
+ if (isManagedLedgerNotFoundException(e)) {
log.info("[{}] Topic was already not existing {}", clientAppId(), topicName, e);
} else {
log.error("[{}] Failed to delete topic forcefully {}", clientAppId(), topicName, e);
@@ -1043,9 +1040,7 @@ public class PersistentTopicsBase extends AdminResource {
} catch (Exception e) {
Throwable t = e.getCause();
log.error("[{}] Failed to delete topic {}", clientAppId(), topicName, t);
- if (t instanceof IllegalStateException){
- throw new RestException(422/* Unprocessable entity*/, t.getMessage());
- } else if (t instanceof TopicBusyException) {
+ 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 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 dacd8ec46e9fc293092abd14854e005d6b2515dc
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 | 28 ++++++++++++++++++++++
2 files changed, 35 insertions(+), 2 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..8b73c39e9e1 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,6 +861,33 @@ 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 = 5, timeOut = 30000)
public void testReplicatorProducerName() throws Exception {
log.info("--- Starting ReplicatorTest::testReplicatorProducerName ---");