Skip to content

Enhance Artemis jms auto configuration #10739

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
pax95 opened this issue Oct 21, 2017 · 14 comments
Closed

Enhance Artemis jms auto configuration #10739

pax95 opened this issue Oct 21, 2017 · 14 comments
Labels
status: superseded An issue that has been superseded by another

Comments

@pax95
Copy link

pax95 commented Oct 21, 2017

Currently you can only auto configure a single broker using the host and port options.
It seems not possible to set other cluster or ha attributes on the auto create ActiveMQConnectionFactory that is created without ha enabled.

It would be nice if you could use auto configuration to configure those options as well. Maybe expose a new property = url that could be used to construct the ActiveMQConnectionFactory

In that way you could set the url property in the configuration to e.g. spring.artemis.url=(tcp://localhost:61616,tcp://localhost:61716)?ha=true&retryInterval=1000&retryIntervalMultiplier=1.0&reconnectAttempts=-1

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

Looking at the Artemis code, it doesn't appear to be possible to construct a connection factory with a URL without losing some control over the transport configuration. Currently, we explicitly configure the use of NettyConnectorFactory. As far as I can tell, we'd lose that if we moved to using the URL-based constructor.

Unless there's a way that can please everyone, I'm leaning towards this not be supported by auto-configuration. @pax95 There's a very good chance that you know more about configuring Artermis than I do, perhaps you can guide us towards a way that will work?

@wilkinsona wilkinsona added the status: waiting-for-feedback We need additional information before we can continue label Oct 24, 2017
@pax95
Copy link
Author

pax95 commented Oct 24, 2017

It seems that the transport configuration actually makes use of NettyConnectorFactory when constructing the connection factory from a URL.

Small sample with a Camel route:

    @ConfigurationProperties(prefix = "spring.artemis")
    public class AmqConfig {
        private String url;

        @Bean("connectionFactory")
        public ConnectionFactory connectionFactory() {
            ActiveMQConnectionFactory cf = new ActiveMQConnectionFactory(this.url);
            LOG.info("cf info {}", cf);
            return cf;
        }

        public void setUrl(String url) {
            this.url = url;
        }

    }

    @Component
    class MyRoute extends RouteBuilder {

        @Override
        public void configure() throws Exception {
            from("jms:foo?connectionFactory=#connectionFactory").transacted().log("log recieved");

            from("timer:bar").setBody(constant("the message")).transacted().log("sending").to("jms:foo?connectionFactory=#connectionFactory");
        }

    }

The log statement when setting the property
spring.artemis.url=(tcp://localhost:61616,tcp://localhost:61716)?ha=true&retryInterval=1000&retryIntervalMultiplier=1.0&reconnectAttempts=-1

ActivemqTestApplication     : cf info ActiveMQConnectionFactory [serverLocator=ServerLocatorImpl [initialConnectors=[TransportConfiguration(name=null, factory=org-apache-activemq-artemis-core-remoting-impl-netty-NettyConnectorFactory) ?port=61616&host=localhost&retryIntervalMultiplier=1-0&reconnectAttempts=-1&ha=true&retryInterval=1000, TransportConfiguration(name=null:tcp://localhost:61716, factory=org-apache-activemq-artemis-core-remoting-impl-netty-NettyConnectorFactory) ?port=61716&host=localhost&retryIntervalMultiplier=1-0&reconnectAttempts=-1&ha=true&retryInterval=1000], discoveryGroupConfiguration=null], clientID=null, consumerWindowSize = 1048576, dupsOKBatchSize=1048576, transactionBatchSize=1048576, readOnly=false]

I will push the sample to GitHub later.

@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 Oct 24, 2017
@pax95
Copy link
Author

pax95 commented Oct 24, 2017

@wilkinsona Pushed a small example here https://github.com/pax95/activemq-test
The example uses just a single broker to make et easy to setup, but the url constructor seems flexible to allow cluster, ha and other properties to be embedded.

@wilkinsona
Copy link
Member

A url property is going to get a bit messy when coupled with the existing host and port properties. We have that elsewhere though and have managed to deal with it.

@wilkinsona wilkinsona added priority: low type: enhancement A general enhancement and removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged labels Oct 27, 2017
@pax95
Copy link
Author

pax95 commented Oct 27, 2017

agreed. Thats why I wanted to hear your opinion. Btw the Activemq auto configuration actually has a brokerUrl property too.

@chanjarster
Copy link

@pax95 Agree with you.

@wilkinsona I think the url version is more flexible then the host:port version. considering the following artemis url:

tcp://localhost:61616?type=CF\
  &useGlobalPools=false\
  &threadPoolMaxSize=10\
  &retryInterval=2000\
  &retryIntervalMultiplier=1.5\
  &maxRetryInterval=30000\
  &reconnectAttempts=-1

The url version gives you more control.

BTW, I implemented a customized ArtemisConfiguration by using ConnectionFactoryParser:

import org.apache.activemq.artemis.uri.ConnectionFactoryParser;

...

public ConnectionFactory connectionFactory() throws Exception {
  ConnectionFactoryParser parser = new ConnectionFactoryParser();
  return parser.newObject(parser.expandURI(artemisProperties.getUri()), "someCFConfig");
}

@philwebb philwebb changed the title enhance Artemis jms auto configuration Enhance Artemis jms auto configuration May 31, 2019
@philwebb philwebb added this to the General Backlog milestone May 31, 2019
@jbertram
Copy link
Contributor

As a developer on ActiveMQ Artemis I would recommend using a URL instead host:port. It's easy to construct an Artemis connection factory using a URL. Just use this constructor - org.apache.activemq.artemis.jms.client.ActiveMQConnectionFactory#ActiveMQConnectionFactory(java.lang.String). The URL will be much more flexible than with just host:port as you'll be able to implicitly support all relevant connection settings (as @chanjarster already demonstrated).

@snicoll
Copy link
Member

snicoll commented Sep 15, 2020

Thanks for the follow-up @jbertram. What would be your recommendation regarding what @wilkinsona pointed above. We may need to tune TransportConfiguration.

@jbertram
Copy link
Contributor

The URL replaces manual configuration of the TransportConfiguration. I haven't looked at the relevant code, but my guess is that it should treat it as an either/or. If the URL is configured use that, if not then use the host:port. FWIW, everything available with manual TransportConfiguration should be available with a URL.

@criew
Copy link

criew commented Nov 19, 2020

Same issue on our project.
We used ActiveMQ before and now cannot use Autoconfiguration for Artemis because it does not allow brokerUrl to be configured.
All TransportConfiguration would be available in our case, so it would be really nice if you can add the brokerUrl property.

@kmandalas
Copy link

Really interested in this feature as well.

@LStampf
Copy link

LStampf commented Nov 30, 2020

I am interested as well. It's easy to setup a ConnectionFactory like @jbertram specified, but it would be nice to have the option.

jbertram added a commit to jbertram/spring-boot that referenced this issue Nov 30, 2020
jbertram added a commit to jbertram/spring-boot that referenced this issue Nov 30, 2020
jbertram added a commit to jbertram/spring-boot that referenced this issue Nov 30, 2020
jbertram added a commit to jbertram/spring-boot that referenced this issue Nov 30, 2020
jbertram added a commit to jbertram/spring-boot that referenced this issue Nov 30, 2020
@jbertram
Copy link
Contributor

FYI, I just sent #24302 to implement this functionality.

jbertram added a commit to jbertram/spring-boot that referenced this issue Nov 30, 2020
jbertram added a commit to jbertram/spring-boot that referenced this issue Dec 1, 2020
@philwebb philwebb added status: superseded An issue that has been superseded by another and removed type: enhancement A general enhancement labels Dec 2, 2020
@philwebb philwebb removed this from the General Backlog milestone Dec 2, 2020
@philwebb
Copy link
Member

philwebb commented Dec 2, 2020

Closing in favor of PR #24302

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: superseded An issue that has been superseded by another
Projects
None yet
Development

No branches or pull requests

10 participants