You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by mvtorres <gi...@git.apache.org> on 2018/08/31 19:47:19 UTC
[GitHub] activemq-artemis pull request #2286: Support useTopologyForLoadBalancing on ...
GitHub user mvtorres opened a pull request:
https://github.com/apache/activemq-artemis/pull/2286
Support useTopologyForLoadBalancing on JMS ConnectionFactory urls
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mvtorres/activemq-artemis master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/2286.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2286
----
commit dbd588ecf869fb6b782f8551facba8ffdf06312d
Author: Mark Torres <mt...@...>
Date: 2018-08-31T19:19:51Z
Support useTopologyForLoadBalancing on JMS ConnectionFactory urls
----
---
[GitHub] activemq-artemis issue #2286: Support useTopologyForLoadBalancing on JMS Con...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/2286
Please look at #2292
---
[GitHub] activemq-artemis issue #2286: Support useTopologyForLoadBalancing on JMS Con...
Posted by jbertram <gi...@git.apache.org>.
Github user jbertram commented on the issue:
https://github.com/apache/activemq-artemis/pull/2286
OK. I see now where this isn't being passed through when used for a JMS connection factory, although using the property in the URL for a `ServerLocator` directly does work. You should add a test to SimpleJNDIClientTest, e.g.:
```
@Test
public void testUseTopologyForLoadBalancing() throws NamingException {
Hashtable<String, String> props = new Hashtable<>();
props.put(Context.INITIAL_CONTEXT_FACTORY, "org.apache.activemq.artemis.jndi.ActiveMQInitialContextFactory");
props.put("connectionFactory.ConnectionFactory", "vm://0?useTopologyForLoadBalancing=false");
Context ctx = new InitialContext(props);
ConnectionFactory connectionFactory = (ConnectionFactory) ctx.lookup("ConnectionFactory");
assertFalse(((ActiveMQConnectionFactory)connectionFactory).getServerLocator().getUseTopologyForLoadBalancing());
}
```
---
[GitHub] activemq-artemis issue #2286: Support useTopologyForLoadBalancing on JMS Con...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/2286
I usually just use the IDE to run tests...
But you probably need to build the project before running the test?
---
[GitHub] activemq-artemis issue #2286: Support useTopologyForLoadBalancing on JMS Con...
Posted by mvtorres <gi...@git.apache.org>.
Github user mvtorres commented on the issue:
https://github.com/apache/activemq-artemis/pull/2286
> Why don't you just use cf = new ActiveMQConnectionFActory(uri); ?
The existing code has the ConnectionFactory lookup via jndi, we're migrating from a different JMS provider(JBoss Messaging).
---
[GitHub] activemq-artemis issue #2286: Support useTopologyForLoadBalancing on JMS Con...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/2286
anyway.. if you add the property.. also add the getter.. as the externalize will it.
---
[GitHub] activemq-artemis issue #2286: Support useTopologyForLoadBalancing on JMS Con...
Posted by mvtorres <gi...@git.apache.org>.
Github user mvtorres commented on the issue:
https://github.com/apache/activemq-artemis/pull/2286
This is an existing feature, I just exposed it so its usable on urls of ConnectionFactory.
Existing documentation is at https://activemq.apache.org/artemis/docs/latest/clusters.html#client-side-load-balancing
Im trying to add a testcase/logic to org.apache.activemq.artemis.tests.integration.cluster.distribution.NettySymmetricClusterTest
which was added by the original pull request
https://github.com/apache/activemq-artemis/commit/b6b8fa411f4dcf9e6ac8248489579dac8d931cb3
but Im having trouble getting that to execute locally.
When I run
`mvn -Ptests -DfailIfNoTests=false -Dtest=NettySymmetricClusterTest test`
I'm getting
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 56.893 s
[INFO] Finished at: 2018-09-04T12:54:56-04:00
[INFO] Final Memory: 127M/1567M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.1:testCompile (default-testCompile) on project unit-tests: Compilation failure: Compilation failure:
[ERROR] /Users/mark/git/activemq-artemis/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/ra/ResourceAdapterTest.java:[62,81] cannot access ActiveMQConnectionFactory
[ERROR] class file for ActiveMQConnectionFactory not found
[ERROR] /Users/mark/git/activemq-artemis/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/ra/ResourceAdapterTest.java:[93,81] incompatible types: ActiveMQConnectionFactory cannot be converted to org.apache.activemq.artemis.jms.client.ActiveMQConnectionFactory
[ERROR] /Users/mark/git/activemq-artemis/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/ra/ResourceAdapterTest.java:[94,82] incompatible types: ActiveMQConnectionFactory cannot be converted to org.apache.activemq.artemis.jms.client.ActiveMQConnectionFactory
Can you point me to the right direction...
Thanks
---
[GitHub] activemq-artemis issue #2286: Support useTopologyForLoadBalancing on JMS Con...
Posted by mvtorres <gi...@git.apache.org>.
Github user mvtorres commented on the issue:
https://github.com/apache/activemq-artemis/pull/2286
This is the config, our app is currently running on jboss 4.3.
<mbean code="org.jboss.naming.ExternalContext"
name="DefaultDomain:service=ExternalContext,jndiName=RemoteJMSJNDI">
<attribute name="JndiName">RemoteJMSJNDI</attribute>
<attribute name="Properties">
java.naming.factory.initial=org.apache.activemq.artemis.jndi.ActiveMQInitialContextFactory
java.naming.provider.url=tcp://localhost:61616?useTopologyForLoadBalancing=false
topic.topic/eventTopic=eventTopic
...
</attribute>
<attribute name="InitialContext">javax.naming.InitialContext</attribute>
</mbean>
Artemis is running inside docker, with the connector dynamically set to the docker container ip.
---
[GitHub] activemq-artemis issue #2286: Support useTopologyForLoadBalancing on JMS Con...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/2286
Why don't you just use cf = new ActiveMQConnectionFActory(uri); ?
---
[GitHub] activemq-artemis issue #2286: Support useTopologyForLoadBalancing on JMS Con...
Posted by mvtorres <gi...@git.apache.org>.
Github user mvtorres commented on the issue:
https://github.com/apache/activemq-artemis/pull/2286
Thanks for the fix.
---
[GitHub] activemq-artemis issue #2286: Support useTopologyForLoadBalancing on JMS Con...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/2286
@jbertram I think the right fix is on the Schemas...
```java
@Override
protected ActiveMQConnectionFactory internalNewObject(URI uri,
Map<String, String> query,
String name) throws Exception {
JMSConnectionOptions options = newConectionOptions(uri, query);
ActiveMQConnectionFactory factory = ActiveMQJMSClient.createConnectionFactoryWithoutHA(options.getFactoryTypeEnum(), InVMTransportConfigurationSchema.createTransportConfiguration(uri, query, name, "org.apache.activemq.artemis.core.remoting.impl.invm.InVMConnectorFactory"));
BeanSupport.setData(uri, factory, query);
BeanSupport.setData(uri, factory.getServerLocator(), query);
return factory;
}
```
---
[GitHub] activemq-artemis issue #2286: Support useTopologyForLoadBalancing on JMS Con...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/2286
TBH: This currently works already. you don't need to expose the property.
The URI is used to create the serverLocator, and it will be set on it already.
---
[GitHub] activemq-artemis pull request #2286: Support useTopologyForLoadBalancing on ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/2286
---
[GitHub] activemq-artemis issue #2286: Support useTopologyForLoadBalancing on JMS Con...
Posted by jbertram <gi...@git.apache.org>.
Github user jbertram commented on the issue:
https://github.com/apache/activemq-artemis/pull/2286
@mvtorres, how is JNDI being configured?
---
[GitHub] activemq-artemis issue #2286: Support useTopologyForLoadBalancing on JMS Con...
Posted by mvtorres <gi...@git.apache.org>.
Github user mvtorres commented on the issue:
https://github.com/apache/activemq-artemis/pull/2286
When I was running on debug mode, it was actually hitting
org.apache.activemq.artemis.jms.client.ActiveMQConnectionFactory.ActiveMQConnectionFactory(boolean, TransportConfiguration...)
instead of
org.apache.activemq.artemis.jms.client.ActiveMQConnectionFactory.ActiveMQConnectionFactory(ServerLocator)
so useTopologyForLoadBalancing was not being set on the ServerLocator.
I was using the following to get the ConnectionFactory
`
Properties p = new Properties(); p.put("java.naming.factory.initial","org.apache.activemq.artemis.jndi.ActiveMQInitialContextFactory");
p.put("connectionFactory.ConnectionFactory","tcp://localhost:61616?useTopologyForLoadBalancing=false");
ConnectionFactory cf = (ConnectionFactory) initialContext.lookup("ConnectionFactory");
`
---
[GitHub] activemq-artemis issue #2286: Support useTopologyForLoadBalancing on JMS Con...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/2286
Test cases needed to cover new feature, also documentation needed to describe the feature for others
---