You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by franz1981 <gi...@git.apache.org> on 2018/10/01 07:49:37 UTC
[GitHub] activemq-artemis pull request #2338: ARTEMIS-2101 Do not cluster OpenWire ad...
GitHub user franz1981 opened a pull request:
https://github.com/apache/activemq-artemis/pull/2338
ARTEMIS-2101 Do not cluster OpenWire advisory topics
ClusterConnectionBridge needs to ignore any advisory topic
address in the filter used by the notification consumer on
bridge activation
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/franz1981/activemq-artemis ARTEMIS-2101
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/2338.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 #2338
----
commit 8af35fdb3ad6ee045a27d570ccf3b748b6e9ae42
Author: Francesco Nigro <ni...@...>
Date: 2018-09-28T17:32:51Z
ARTEMIS-2101 Do not cluster OpenWire advisory topics
ClusterConnectionBridge needs to ignore any advisory topic
address in the filter used by the notification consumer on
bridge activation
----
---
[GitHub] activemq-artemis issue #2338: ARTEMIS-2101 Do not cluster OpenWire advisory ...
Posted by jbertram <gi...@git.apache.org>.
Github user jbertram commented on the issue:
https://github.com/apache/activemq-artemis/pull/2338
Perhaps the OpenWireProtocolManager could change the filter of the cluster connection when it's started or it could register an ActivateCallback and do the changes there.
---
[GitHub] activemq-artemis pull request #2338: ARTEMIS-2101 Do not cluster OpenWire ad...
Posted by franz1981 <gi...@git.apache.org>.
Github user franz1981 closed the pull request at:
https://github.com/apache/activemq-artemis/pull/2338
---
[GitHub] activemq-artemis pull request #2338: ARTEMIS-2101 Do not cluster OpenWire ad...
Posted by franz1981 <gi...@git.apache.org>.
Github user franz1981 commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2338#discussion_r222008074
--- Diff: artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/OpenWireProtocolManager.java ---
@@ -154,6 +154,11 @@ public OpenWireProtocolManager(OpenWireProtocolManagerFactory factory, ActiveMQS
if (cc != null) {
cc.addClusterTopologyListener(this);
}
+ if (supportAdvisory) {
--- End diff --
I'm getting a failure with the last changes...
---
[GitHub] activemq-artemis pull request #2338: ARTEMIS-2101 Do not cluster OpenWire ad...
Posted by franz1981 <gi...@git.apache.org>.
Github user franz1981 commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2338#discussion_r221536659
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/cluster/impl/ClusterConnectionBridge.java ---
@@ -350,6 +352,8 @@ private String appendIgnoresToFilter(String filterString) {
filterString += "!" + storeAndForwardPrefix;
filterString += ",!" + managementAddress;
filterString += ",!" + managementNotificationAddress;
+ //advisory topics shouldn't be clustered
+ filterString += ",!" + ADVISORY_TOPIC_PREFIX;
--- End diff --
The best wat I could think is to inject it as a constructor parameter similar to managementNotificationAddress; but advisory topics are just an OpenWire specific feature anyway
---
[GitHub] activemq-artemis pull request #2338: ARTEMIS-2101 Do not cluster OpenWire ad...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2338#discussion_r221985696
--- Diff: artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/OpenWireProtocolManager.java ---
@@ -154,6 +154,11 @@ public OpenWireProtocolManager(OpenWireProtocolManagerFactory factory, ActiveMQS
if (cc != null) {
cc.addClusterTopologyListener(this);
}
+ if (supportAdvisory) {
--- End diff --
I wouldn't make it dependent on supportAdvisory.
Say you have two acceptors.. different IPs.. you will have this called twice.
I would always add the filter, and check if the filter is not part of the expression before you called it.
---
[GitHub] activemq-artemis issue #2338: ARTEMIS-2101 Do not cluster OpenWire advisory ...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/2338
Point here is this could be something on the address that could be made much more generic. We shouldnt do anything thats protocol specific in core. We should find a generic way
---
[GitHub] activemq-artemis issue #2338: ARTEMIS-2101 Do not cluster OpenWire advisory ...
Posted by franz1981 <gi...@git.apache.org>.
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/2338
@michaelandrepearce @jbertram @clebertsuconic I have made it more generic and less OpenWire bounded, but I'm not quite 100% sure about it so please, fi you have any comments or worries share them :+1:
---
[GitHub] activemq-artemis issue #2338: ARTEMIS-2101 Do not cluster OpenWire advisory ...
Posted by franz1981 <gi...@git.apache.org>.
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/2338
Thanks guys: I'm working on this to improve the decoupling of the solution from being OpenWire-centric :+1:
---
[GitHub] activemq-artemis pull request #2338: ARTEMIS-2101 Do not cluster OpenWire ad...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2338#discussion_r221534313
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/cluster/impl/ClusterConnectionBridge.java ---
@@ -350,6 +352,8 @@ private String appendIgnoresToFilter(String filterString) {
filterString += "!" + storeAndForwardPrefix;
filterString += ",!" + managementAddress;
filterString += ",!" + managementNotificationAddress;
+ //advisory topics shouldn't be clustered
+ filterString += ",!" + ADVISORY_TOPIC_PREFIX;
--- End diff --
This is Openwire specific, should avoid pushing protocol specifics into core.
What happens it another protocol needs an address not clustered?
Would be better to design this in an agnostic way, eg a flag on the address or queue, to not cluster bridge, which the protocol managers can set on creating
---
[GitHub] activemq-artemis issue #2338: ARTEMIS-2101 Do not cluster OpenWire advisory ...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/2338
@jbertram ++
Add a method on ProtocolManager, changeFilter, or changeClusterManager. run it on all the protocols to that.
For example that will be an issue with WildFly as they don't have OpenWire.
---