AnonHxy commented on code in PR #17957:
URL: https://github.com/apache/pulsar/pull/17957#discussion_r989798066


##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/service/BacklogQuotaManagerTest.java:
##########
@@ -111,7 +111,7 @@ void setup() throws Exception {
             config.setBrokerServicePort(Optional.of(0));
             config.setAuthorizationEnabled(false);
             config.setAuthenticationEnabled(false);
-            
config.setBacklogQuotaCheckIntervalInSeconds(TIME_TO_CHECK_BACKLOG_QUOTA);
+            
config.setBacklogQuotaCheckIntervalInSeconds(TIME_TO_CHECK_BACKLOG_QUOTA / 2);

Review Comment:
   It looks that this is a common config,  and this change  maybe break other 
test case



##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/service/BacklogQuotaManagerTest.java:
##########
@@ -527,6 +527,60 @@ public void testConsumerBacklogEvictionTimeQuota() throws 
Exception {
         client.close();
     }
 
+    @Test(timeOut = 60000)
+    public void testConsumerBacklogEvictionTimeQuotaWithoutEviction() throws 
Exception {
+        assertEquals(admin.namespaces().getBacklogQuotaMap("prop/ns-quota"),
+                new HashMap<>());
+        admin.namespaces().setBacklogQuota("prop/ns-quota",
+                BacklogQuota.builder()
+                        .limitTime(5) // set limit time as 5 seconds
+                        
.retentionPolicy(BacklogQuota.RetentionPolicy.consumer_backlog_eviction)
+                        .build(), BacklogQuota.BacklogQuotaType.message_age);
+        PulsarClient client = 
PulsarClient.builder().serviceUrl(adminUrl.toString()).statsInterval(0, 
TimeUnit.SECONDS)
+                .build();
+
+        final String topic1 = "persistent://prop/ns-quota/topic3" + 
UUID.randomUUID();
+        final String subName1 = "c1";
+        final String subName2 = "c2";
+        int numMsgs = 5;
+
+        Consumer<byte[]> consumer1 = 
client.newConsumer().topic(topic1).subscriptionName(subName1).subscribe();
+        Consumer<byte[]> consumer2 = 
client.newConsumer().topic(topic1).subscriptionName(subName2).subscribe();
+        org.apache.pulsar.client.api.Producer<byte[]> producer = 
createProducer(client, topic1);
+        byte[] content = new byte[1024];
+        for (int i = 0; i < numMsgs; i++) {
+            producer.send(content);
+            consumer1.receive();
+            consumer2.receive();
+        }
+
+        TopicStats stats = getTopicStats(topic1);
+        assertEquals(stats.getSubscriptions().get(subName1).getMsgBacklog(), 
5);
+        assertEquals(stats.getSubscriptions().get(subName2).getMsgBacklog(), 
5);
+
+        // Sleep 5000 mills for first 5 messages.
+        Thread.sleep(5000l);
+        numMsgs = 9;
+        for (int i = 0; i < numMsgs; i++) {
+            producer.send(content);
+            consumer1.receive();
+            consumer2.receive();

Review Comment:
   It seems that there is no need for the two line above



##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/service/BacklogQuotaManagerTest.java:
##########
@@ -527,6 +527,60 @@ public void testConsumerBacklogEvictionTimeQuota() throws 
Exception {
         client.close();
     }
 
+    @Test(timeOut = 60000)
+    public void testConsumerBacklogEvictionTimeQuotaWithoutEviction() throws 
Exception {
+        assertEquals(admin.namespaces().getBacklogQuotaMap("prop/ns-quota"),
+                new HashMap<>());
+        admin.namespaces().setBacklogQuota("prop/ns-quota",
+                BacklogQuota.builder()
+                        .limitTime(5) // set limit time as 5 seconds
+                        
.retentionPolicy(BacklogQuota.RetentionPolicy.consumer_backlog_eviction)
+                        .build(), BacklogQuota.BacklogQuotaType.message_age);
+        PulsarClient client = 
PulsarClient.builder().serviceUrl(adminUrl.toString()).statsInterval(0, 
TimeUnit.SECONDS)
+                .build();
+
+        final String topic1 = "persistent://prop/ns-quota/topic3" + 
UUID.randomUUID();
+        final String subName1 = "c1";
+        final String subName2 = "c2";
+        int numMsgs = 5;
+
+        Consumer<byte[]> consumer1 = 
client.newConsumer().topic(topic1).subscriptionName(subName1).subscribe();
+        Consumer<byte[]> consumer2 = 
client.newConsumer().topic(topic1).subscriptionName(subName2).subscribe();
+        org.apache.pulsar.client.api.Producer<byte[]> producer = 
createProducer(client, topic1);
+        byte[] content = new byte[1024];
+        for (int i = 0; i < numMsgs; i++) {
+            producer.send(content);
+            consumer1.receive();
+            consumer2.receive();
+        }
+
+        TopicStats stats = getTopicStats(topic1);
+        assertEquals(stats.getSubscriptions().get(subName1).getMsgBacklog(), 
5);
+        assertEquals(stats.getSubscriptions().get(subName2).getMsgBacklog(), 
5);
+
+        // Sleep 5000 mills for first 5 messages.
+        Thread.sleep(5000l);

Review Comment:
   We'd better use '5000L'  instead of '5000l'



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to