jbertram commented on code in PR #5291: URL: https://github.com/apache/activemq-artemis/pull/5291#discussion_r1853985284
########## artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ServerLocatorImpl.java: ########## @@ -1298,6 +1298,18 @@ public ServerLocatorImpl setInitialMessagePacketSize(final int size) { return this; } + @Override + public int getOnMessageCloseTimeout() { + return config.onMessageCloseTimeout; + } + + @Override Review Comment: I changed the JavaDoc to be more clear. I didn't want to add validation as it wouldn't be consistent with the rest of the method and adding validation to all the places that could realistically use it is well beyond the scope of this PR. ########## 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: Done. -- 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