Skip to content

GH-3031: Defer SMLC shutdown for pending replies #3147

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
artembilan merged 6 commits into from
Aug 7, 2025
Merged

GH-3031: Defer SMLC shutdown for pending replies #3147

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
368fe54
GH-3031: Defer SMLC shutdown for pending replies
m3k0813 Aug 6, 2025
1a48e51
Fix checkstyle violations
m3k0813 Aug 6, 2025
7a732db
Apply review suggestions for shutdown deferral
m3k0813 Aug 6, 2025
cc2e48c
Refactor shutdown deferral logic based on review feedback
m3k0813 Aug 7, 2025
1dd2d72
Refine test and update whats-new
m3k0813 Aug 7, 2025
00487ae
Refine shutdown deferral test for robustness
m3k0813 Aug 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions ...st/java/org/springframework/amqp/rabbit/listener/SimpleMessageListenerContainerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -736,26 +736,28 @@ void testShutdownWithPendingReplies() {

template.setReplyAddress(container.getQueueNames()[0]);

long shutdownTimeout = 2000L;
long shutdownTimeout = 500L;
container.setShutdownTimeout(shutdownTimeout);

ActiveObjectCounter<Object> replyCounter =
(ActiveObjectCounter<Object>) ReflectionTestUtils.getField(template, "pendingRepliesCounter");
ActiveObjectCounter<Object> replyCounter = template.getPendingReplyCounter();
assertThat(replyCounter).isNotNull();

Object pending = new Object();
replyCounter.add(pending);
assertThat(replyCounter.getCount()).isEqualTo(1);

Log logger = spy(TestUtils.getPropertyValue(container, "logger", Log.class));
new DirectFieldAccessor(container).setPropertyValue("logger", logger);

container.start();

long startTime = System.currentTimeMillis();
container.stop();
long stopDuration = System.currentTimeMillis() - startTime;

replyCounter.release(pending);
await().atMost(Duration.ofMillis(500)).untilAsserted(() ->
Copy link
Member

@artembilan artembilan Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think those max 30 seconds by default is fine with us in our tests.
It is really not going to be blocked that long.
Might really maximum those 500 millis.
However, if we have it as exact our given time, there is no guarantee that CPU resources will be available for us in time to satisfy this expectation.

verify(logger).warn("Shutdown timeout expired, but 1 pending replies still remain.")
);

assertThat(stopDuration).isGreaterThanOrEqualTo(shutdownTimeout - 500);
replyCounter.release(pending);
Copy link
Member

@artembilan artembilan Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this call makes any difference or sense for the test logic.
Therefore let's remove altogether to avoid extra reading noise!

}

private Answer<Object> messageToConsumer(final Channel mockChannel, final SimpleMessageListenerContainer container,
Expand Down
6 changes: 3 additions & 3 deletions src/reference/antora/modules/ROOT/pages/whats-new.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ Respective new classes have been introduced to support Jackson 3.

See xref:amqp/message-converters.adoc[] for more information.

[[x40-enhancements]]
=== Enhancements
[[x40-smlc-changes]]
=== MessageListenerContainer Changes

Defer `SimpleMessageListenerContainer` shutdown for pending `RabbitTemplate` replies.
The `SimpleMessageListenerContainer` now awaits at most `shutdownTimeout` for pending replies from the provided `RabbitTemplate` listener on its shutdown.