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/22 09:46:00 UTC
[pulsar] branch branch-2.10 updated: [fix] [admin] Make response code to 400 instead of 500 when delete topic fails due to enabled geo-replication (#19879)
This is an automated email from the ASF dual-hosted git repository.
yubiao pushed a commit to branch branch-2.10
in repository https://gitbox.apache.org/repos/asf/pulsar.git
The following commit(s) were added to refs/heads/branch-2.10 by this push:
new db6a59b0c3a [fix] [admin] Make response code to 400 instead of 500 when delete topic fails due to enabled geo-replication (#19879)
db6a59b0c3a is described below
commit db6a59b0c3a8f978c6e159a48da37544fb9da3a6
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 51525303ac0..a426aa83ff3 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
@@ -338,7 +338,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);
@@ -1104,7 +1107,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, "Topic has active producers/subscriptions");
} 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 2468ace10f8..d4d7c9d5e13 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
@@ -70,6 +70,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;
@@ -766,6 +767,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 = 4, timeOut = 30000)
public void testReplicatorProducerName() throws Exception {
log.info("--- Starting ReplicatorTest::testReplicatorProducerName ---");