Considerations regarding shutdown timeout

[marcogramy]

Hi,
following the resolution of issues #2941 and #3032, we have conducted a further analysis concerning the shutdown process within our custom requirements.

In our custom usage scenario, we customize the ShutdownTimeout, which is used within the shutdownAndWaitOrCallback() method to allow time for processing messages after the BasicCancel before the consumers stop.

However, the commit 02db80b has introduced a modification to the basicCancel() method's behavior. The invocation of RabbitUtils.closeMessageConsumer() now triggers a sequential execution of both the Cancel and the Recover operations.

To restore the previous behavior (with a timeout on shutdown), one potential solution would involve the introduction of a distinct timeout period between the execution of the BasicCancel and Recover commands.
Or, better yet, in our opinion, the basicCancel() method should be restored to its state before the change, and the Recover command should be added after waiting for the shutdown timeout.
I am referring to the shutdownAndWaitOrCallback() method after the instruction:
boolean finished = this.cancellationLock.await(this.getShutdownTimeout(), TimeUnit.MILLISECONDS);
if the result is false.

We are unaware of the reasoning behind the aforementioned change, so we are unsure of the most appropriate approach.
We do agree that, before the change, unacknowledged messages were never requeued in the case of channel transacted, but we think that the shutdown timeout should be anyway considered before the Recover.

What are your thoughts on this?

[artembilan]

We do agree that, before the change, unacknowledged messages were never requeued in the case of channel transacted,

That's exactly why that RabbitUtils.closeMessageConsumer() was added to the basicCancel() logic: to rollback all the in-flight messages for the transactional channel on this consumer.
There is no any other reasoning: just satisfaction of the transaction semantics in RabbitMQ.

I'll think about moving channel.basicRecover(true); call already after the mention shutdown timeout.

However I still would like to understand why do you insist in using transactional channel?
There are no transactions with AMQP 1.0: https://www.rabbitmq.com/docs/next/amqp#amqp-091-features.

Have you considered to use a DirectMessageListenerContainer instead: https://docs.spring.io/spring-amqp/reference/amqp/receiving-messages/choose-container.html?

[artembilan]

I have pushed some fix according to your request.
I'd appreciate if you can test it on your side before our next release cycle on April, 22.

Thanks

[marcogramy]

Hi,
thank you for the fix!

I have successfully tested it in the demo project, confirming that the shutdown process now waits correctly. I did not observe any redelivered messages, indicating that the consumers were given sufficient time to process the in-flight messages.

We will integrate the new version into our production projects as soon as it becomes available, for further testing.

I am currently maintaining a large, decade-old project that has consistently utilized transactional channels. Before removing this configuration we need to fully understand the implications of the change. Our primary requirement is to ensure no message loss occurs.

We process several million messages daily, with traffic loads fluctuating hourly. Consequently, we maintain a dynamically varying number of consumers, leveraging the auto-scaling capabilities of the SimpleMessageListenerContainer, a feature not present in the DirectMessageListenerContainer.

[marcogramy]

Hi Artem,
we've had the opportunity to test the latest version in our production environments as well, and I can confirm that it works correctly, exhibiting the expected behavior.

Thank you once more for your support!