[
https://issues.apache.org/jira/browse/ARTEMIS-5093?focusedWorklogId=945512&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-945512
]
ASF GitHub Bot logged work on ARTEMIS-5093:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 25/Nov/24 16:59
Start Date: 25/Nov/24 16:59
Worklog Time Spent: 10m
Work Description: gemmellr commented on code in PR #5291:
URL: https://github.com/apache/activemq-artemis/pull/5291#discussion_r1856960550
##########
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:
You stopped the thread hanging around much longer than the checks, which is
good...however the checks still remain as brittle to scheduling as they were
before the change.
The main point is surely that it did not exit during the close timeout, and
that the close takes at least the timeout. So you control the handler thread
cant exit while those checks are happening, but then exits immediately as soon
as they have completed, once you allow it to. The 800ms magic window doesnt
need to be there.
Is it likely to take that long? No. Though could it happen now and then, on
e.g a slow CI env with low thread count and shared resources? Yes, when someone
then needs to go look at the sporadic failure. Or it adds to the many that have
sporadically failed and encouraged people not to look.
If you want to use a timeout in the handler that fair enough, but I'd make
it a lot longer so it can never reasonably happen unless the test has hung
elsewhere.
It also needs add a try-finally to ensure the gating boolean is tripped to
have it exit immediately even if the asserts somehow failed.
Issue Time Tracking
-------------------
Worklog Id: (was: 945512)
Time Spent: 3h (was: 2h 50m)
> Support configurable onMessage timeout when closing consumer
> ------------------------------------------------------------
>
> Key: ARTEMIS-5093
> URL: https://issues.apache.org/jira/browse/ARTEMIS-5093
> Project: ActiveMQ Artemis
> Issue Type: Improvement
> Reporter: Justin Bertram
> Assignee: Justin Bertram
> Priority: Major
> Labels: pull-request-available
> Time Spent: 3h
> Remaining Estimate: 0h
>
> When closing a consumer the Core client is hard-coded to wait for 10,000ms
> for any {{onMessage}} to complete. This timeout should be configurable.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
