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