You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by lulf <gi...@git.apache.org> on 2016/08/18 13:54:54 UTC
[GitHub] activemq-artemis pull request #726: Ignore jgroup messages coming from itsel...
GitHub user lulf opened a pull request:
https://github.com/apache/activemq-artemis/pull/726
Ignore jgroup messages coming from itself
In our broker.xml we have a broadcast group, a discovery group and a core bridge referring to the discovery group to discover other brokers. It appears, however, that the discovery mechanism discovers itself even when only 1 broker is running.
In DiscoveryGroup.java, it compares the node id of the broadcaster with the node id of the discoverygroup, but these id's appears to be generated randomly for both the broadcast and discovery group. The proper fix might be to ensure that the node id is the same for both of these, but I am not sure if it is intentional that they don't have the same id or not.
This PR is an attempted fix of an issue at the jgroups level, by ignoring jgroups messages coming from oneself. Feel free to close the PR if this should be fixed elsewhere.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/lulf/activemq-artemis lulf/ignore-jgroups-payload-if-self
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/726.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 #726
----
commit 92713b77cb410a69577f860d0ef5405b73c213ed
Author: Ulf Lilleengen <lu...@redhat.com>
Date: 2016-08-18T13:46:09Z
Ignore jgroup messages coming from itself
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #726: Don't send jgroup messages to self
Posted by lulf <gi...@git.apache.org>.
Github user lulf commented on the issue:
https://github.com/apache/activemq-artemis/pull/726
@belaban It looks that discovery and broadcast groups create different channels that will be given different addresses rather using the same channel. It actually looks like it is running two jgroup instances. For some reason, the broadcast group will send using the address of the other channel and then suddenly switch...
If I modify the JChannelManager channel pool to be global, so that the channel is only created once, everything works as expected. Then the DONT_LOOPBACK setting also work as expected.
I'm using jgroups-kubernetes KUBE_PING.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #726: Don't send jgroup messages to self
Posted by lulf <gi...@git.apache.org>.
Github user lulf commented on the issue:
https://github.com/apache/activemq-artemis/pull/726
@belaban Actually, now I see that my original fix doesn't work either. From debugging I see that it ignores messages sent by itself, but it still receives broadcasts from someone on a different channel. I need to dig more into who is sending those messages and I will re-test.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #726: Ignore jgroup messages coming from itself
Posted by belaban <gi...@git.apache.org>.
Github user belaban commented on the issue:
https://github.com/apache/activemq-artemis/pull/726
An even more efficient way would be to tag the message with transient flag DONT_LOOPBACK
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request #726: Don't send jgroup messages to self
Posted by lulf <gi...@git.apache.org>.
Github user lulf closed the pull request at:
https://github.com/apache/activemq-artemis/pull/726
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #726: Don't send jgroup messages to self
Posted by lulf <gi...@git.apache.org>.
Github user lulf commented on the issue:
https://github.com/apache/activemq-artemis/pull/726
@belaban I was a bit too quick when testing this, sorry. Setting DONT_LOOPBACK doesn't work after all. My jgroups config us using TCP unicast, in which case its unclear to me from the documentation if the setting will have an effect.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request #726: Don't send jgroup messages to self
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/726#discussion_r75871028
--- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/jgroups/JChannelWrapper.java ---
@@ -39,7 +40,7 @@
final List<JGroupsReceiver> receivers = new ArrayList<>();
private final JChannelManager manager;
- public JChannelWrapper(JChannelManager manager, final String channelName, JChannel channel) throws Exception {
--- End diff --
I don't see why you would need to make this change here? can you place the final back?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request #726: Don't send jgroup messages to self
Posted by lulf <gi...@git.apache.org>.
Github user lulf commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/726#discussion_r76062794
--- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/jgroups/JChannelManager.java ---
@@ -34,7 +34,7 @@
private static final Logger logger = Logger.getLogger(JChannelManager.class);
- private Map<String, JChannelWrapper> channels;
--- End diff --
@clebertsuconic The intention was that multiple instances of JChannelManager would share the same set of channels. However, I see that I forgot to synchronize global access to this map, so this change has race conditions.
In any case, what approach would you prefer to make wrt. making only one channel get created across broadcast and discovery groups?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #726: Don't send jgroup messages to self
Posted by belaban <gi...@git.apache.org>.
Github user belaban commented on the issue:
https://github.com/apache/activemq-artemis/pull/726
Try this sample program below (args are config and name):
````
package org.jgroups.tests;
import org.jgroups.JChannel;
import org.jgroups.Message;
import org.jgroups.ReceiverAdapter;
import org.jgroups.View;
import org.jgroups.util.Util;
/**
* @author Bela Ban
* @since x.y
*/
public class bla2 extends ReceiverAdapter {
protected JChannel ch;
public void receive(Message msg) {
System.out.printf("-- msg from %s: %s\n", msg.src(), msg.getObject());
}
public void viewAccepted(View view) {
System.out.printf("view: %s\n", view);
}
protected void start(String config, String name) throws Exception {
ch=new JChannel(config).name(name).receiver(this).connect("demo");
int counter=1;
for(;;) {
Util.keyPress("<send msg to all except self>");
Message msg=new Message (null, "hello-" + counter++).setTransientFlag(Message.TransientFlag.DONT_LOOPBACK);
ch.send(msg);
}
}
public static void main(String[] args) throws Exception {
new bla2().start(args[0], args[1]);
}
}
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #726: Don't send jgroup messages to self
Posted by lulf <gi...@git.apache.org>.
Github user lulf commented on the issue:
https://github.com/apache/activemq-artemis/pull/726
@clebertsuconic Great, thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request #726: Don't send jgroup messages to self
Posted by lulf <gi...@git.apache.org>.
Github user lulf commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/726#discussion_r76041070
--- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/jgroups/JChannelManager.java ---
@@ -34,7 +34,7 @@
private static final Logger logger = Logger.getLogger(JChannelManager.class);
- private Map<String, JChannelWrapper> channels;
--- End diff --
@clebertsuconic The ChannelBroadcastEndpointFactory is not used in my code path. Instead, the JGroupsFileBroadcastEndpointFactory is used, see FileConfigurationParser line 1218 and 1251.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #726: Don't send jgroup messages to self
Posted by belaban <gi...@git.apache.org>.
Github user belaban commented on the issue:
https://github.com/apache/activemq-artemis/pull/726
Oops, sorry! You're sending a message to ABCD as single unicasts to B, C and D individually? In this case, the above sample program won't work, correct...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #726: Don't send jgroup messages to self
Posted by lulf <gi...@git.apache.org>.
Github user lulf commented on the issue:
https://github.com/apache/activemq-artemis/pull/726
i - Created jira: https://issues.apache.org/jira/browse/ARTEMIS-697
ii - I think the test would have to trigger the jgroupsfile path, in which case it would have to use the network. Unless there is a jgroups 'test network' that could be used.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request #726: Don't send jgroup messages to self
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/726#discussion_r75872240
--- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/jgroups/JChannelManager.java ---
@@ -34,7 +34,7 @@
private static final Logger logger = Logger.getLogger(JChannelManager.class);
- private Map<String, JChannelWrapper> channels;
--- End diff --
It is not needed anyways.
ChannelBroadcastEndpointFactory::singletonManager is already static.
Revert this please. If you want to make sure this is static, we could make the whole JChannelManager singleton. But I'm not sure now if there is a reason to not to.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request #726: Don't send jgroup messages to self
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/726#discussion_r76065017
--- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/jgroups/JChannelManager.java ---
@@ -34,7 +34,7 @@
private static final Logger logger = Logger.getLogger(JChannelManager.class);
- private Map<String, JChannelWrapper> channels;
--- End diff --
the factory itself was supposed to be singleton. I will make JChannelManager singleton on a separate commit. Let me handle it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #726: Don't send jgroup messages to self
Posted by belaban <gi...@git.apache.org>.
Github user belaban commented on the issue:
https://github.com/apache/activemq-artemis/pull/726
@lulf I'm not familiar with how Artemis uses JGroups, so no feedback from me there...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #726: Don't send jgroup messages to self
Posted by lulf <gi...@git.apache.org>.
Github user lulf commented on the issue:
https://github.com/apache/activemq-artemis/pull/726
@belaban Ok, thanks anyway.
I've updated the PR with a fix that is working for me now. I imagine that ideally broadcast and discovery should not create two channels, and maybe share the JChannelManager instance, but it comes from quite different code paths AFAICT.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request #726: Don't send jgroup messages to self
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/726#discussion_r76061054
--- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/jgroups/JChannelManager.java ---
@@ -34,7 +34,7 @@
private static final Logger logger = Logger.getLogger(JChannelManager.class);
- private Map<String, JChannelWrapper> channels;
--- End diff --
@lulf that doesn't explain (at least I don't think so) why you had to make channels a static attribute
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request #726: Don't send jgroup messages to self
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/726#discussion_r75872292
--- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/jgroups/JChannelWrapper.java ---
@@ -127,6 +128,7 @@ public void addReceiver(JGroupsReceiver jGroupsReceiver) {
public void send(org.jgroups.Message msg) throws Exception {
if (logger.isTraceEnabled()) logger.trace(this + "::Sending JGroups Message: Open=" + channel.isOpen() + " on channel " + channelName + " msg=" + msg);
+ msg.setTransientFlag(Message.TransientFlag.DONT_LOOPBACK);
--- End diff --
this is probably the only line needed here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #726: Don't send jgroup messages to self
Posted by lulf <gi...@git.apache.org>.
Github user lulf commented on the issue:
https://github.com/apache/activemq-artemis/pull/726
@belaban Originally, I used loopback=false in the jgroups config, but that didn't seem to have the desired effect. Setting the flag on the message works though, thanks!
Updated the PR with fix suggested by @belaban
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request #726: Don't send jgroup messages to self
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/726#discussion_r75871149
--- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/jgroups/JChannelManager.java ---
@@ -34,7 +34,7 @@
private static final Logger logger = Logger.getLogger(JChannelManager.class);
- private Map<String, JChannelWrapper> channels;
--- End diff --
why the static change here? this changes the semantic completely from what you needed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #726: Don't send jgroup messages to self
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/726
2 things:
i - can you create a JIRA.
ii - is it possible to validate this with a unit test?
Also: can you make the ammends I asked here? if you can't let me know and I can do them during merge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #726: Don't send jgroup messages to self
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/726
@lulf look at #731. I added an extra commit there.
Can you close this PR, and I have imported your commit into #731
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---