Skip to content
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.

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

Doc: Clarify AUTO_ACKNOWLEDGE semantics with SimpleMessageListenerContainer [SPR-13278] #17869

Closed
spring-projects-issues opened this issue Jul 24, 2015 · 5 comments
Assignees
Labels
in: messaging status: backported type: task
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Jul 24, 2015

Lubos Krnac opened SPR-13278 and commented

Re-delivery of message when exception happen for combination AUTO_ACKNOWLEDGE and SimpleJmsListenerContainerFactory doesn't work since 4.1.7.RELEASE.

To reproduce I submitted spring-framework-issues sample project. Test expects duplicate message to be stored in DB, because app simulates error for first received message.

Behavior is different for Spring Boot 1.2.5.RELEASE in comparison to 1.2.4.RELEASE.


Affects: 3.2.14, 4.1.7

Reference URL: spring-attic/spring-framework-issues#100

Issue Links:

  • #16631 AbstractMessageListenerContainer#doExecuteListener can cause a dropped message if using CLIENT_ACKNOWLEDGE and the container is stopped and subsequently started again.
  • #17644 Remoting over JMS with receiveTimeout blocks service forever

Backported to: 4.1.8, 3.2.15

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jul 25, 2015

Juergen Hoeller commented

This is a consequence of the fix for #17644, since we only call Session.recover() in case of CLIENT_ACKNOWLEDGE now. Note that we did not call recover() before #16631 at all, so what you're relying on is part of a rather recent initiative to make non-transacted JMS more reliable - on a best effort basis, with no strong guarantees.

From a common JMS perspective, Session.recover() is only really meant to be used with CLIENT_ACKNOWLEDGE, so the current behavior seems to be a fine compromise. In the case of AUTO_ACKNOWLEDGE, the JMS broker itself is responsible for redelivery decisions; I'm surprised that you're not seeing redelivery since that's what most brokers do.

So from that perspective, you've been relying on non-guaranteed behavior in AUTO mode before. My recommendation would be to switch to CLIENT_ACKNOWLEDGE if you are expecting best-effort recovery behavior there. For a higher degree of redelivery guarantees, you might want to switch to the traditional sessionTransacted=true instead.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jul 25, 2015

Lubos Krnac commented

I see, nice explanation.

I'm surprised that you're not seeing redelivery since that's what most brokers do.

I was trying it with HornetQ and ActiveMQ. Same behavior. It's because AbstractMessageListenerContainer.executeListener(Session session, Message message) doesn't bubble up exception to JMS listener library when error handler isn't defined. That is why I reopened the issue.

  1. Does that mean this JMS configuration is designed to work in JMS compliant way only when error handler is defined and re-throws exception? Maybe I missed this fact in reference/java docs.
  2. Is it intentional to allow user to control bubbling up exceptions in error handler for JMS module?
  3. Wouldn't be a good idea to re-throw exception in AbstractMessageListenerContainer.invokeErrorHandler(Throwable ex) if default error handler is not defined? I understand that other ack/transacted modes should be considered for such change. But it would help to behave in JMS compliant way in such configuration.(Would you consider such pull request?)

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jul 25, 2015

Stéphane Nicoll commented

A sample project is available here

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jul 26, 2015

Juergen Hoeller commented

I am afraid I was mistaken above: JMS brokers do not provide consistent redelivery semantics for MessageListener exceptions; they may even consider this an application error and stop delivery of messages to that listener altogether. The following blog post summarizes the situation pretty well: http://effectivemessaging.blogspot.co.at/2009/01/messagelistener-exception-handling.html

This is actually why we opted to not propagate exceptions to the broker in SimpleMessageListenerContainer. This also brings it in sync with DefaultMessageListenerContainer where such propagation isn't even possible. However, our documentation suggests that SMLC may provide redelivery in AUTO mode while DMLC doesn't; this needs to be rephrased to clarify that exception handling is effectively equivalent. The only difference is what happens when the JVM dies during listener execution: With SMLC, messages are likely to get redelivered in such a scenario; with DMLC, they definitely won't get redelivered since they have been acknowledged before delegating to the listener.

As a consequence, I'm turning this into a documentation task. The current runtime behavior is correct from my perspective, both from our intentions and also following the JMS spec, and just needs to be clearly stated. To get the desired effect of consistent redelivery, you'll have to use CLIENT_ACKNOWLEDGE or sessionTransacted=true.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jul 26, 2015

Lubos Krnac commented

I was also mistaken and though that throwing runtime exception from message listener is valid scenario. JEE 7 Tutorial clearly states that it's not true:

Your onMessage method should handle all exceptions. Throwing a RuntimeException is considered a programming error.

Sorry for confusion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: messaging status: backported type: task
Projects
None yet
Development

No branches or pull requests

2 participants