You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/03/31 07:39:53 UTC

[GitHub] [pulsar] nodece opened a new pull request #14968: [test][broker] Increase batch publish delay

nodece opened a new pull request #14968:
URL: https://github.com/apache/pulsar/pull/14968


   Signed-off-by: Zixuan Liu <no...@gmail.com>
   
   ### Motivation
   
   When the `batchingMaxPublishDelay` is too short, the client will publish the batch to the broker immediately, this will break the test.
   
   ### Modifications
   
   - Increase batch publish delay to `Integer.MAX_VALUE`
   
   ### Documentation
   
   - [x] `no-need-doc` 
     
   Fix tests
   
   
   


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] nodece commented on a change in pull request #14968: [test][broker] Increase batch publish delay

Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #14968:
URL: https://github.com/apache/pulsar/pull/14968#discussion_r839374181



##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/service/BatchMessageTest.java
##########
@@ -253,7 +251,8 @@ public void testSimpleBatchProducerWithFixedBatchSizeAndTime(CompressionType com
         consumer.close();
 
         Producer<byte[]> producer = pulsarClient.newProducer().topic(topicName)
-                .batchingMaxPublishDelay(10, TimeUnit.MILLISECONDS).batchingMaxMessages(5)
+                .batchingMaxPublishDelay(Integer.MAX_VALUE, TimeUnit.MILLISECONDS)

Review comment:
       Let me check every test carefully.




-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] RobertIndie commented on a change in pull request #14968: [test][broker] Increase batch publish delay

Posted by GitBox <gi...@apache.org>.
RobertIndie commented on a change in pull request #14968:
URL: https://github.com/apache/pulsar/pull/14968#discussion_r839312652



##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/service/BatchMessageTest.java
##########
@@ -217,7 +214,8 @@ public void testSimpleBatchProducerWithFixedBatchTime(CompressionType compressio
         consumer.close();
 
         Producer<byte[]> producer = pulsarClient.newProducer().topic(topicName).compressionType(compressionType)
-                .batchingMaxPublishDelay(10, TimeUnit.MILLISECONDS).enableBatching(true)
+                .batchingMaxPublishDelay(Integer.MAX_VALUE, TimeUnit.MILLISECONDS)

Review comment:
       We should not change here. This will cause the test to hang forever.

##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/service/BatchMessageTest.java
##########
@@ -253,7 +251,8 @@ public void testSimpleBatchProducerWithFixedBatchSizeAndTime(CompressionType com
         consumer.close();
 
         Producer<byte[]> producer = pulsarClient.newProducer().topic(topicName)
-                .batchingMaxPublishDelay(10, TimeUnit.MILLISECONDS).batchingMaxMessages(5)
+                .batchingMaxPublishDelay(Integer.MAX_VALUE, TimeUnit.MILLISECONDS)

Review comment:
       I think 10 mills here may be intended.




-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] nodece commented on pull request #14968: [test][broker] Increase batch publish delay

Posted by GitBox <gi...@apache.org>.
nodece commented on pull request #14968:
URL: https://github.com/apache/pulsar/pull/14968#issuecomment-1084313920


   @RobertIndie Thanks.
   
   > Are these flaky tests? 
   
   Yes, some batch tests are unstable.
   
   > Could you post the stacktrace of these failed flaky tests? 
   
   org.apache.pulsar.broker.service.BatchMessageTest#testBatchProducerWithLargeMessage
   
   ```
   java.lang.AssertionError: 
   Expected :3
   Actual   :4
   <Click to see difference>
   
   
   	at org.testng.Assert.fail(Assert.java:99)
   	at org.testng.Assert.failNotEquals(Assert.java:1037)
   	at org.testng.Assert.assertEqualsImpl(Assert.java:140)
   	at org.testng.Assert.assertEquals(Assert.java:122)
   	at org.testng.Assert.assertEquals(Assert.java:797)
   	at org.testng.Assert.assertEquals(Assert.java:807)
   	at org.apache.pulsar.broker.service.BatchMessageTest.testBatchProducerWithLargeMessage(BatchMessageTest.java:322)
   	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
   	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
   	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
   	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
   	at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:132)
   	at org.testng.internal.TestInvoker.invokeMethod(TestInvoker.java:599)
   	at org.testng.internal.TestInvoker.invokeTestMethod(TestInvoker.java:174)
   	at org.testng.internal.MethodRunner.runInSequence(MethodRunner.java:46)
   	at org.testng.internal.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:822)
   	at org.testng.internal.TestInvoker.invokeTestMethods(TestInvoker.java:147)
   	at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:146)
   	at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:128)
   	at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)
   	at org.testng.TestRunner.privateRun(TestRunner.java:764)
   	at org.testng.TestRunner.run(TestRunner.java:585)
   	at org.testng.SuiteRunner.runTest(SuiteRunner.java:384)
   	at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:378)
   	at org.testng.SuiteRunner.privateRun(SuiteRunner.java:337)
   	at org.testng.SuiteRunner.run(SuiteRunner.java:286)
   	at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:53)
   	at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:96)
   	at org.testng.TestNG.runSuitesSequentially(TestNG.java:1218)
   	at org.testng.TestNG.runSuitesLocally(TestNG.java:1140)
   	at org.testng.TestNG.runSuites(TestNG.java:1069)
   	at org.testng.TestNG.run(TestNG.java:1037)
   	at com.intellij.rt.testng.IDEARemoteTestNG.run(IDEARemoteTestNG.java:66)
   	at com.intellij.rt.testng.RemoteTestNGStarter.main(RemoteTestNGStarter.java:109)
   ```
   
   > This sounds weird. I think 5 seconds is enough time to send batch messages in these tests.
   
   Sometimes sending these messages needs more time. I think I can increase it to 10 seconds.
   


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] nodece commented on a change in pull request #14968: [test][broker] Increase batch publish delay

Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #14968:
URL: https://github.com/apache/pulsar/pull/14968#discussion_r839364888



##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/service/BatchMessageTest.java
##########
@@ -217,7 +214,8 @@ public void testSimpleBatchProducerWithFixedBatchTime(CompressionType compressio
         consumer.close();
 
         Producer<byte[]> producer = pulsarClient.newProducer().topic(topicName).compressionType(compressionType)
-                .batchingMaxPublishDelay(10, TimeUnit.MILLISECONDS).enableBatching(true)
+                .batchingMaxPublishDelay(Integer.MAX_VALUE, TimeUnit.MILLISECONDS)

Review comment:
       Good catch!




-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org