You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2021/01/22 17:17:16 UTC
[GitHub] [activemq-artemis] gtully opened a new pull request #3418: ARTEMIS-3064 - ensure useTopologyForLoadBalancing is disabled when ha…
gtully opened a new pull request #3418:
URL: https://github.com/apache/activemq-artemis/pull/3418
…=false
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gtully commented on a change in pull request #3418: ARTEMIS-3064 - ensure useTopologyForLoadBalancing is disabled when ha…
Posted by GitBox <gi...@apache.org>.
gtully commented on a change in pull request #3418:
URL: https://github.com/apache/activemq-artemis/pull/3418#discussion_r563843155
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/federation/FederationConnection.java
##########
@@ -69,6 +69,10 @@ public FederationConnection(Configuration configuration, String name, Federation
}
}
+ if (!config.isHA()) {
+ serverLocator.setUseTopologyForLoadBalancing(false);
Review comment:
interesting, https://issues.apache.org/jira/browse/ARTEMIS-347 introduces a cluster-connection-url in place of a connector-ref. The configuration is now a choice.
Maybe we can deprecate all of the server locator config for federation and just use a url? Or do we need to retain a choice? I think the server locator url parser is smart enough to cover all options w.r.t discovery groups etc.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3418: ARTEMIS-3064 - ensure useTopologyForLoadBalancing is disabled when ha…
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3418:
URL: https://github.com/apache/activemq-artemis/pull/3418#discussion_r563753347
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/federation/FederationConnection.java
##########
@@ -69,6 +69,10 @@ public FederationConnection(Configuration configuration, String name, Federation
}
}
+ if (!config.isHA()) {
+ serverLocator.setUseTopologyForLoadBalancing(false);
Review comment:
@gtully can you run the whole test suite and post results?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gtully commented on a change in pull request #3418: ARTEMIS-3064 - ensure useTopologyForLoadBalancing is disabled when ha…
Posted by GitBox <gi...@apache.org>.
gtully commented on a change in pull request #3418:
URL: https://github.com/apache/activemq-artemis/pull/3418#discussion_r565318390
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/federation/FederationConnection.java
##########
@@ -69,6 +69,10 @@ public FederationConnection(Configuration configuration, String name, Federation
}
}
+ if (!config.isHA()) {
+ serverLocator.setUseTopologyForLoadBalancing(false);
Review comment:
essentially a version of ARTEMIS-347 for federation
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3418: ARTEMIS-3064 - ensure useTopologyForLoadBalancing is disabled when ha…
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3418:
URL: https://github.com/apache/activemq-artemis/pull/3418#discussion_r562796867
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/federation/FederationConnection.java
##########
@@ -69,6 +69,10 @@ public FederationConnection(Configuration configuration, String name, Federation
}
}
+ if (!config.isHA()) {
+ serverLocator.setUseTopologyForLoadBalancing(false);
Review comment:
Should we maybe just like for others look to make this pick up from url configuration?
What do bridges do?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3418: ARTEMIS-3064 - ensure useTopologyForLoadBalancing is disabled when ha…
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3418:
URL: https://github.com/apache/activemq-artemis/pull/3418#discussion_r563753347
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/federation/FederationConnection.java
##########
@@ -69,6 +69,10 @@ public FederationConnection(Configuration configuration, String name, Federation
}
}
+ if (!config.isHA()) {
+ serverLocator.setUseTopologyForLoadBalancing(false);
Review comment:
@gtully can you run the whole test suite and post results?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] asfgit closed pull request #3418: ARTEMIS-3064 - ensure useTopologyForLoadBalancing is disabled when ha…
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #3418:
URL: https://github.com/apache/activemq-artemis/pull/3418
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gtully commented on a change in pull request #3418: ARTEMIS-3064 - ensure useTopologyForLoadBalancing is disabled when ha…
Posted by GitBox <gi...@apache.org>.
gtully commented on a change in pull request #3418:
URL: https://github.com/apache/activemq-artemis/pull/3418#discussion_r565318058
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/federation/FederationConnection.java
##########
@@ -69,6 +69,10 @@ public FederationConnection(Configuration configuration, String name, Federation
}
}
+ if (!config.isHA()) {
+ serverLocator.setUseTopologyForLoadBalancing(false);
Review comment:
the tests look good. I think even is we provide an alternative url config, we will need this fix for the existing configuration.
I can raise an issue to bypass the connector-refs and have a connector-uri option where the rich url has all that is needed
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gtully commented on a change in pull request #3418: ARTEMIS-3064 - ensure useTopologyForLoadBalancing is disabled when ha…
Posted by GitBox <gi...@apache.org>.
gtully commented on a change in pull request #3418:
URL: https://github.com/apache/activemq-artemis/pull/3418#discussion_r563843155
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/federation/FederationConnection.java
##########
@@ -69,6 +69,10 @@ public FederationConnection(Configuration configuration, String name, Federation
}
}
+ if (!config.isHA()) {
+ serverLocator.setUseTopologyForLoadBalancing(false);
Review comment:
interesting, https://issues.apache.org/jira/browse/ARTEMIS-347 introduces a cluster-connection-url in place of a connector-ref. The configuration is now a choice.
Maybe we can deprecate all of the server locator config for federation and just use a url? Or do we need to retain a choice? I think the server locator url parser is smart enough to cover all options w.r.t discovery groups etc.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org