gemmellr commented on code in PR #5291:
URL: https://github.com/apache/activemq-artemis/pull/5291#discussion_r1854201999


##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/client/MessageHandlerTest.java:
##########
@@ -117,6 +118,42 @@ public void onMessage(final ClientMessage message) {
       session.close();
    }
 
+   @Test
+   public void testMessageHandlerCloseTimeout() throws Exception {
+      // create Netty acceptor so client can use new onMessageCloseTimeout URL 
parameter
+      server.getRemotingService().createAcceptor("netty", 
"tcp://127.0.0.1:61616").start();
+      final int timeout = 1000;
+      locator = 
ActiveMQClient.createServerLocator("tcp://127.0.0.1:61616?onMessageCloseTimeout="
 + timeout);
+      sf = createSessionFactory(locator);
+      ClientSession session = addClientSession(sf.createSession(false, true, 
true));
+      session.createQueue(QueueConfiguration.of(QUEUE).setDurable(false));
+      ClientProducer producer = session.createProducer(QUEUE);
+      producer.send(createTextMessage(session, "m"));
+
+      ClientConsumer consumer = session.createConsumer(QUEUE, null, false);
+
+      session.start();
+
+      CountDownLatch latch = new CountDownLatch(1);
+      consumer.setMessageHandler(message -> {
+         latch.countDown();
+         // don't just Thread.sleep() here because it will be interrupted on 
ClientConsumer.close()
+         long start = System.currentTimeMillis();
+         while (System.currentTimeMillis() - start < 2000) {
+            try {
+               Thread.sleep(100);
+            } catch (InterruptedException e) {
+               // ignore
+            }
+         }
+      });
+      latch.await();
+      long start = System.currentTimeMillis();
+      consumer.close();
+      long end = System.currentTimeMillis();
+      assertTrue( (end - start >= timeout) && (end - start <= 2000), "Closing 
consumer took " + (end - start) + "ms");

Review Comment:
   This actually seems more brittle now than it was when I commented on it 
originally.
   
   Faster than before, yes (though could still be faster, if not just relying 
on friendly scheduling), but also more brittle as a result of now needing 
separately scheduled things to always happen <600ms apart, rather than <1000ms 
before.
   
   Governing the exit point of the handler rather than just the time would 
entirely avoid the brittle reliance on nice scheduling.



-- 
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: gitbox-unsubscr...@activemq.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact


Reply via email to