Skip to content

Use native connection factory with JMS message listener containers #39816

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

Closed
jschmied opened this issue Mar 1, 2024 · 8 comments
Closed

Use native connection factory with JMS message listener containers #39816

jschmied opened this issue Mar 1, 2024 · 8 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@jschmied
Copy link

jschmied commented Mar 1, 2024

artemis 2.19 / spring boot 2.7.8

I searched why a native broker connection does not recover fast after restarting broker, but takes 10 min to recover.

I found

spring.jms.cache.enable=false solves the problem

switching it off by property is no option since I use JMS template in same component.

Javadoc writes:

Note: Don't use Spring's CachingConnectionFactory in combination with dynamic scaling. Ideally, don't use it with a message listener container at all, since it is generally preferable to let the listener container itself handle appropriate caching within its lifecycle. Also, stopping and restarting a listener container will only work with an independent, locally cached Connection - not with an externally cached one.

I assume autoconfiguration shoud use unwrapped connection factory for listener container.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Mar 1, 2024
@wilkinsona
Copy link
Member

OSS support for Spring Boot 2.7.x has ended so I'm afraid this won't change in Spring Boot 2.x.

If you want to use a caching connection factory with your JMS template but not with your message listener container, you could define either the template or the listener container yourself. The latter is probably the easier of the two:

@Bean
DefaultJmsListenerContainerFactory jmsListenerContainerFactory(
		DefaultJmsListenerContainerFactoryConfigurer configurer, ConnectionFactory connectionFactory) {
	DefaultJmsListenerContainerFactory factory = new DefaultJmsListenerContainerFactory();
	configurer.configure(factory, ((CachingConnectionFactory)connectionFactory).getTargetConnectionFactory());
	return factory;
}

Spring Boot 3.x uses Spring Framework 6.x where the javadoc has been amended and the note softened quite a bit. That said, it still leans towards not using a CachingConnectionFactory with a listener container.

@jhoeller, what would you recommend here for Boot 3? I doubt that we'd change this in a maintenance release but we could consider unwrapping any caching connection factory in a new minor.

@wilkinsona wilkinsona added the status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team label Mar 1, 2024
@jschmied
Copy link
Author

jschmied commented Mar 1, 2024

Yes, unwrapping is easy. It just breaks reconnecting after broker restart badly.

@jschmied
Copy link
Author

jschmied commented Mar 2, 2024

I just reviewed the javadoc change. The change of documentation relating to dynamic scaling might be right. What should be stressed: the CachingConnectionFactory breaks (might be depending on type underlying ConnectionFactory) recovering from broker outage. This is clearly reproducable. In our case it affected the system badly leading to outage of our production system and unessesary downtime affecting our customers.

@jhoeller
Copy link

For listener recovery, it is indeed recommendable to configure the message listener containers with the target ConnectionFactory directly, letting them use one Connection per listener container. It's a tradeoff with the number of Connections in the overall application which should be fine in many scenarios, giving full responsibility to each listener container in terms of local recovery.

With a high number of listener containers, I'd consider Connection reuse in the setup but even that does not have to be a global CachingConnectionFactory, could also be a SingleConnectionFactory shared among the listener containers - whereas for JmsTemplate usage, there could still be a separate CachingConnectionFactory for template operations only.

Customizing the DefaultJmsListenerContainerFactory should do the trick for the time being. For Boot 3.3, we could consider a revised setup strategy by default indeed.

@philwebb philwebb added the for: team-meeting An issue we'd like to discuss as a team to make progress label Apr 22, 2024
@philwebb philwebb removed the for: team-meeting An issue we'd like to discuss as a team to make progress label Jul 10, 2024
@snicoll snicoll added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team labels Jul 12, 2024
@snicoll snicoll added this to the 3.4.x milestone Jul 12, 2024
@snicoll
Copy link
Member

snicoll commented Jul 12, 2024

Brainstorming with Juergen, we need to make sure to configure the DMLC with the target/native ConnectionFactory rather than a caching ConnectionFactory that is more suited to JmsTemplate. We have two candidates for wrapping the native broker connection factory: CachingConnectionFactory and JmsPoolConnectionFactory.

The easiest would be to be opinionated in our own auto-configuration, extracting the target ConnectionFactory. However, since users may want to create their own DefaultJmsListenerContainerFactory we should provide an API that does the unwrap so that they don't have to do that themselves.

@snicoll snicoll modified the milestones: 3.4.x, 3.4.0-M1 Jul 17, 2024
@wilkinsona wilkinsona changed the title DefaultMessageListenerContainer should not been autoconfigured with CachingConnectionFactory Use native connection factory with JMS message listener containers Jul 18, 2024
@graben

This comment was marked as resolved.

@snicoll

This comment was marked as resolved.

@graben

This comment was marked as resolved.

snicoll added a commit that referenced this issue Nov 26, 2024
This commit refines the optimization introduced in gh-39816 to only
unwrap our own caching connection factory. The more advanced unwrap
algorithm is still available, but opt-in only.

Unwrapping more aggressively may break use cases where the wrapped
ConnectionFactory is required, i.e. for transactional purposes.

Closes gh-43277
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

7 participants