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

Add support for auto-configuring SimpleMessageListenerContainer #44465

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vpavic
Copy link
Contributor

@vpavic vpavic commented Feb 27, 2025

This commit introduces a new spring.jms.listener.container-type configuration property which can be used to auto-configure JMS listener support backend by SimpleMessageListenerContainer instead of DefaultMessageListenerContainer.

@snicoll
Copy link
Member

snicoll commented Feb 28, 2025

Thanks for the suggestion but we don't really consider a promotion of SimpleJmsListenerContainer to that level. We invest in DefaultMessageListenerContainer for many years and I'd rather not confuse users by giving them a choice.

What's your use of SimpleJmsListenerContainer and why can't you use DefaultMessageListenerContainer.

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Feb 28, 2025
@vpavic
Copy link
Contributor Author

vpavic commented Feb 28, 2025

Semantics of SimpleMessageListenerContainer are quite different from polling-based approach that DefaultMessageListenerContainer are a better fit for some use cases. Your comment sounds like use of SimpleMessageListenerContainer is discouraged and I don't see that being the case looking at Message Listener Containers section of the Spring Framework reference docs that says this about SimpleMessageListenerContainer:

This message listener container is the simpler of the two standard flavors. It creates a fixed number of JMS sessions and consumers at startup, registers the listener by using the standard JMS MessageConsumer.setMessageListener() method, and leaves it up the JMS provider to perform listener callbacks. This variant does not allow for dynamic adaption to runtime demands or for participation in externally managed transactions. Compatibility-wise, it stays very close to the spirit of the standalone JMS specification, but is generally not compatible with Jakarta EE’s JMS restrictions.

My specific use case is that I'm using SQS and Amazon SQS Java Messaging Library provided by AWS. This JMS implementation doesn't support transactions, but provides custom acknowledge mode that better fits SQS, and internally has its own prefetching/polling mechanism that aligns usage of SQS API with JMS spec. From my experience SimpleMessageListenerContainer is a better fit here since I don't want another layer of polling (and associated set of threads) and dynamic concurrency.

I'd like to avoid copying boilerplate configuration from project to project and use configuration property based approach to opt into using SimpleMessageListenerContainer while also retaining the ability to configure other message listener aspects using properties.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Feb 28, 2025
@snicoll snicoll changed the title Add support for auto-configuring SimpleJmsListenerContainer Add support for auto-configuring SimpleMessageListenerContainer Feb 28, 2025
@vpavic vpavic force-pushed the simple-jms-listener-container branch 2 times, most recently from 5ced34f to c71222e Compare February 28, 2025 18:37
This commit introduces a new `spring.jms.listener.container-type`
configuration property which can be used to auto-configure JMS listener
support backend by `SimpleMessageListenerContainer` instead of
`DefaultMessageListenerContainer`.

Signed-off-by: Vedran Pavic <[email protected]>
@vpavic vpavic force-pushed the simple-jms-listener-container branch from c71222e to 2c4eedc Compare March 10, 2025 08:50
@snicoll
Copy link
Member

snicoll commented Mar 12, 2025

Your comment sounds like use of SimpleMessageListenerContainer is discouraged

No, it doesn't sound like that. If it was, the class would be deprecated. I wrote "invest", which means we haven't been touching the simple implementation and recommend folks to use the pooling approach. I think we can move forward but I would make the following changes to make the PR actionable:

  • Remove the property that switches from one to another. If you want to use the simple implementation, then it's fair to ask folks to create a dedicated factor (potentially replacing the default if they want to switch everything over it). In particular, I don't like the update to configuration properties where half of the properties are now working with one, but not the other
  • Update the documentation to explain the difference between the two implementations and why the Simple implementation would be a good fit. I don't want to offer a new option without a justification. Right now, the change is bringing more confusion to regular users.

We can reconsider based on the above, thanks!

@snicoll snicoll added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Mar 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting-for-feedback We need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants