You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by clebertsuconic <gi...@git.apache.org> on 2018/06/12 20:56:21 UTC
[GitHub] activemq-artemis pull request #2139: ARTEMIS-1927 treat exceptions and disco...
GitHub user clebertsuconic opened a pull request:
https://github.com/apache/activemq-artemis/pull/2139
ARTEMIS-1927 treat exceptions and disconnect properly
I couldn't do this with a test as this is dependent on windows. Regular testsuite should be sufficient as long as you run in Windows.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/clebertsuconic/activemq-artemis ARTEMIS-1927
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/2139.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 #2139
----
commit aea51a23af7a4a94e18835cbd4e7ae056946484e
Author: Clebert Suconic <cl...@...>
Date: 2018-06-12T20:29:13Z
ARTEMIS-1927 treat exceptions and disconnect properly
I couldn't do this with a test as this is dependent on windows. Regular testsuite should be sufficient as long as you run in Windows.
----
---
[GitHub] activemq-artemis pull request #2139: ARTEMIS-1927 treat exceptions and disco...
Posted by jbertram <gi...@git.apache.org>.
Github user jbertram commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2139#discussion_r194890149
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/server/impl/RemotingServiceImpl.java ---
@@ -554,20 +558,13 @@ public void connectionDestroyed(final Object connectionID) {
return;
}
}
- conn.connection.fail(new ActiveMQRemoteDisconnectException());
+ conn.connection.fail(e);
}
}
@Override
public void connectionException(final Object connectionID, final ActiveMQException me) {
- // We DO NOT call fail on connection exception, otherwise in event of real connection failure, the
- // connection will be failed, the session will be closed and won't be able to reconnect
-
- // E.g. if live server fails, then this handler wil be called on backup server for the server
- // side replicating connection.
- // If the connection fail() is called then the sessions on the backup will get closed.
-
- // Connections should only fail when TTL is exceeded
+ issueFailure(connectionID, me);
}
--- End diff --
Does this comment no longer apply? Is there not a risk that core clients won't be able to reconnect to their server session in the case of a spurious network failure?
---
[GitHub] activemq-artemis pull request #2139: ARTEMIS-1927 treat exceptions and disco...
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/2139#discussion_r194892207
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/server/impl/RemotingServiceImpl.java ---
@@ -554,20 +558,13 @@ public void connectionDestroyed(final Object connectionID) {
return;
}
}
- conn.connection.fail(new ActiveMQRemoteDisconnectException());
+ conn.connection.fail(e);
}
}
@Override
public void connectionException(final Object connectionID, final ActiveMQException me) {
- // We DO NOT call fail on connection exception, otherwise in event of real connection failure, the
- // connection will be failed, the session will be closed and won't be able to reconnect
-
- // E.g. if live server fails, then this handler wil be called on backup server for the server
- // side replicating connection.
- // If the connection fail() is called then the sessions on the backup will get closed.
-
- // Connections should only fail when TTL is exceeded
+ issueFailure(connectionID, me);
}
--- End diff --
I'm running the full testsuite to make sure.
---
[GitHub] activemq-artemis pull request #2139: ARTEMIS-1927 treat exceptions and disco...
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/2139#discussion_r194892092
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/server/impl/RemotingServiceImpl.java ---
@@ -554,20 +558,13 @@ public void connectionDestroyed(final Object connectionID) {
return;
}
}
- conn.connection.fail(new ActiveMQRemoteDisconnectException());
+ conn.connection.fail(e);
}
}
@Override
public void connectionException(final Object connectionID, final ActiveMQException me) {
- // We DO NOT call fail on connection exception, otherwise in event of real connection failure, the
- // connection will be failed, the session will be closed and won't be able to reconnect
-
- // E.g. if live server fails, then this handler wil be called on backup server for the server
- // side replicating connection.
- // If the connection fail() is called then the sessions on the backup will get closed.
-
- // Connections should only fail when TTL is exceeded
+ issueFailure(connectionID, me);
}
--- End diff --
no.. it didn't apply for some time already.
This is legacy from hornetq where we would only issue disconnects based on TTL.
---
[GitHub] activemq-artemis pull request #2139: ARTEMIS-1927 treat exceptions and disco...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/2139
---