[GitHub] activemq-artemis pull request #2139: ARTEMIS-1927 treat exceptions and disco...
Github user asfgit closed the pull request at: https://github.com/apache/activemq-artemis/pull/2139 ---
[GitHub] activemq-artemis pull request #2139: ARTEMIS-1927 treat exceptions and disco...
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...
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...
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...
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 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. ---