From 4e9673c08b0b44773e0731e57f6d173cfc3e1763 Mon Sep 17 00:00:00 2001 From: fengyubiao Date: Sat, 21 Jan 2023 00:13:53 +0800 Subject: [PATCH 1/3] [improve] [admin] Make the default value of param --get-subscription-backlog-size of admin API topics stats true --- .../org/apache/pulsar/broker/admin/v2/PersistentTopics.java | 4 ++-- .../java/org/apache/pulsar/admin/cli/PulsarAdminToolTest.java | 4 ++-- .../src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) 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 index 2c02955c5abae..7eeaec403d842 100644 --- 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 @@ -1178,7 +1178,7 @@ public void getStats( @QueryParam("getPreciseBacklog") @DefaultValue("false") boolean getPreciseBacklog, @ApiParam(value = "If return backlog size for each subscription, require locking on ledger so be careful " + "not to use when there's heavy traffic.") - @QueryParam("subscriptionBacklogSize") @DefaultValue("false") boolean subscriptionBacklogSize, + @QueryParam("subscriptionBacklogSize") @DefaultValue("true") boolean subscriptionBacklogSize, @ApiParam(value = "If return time of the earliest message in backlog") @QueryParam("getEarliestTimeInBacklog") @DefaultValue("false") boolean getEarliestTimeInBacklog) { validateTopicName(tenant, namespace, encodedTopic); @@ -1280,7 +1280,7 @@ public void getPartitionedStats( @QueryParam("getPreciseBacklog") @DefaultValue("false") boolean getPreciseBacklog, @ApiParam(value = "If return backlog size for each subscription, require locking on ledger so be careful " + "not to use when there's heavy traffic.") - @QueryParam("subscriptionBacklogSize") @DefaultValue("false") boolean subscriptionBacklogSize, + @QueryParam("subscriptionBacklogSize") @DefaultValue("true") boolean subscriptionBacklogSize, @ApiParam(value = "If return the earliest time in backlog") @QueryParam("getEarliestTimeInBacklog") @DefaultValue("false") boolean getEarliestTimeInBacklog) { try { diff --git a/pulsar-client-tools-test/src/test/java/org/apache/pulsar/admin/cli/PulsarAdminToolTest.java b/pulsar-client-tools-test/src/test/java/org/apache/pulsar/admin/cli/PulsarAdminToolTest.java index 2a16d09ad0adb..af58aa6845626 100644 --- a/pulsar-client-tools-test/src/test/java/org/apache/pulsar/admin/cli/PulsarAdminToolTest.java +++ b/pulsar-client-tools-test/src/test/java/org/apache/pulsar/admin/cli/PulsarAdminToolTest.java @@ -1493,7 +1493,7 @@ public void topics() throws Exception { verify(mockTopics).deleteSubscription("persistent://myprop/clust/ns1/ds1", "sub1", false); cmdTopics.run(split("stats persistent://myprop/clust/ns1/ds1")); - verify(mockTopics).getStats("persistent://myprop/clust/ns1/ds1", false, false, false); + verify(mockTopics).getStats("persistent://myprop/clust/ns1/ds1", false, true, false); cmdTopics.run(split("stats-internal persistent://myprop/clust/ns1/ds1")); verify(mockTopics).getInternalStats("persistent://myprop/clust/ns1/ds1", false); @@ -1541,7 +1541,7 @@ public void topics() throws Exception { cmdTopics.run(split("partitioned-stats persistent://myprop/clust/ns1/ds1 --per-partition")); verify(mockTopics).getPartitionedStats("persistent://myprop/clust/ns1/ds1", - true, false, false, false); + true, false, true, false); cmdTopics.run(split("partitioned-stats-internal persistent://myprop/clust/ns1/ds1")); verify(mockTopics).getPartitionedInternalStats("persistent://myprop/clust/ns1/ds1"); diff --git a/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java b/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java index 192770258921a..ce801b53cc461 100644 --- a/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java +++ b/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java @@ -793,7 +793,7 @@ private class GetStats extends CliCommand { @Parameter(names = { "-sbs", "--get-subscription-backlog-size" }, description = "Set true to get backlog size for each subscription" + ", locking required.") - private boolean subscriptionBacklogSize = false; + private boolean subscriptionBacklogSize = true; @Parameter(names = { "-etb", "--get-earliest-time-in-backlog" }, description = "Set true to get earliest time in backlog") @@ -858,7 +858,7 @@ private class GetPartitionedStats extends CliCommand { @Parameter(names = { "-sbs", "--get-subscription-backlog-size" }, description = "Set true to get backlog size for each subscription" + ", locking required.") - private boolean subscriptionBacklogSize = false; + private boolean subscriptionBacklogSize = true; @Parameter(names = { "-etb", "--get-earliest-time-in-backlog" }, description = "Set true to get earliest time in backlog") From 68a7bca40af108014b918a9527866c603d5710de Mon Sep 17 00:00:00 2001 From: fengyubiao Date: Sat, 21 Jan 2023 01:19:43 +0800 Subject: [PATCH 2/3] fix test --- .../java/org/apache/pulsar/admin/cli/PulsarAdminToolTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pulsar-client-tools-test/src/test/java/org/apache/pulsar/admin/cli/PulsarAdminToolTest.java b/pulsar-client-tools-test/src/test/java/org/apache/pulsar/admin/cli/PulsarAdminToolTest.java index af58aa6845626..37b8c33e114e5 100644 --- a/pulsar-client-tools-test/src/test/java/org/apache/pulsar/admin/cli/PulsarAdminToolTest.java +++ b/pulsar-client-tools-test/src/test/java/org/apache/pulsar/admin/cli/PulsarAdminToolTest.java @@ -2107,7 +2107,7 @@ public void nonPersistentTopics() throws Exception { CmdTopics topics = new CmdTopics(() -> admin); topics.run(split("stats non-persistent://myprop/ns1/ds1")); - verify(mockTopics).getStats("non-persistent://myprop/ns1/ds1", false, false, false); + verify(mockTopics).getStats("non-persistent://myprop/ns1/ds1", false, true, false); topics.run(split("stats-internal non-persistent://myprop/ns1/ds1")); verify(mockTopics).getInternalStats("non-persistent://myprop/ns1/ds1", false); From 7f572b35ec37460f91499df39747b3ef74a6339c Mon Sep 17 00:00:00 2001 From: fengyubiao Date: Mon, 23 Jan 2023 13:21:48 +0800 Subject: [PATCH 3/3] Change the attribute 'backlogSize' in the response to -1 if set -sbs to false --- .../broker/service/persistent/PersistentSubscription.java | 2 ++ .../java/org/apache/pulsar/broker/admin/AdminApiTest.java | 7 +++++-- .../main/java/org/apache/pulsar/admin/cli/CmdTopics.java | 2 +- .../common/policies/data/stats/SubscriptionStatsImpl.java | 2 +- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentSubscription.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentSubscription.java index 21a497587245e..2012aa06b3006 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentSubscription.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentSubscription.java @@ -1131,6 +1131,8 @@ public SubscriptionStatsImpl getStats(Boolean getPreciseBacklog, boolean subscri if (subscriptionBacklogSize) { subStats.backlogSize = ((ManagedLedgerImpl) topic.getManagedLedger()) .getEstimatedBacklogSize((PositionImpl) cursor.getMarkDeletedPosition()); + } else { + subStats.backlogSize = -1; } if (getEarliestTimeInBacklog && subStats.msgBacklog > 0) { ManagedLedgerImpl managedLedger = ((ManagedLedgerImpl) cursor.getManagedLedger()); diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiTest.java index bfd015705cd03..938b3cbb63aeb 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiTest.java @@ -1234,14 +1234,16 @@ public void testGetStats() throws Exception { assertEquals(topicStats.getEarliestMsgPublishTimeInBacklogs(), 0); assertEquals(topicStats.getSubscriptions().get(subName).getEarliestMsgPublishTimeInBacklog(), 0); + assertEquals(topicStats.getSubscriptions().get(subName).getBacklogSize(), -1); // publish several messages publishMessagesOnPersistentTopic(topic, 10); Thread.sleep(1000); - topicStats = admin.topics().getStats(topic, false, false, true); + topicStats = admin.topics().getStats(topic, false, true, true); assertTrue(topicStats.getEarliestMsgPublishTimeInBacklogs() > 0); assertTrue(topicStats.getSubscriptions().get(subName).getEarliestMsgPublishTimeInBacklog() > 0); + assertTrue(topicStats.getSubscriptions().get(subName).getBacklogSize() > 0); for (int i = 0; i < 10; i++) { Message message = consumer.receive(); @@ -1249,9 +1251,10 @@ public void testGetStats() throws Exception { } Thread.sleep(1000); - topicStats = admin.topics().getStats(topic, false, false, true); + topicStats = admin.topics().getStats(topic, false, true, true); assertEquals(topicStats.getEarliestMsgPublishTimeInBacklogs(), 0); assertEquals(topicStats.getSubscriptions().get(subName).getEarliestMsgPublishTimeInBacklog(), 0); + assertEquals(topicStats.getSubscriptions().get(subName).getBacklogSize(), 0); } diff --git a/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java b/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java index ce801b53cc461..5f1e4b6dbb322 100644 --- a/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java +++ b/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java @@ -792,7 +792,7 @@ private class GetStats extends CliCommand { @Parameter(names = { "-sbs", "--get-subscription-backlog-size" }, description = "Set true to get backlog size for each subscription" - + ", locking required.") + + ", locking required. If set to false, the attribute 'backlogSize' in the response will be -1") private boolean subscriptionBacklogSize = true; @Parameter(names = { "-etb", diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/stats/SubscriptionStatsImpl.java b/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/stats/SubscriptionStatsImpl.java index 8eb77e981a883..90b1e24f3c197 100644 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/stats/SubscriptionStatsImpl.java +++ b/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/stats/SubscriptionStatsImpl.java @@ -58,7 +58,7 @@ public class SubscriptionStatsImpl implements SubscriptionStats { /** Number of entries in the subscription backlog. */ public long msgBacklog; - /** Size of backlog in byte. **/ + /** Size of backlog in byte, -1 means that the argument "subscriptionBacklogSize" is false when calling the API. **/ public long backlogSize; /** Get the publish time of the earliest message in the backlog. */