You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by JiriOndrusek <gi...@git.apache.org> on 2018/01/09 14:27:15 UTC
[GitHub] activemq-artemis pull request #1761: [ARTEMIS-1527] - [Artemis Testsuite] Ac...
GitHub user JiriOndrusek opened a pull request:
https://github.com/apache/activemq-artemis/pull/1761
[ARTEMIS-1527] - [Artemis Testsuite] ActiveMQMessageHandlerTest#testS…
…erverShutdownAndReconnect fails
Issue: https://issues.apache.org/jira/browse/ARTEMIS-1527
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/JiriOndrusek/activemq-artemis ARTEMIS-1527-fix
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/1761.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 #1761
----
commit 367e633846b020bb002e0cc0ff228df6bddefaa3
Author: JiriOndrusek <jo...@...>
Date: 2018-01-09T08:24:03Z
[ARTEMIS-1527] - [Artemis Testsuite] ActiveMQMessageHandlerTest#testServerShutdownAndReconnect fails
----
---
[GitHub] activemq-artemis issue #1761: [ARTEMIS-1527] - [Artemis Testsuite] ActiveMQM...
Posted by JiriOndrusek <gi...@git.apache.org>.
Github user JiriOndrusek commented on the issue:
https://github.com/apache/activemq-artemis/pull/1761
In 2 places I have to use "constant" for repetitive checking, whether action is already finished. May I use some generic timeout used in project (or define new attribute, ...)
(ClientSessionFactoryImpl line 1123 and RemotingConnectionImpl line 254)
---
[GitHub] activemq-artemis pull request #1761: [ARTEMIS-1527] - [Artemis Testsuite] Ac...
Posted by JiriOndrusek <gi...@git.apache.org>.
Github user JiriOndrusek commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/1761#discussion_r160420636
--- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/RemotingConnectionImpl.java ---
@@ -205,14 +211,59 @@ public void fail(final ActiveMQException me, String scaleDownTargetNodeID) {
}
// Then call the listeners
- callFailureListeners(me, scaleDownTargetNodeID);
+ List<CountDownLatch> failureLatches = callFailureListeners(me, scaleDownTargetNodeID);
callClosingListeners();
- internalClose();
+ CountDownLatch latch = new CountDownLatch(1);
- for (Channel channel : channels.values()) {
- channel.returnBlocking(me);
+ new ScheduledInternalClose(failureLatches, me, latch).run();
+
+ return latch;
+ }
+
+ // internalClose has to be called after failureListeners are finished.
+ private class ScheduledInternalClose implements Runnable {
+
+ private final List<CountDownLatch> failureLatches;
+ private final ActiveMQException me;
+ private final CountDownLatch latch;
+
+ public ScheduledInternalClose(List<CountDownLatch> failureLatches, final ActiveMQException me, CountDownLatch latch) {
+ this.failureLatches = failureLatches;
+ this.me = me;
+ this.latch = latch;
+ }
+
+ @Override
+ public void run() {
+ List<CountDownLatch> running = new LinkedList<>();
+ failureLatches.forEach((l) -> {
+ try {
+ //interval is defined during scheduled execution
+ if (!l.await(1, TimeUnit.MILLISECONDS)) {
+ running.add(l);
+ }
+ } catch (InterruptedException e) {
+ ActiveMQClientLogger.LOGGER.warn(e.getMessage(), e);
+ }
+ });
+
+ if (!running.isEmpty()) {
+ //TODO(jondruse) is there constant or propertyF usable for this kind of wait interval?
+ scheduledExecutorService.schedule(new ScheduledInternalClose(running, me, latch), 500, TimeUnit.MILLISECONDS);
--- End diff --
I have too use "constant" for repetitive checking, whether action is already finished. May I use some generic timeout used in project (or define new attribute, ...)
---
[GitHub] activemq-artemis issue #1761: [ARTEMIS-1527] - [Artemis Testsuite] ActiveMQM...
Posted by jbertram <gi...@git.apache.org>.
Github user jbertram commented on the issue:
https://github.com/apache/activemq-artemis/pull/1761
@JiriOndrusek, I don't think this is going to get merged. Can you go ahead and close this PR?
---
[GitHub] activemq-artemis issue #1761: [ARTEMIS-1527] - [Artemis Testsuite] ActiveMQM...
Posted by JiriOndrusek <gi...@git.apache.org>.
Github user JiriOndrusek commented on the issue:
https://github.com/apache/activemq-artemis/pull/1761
I agree, closing as requested in previous comment.
---
[GitHub] activemq-artemis issue #1761: [ARTEMIS-1527] - [Artemis Testsuite] ActiveMQM...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1761
-1000 on this PR...
it's a brittle change over a test failure. .you could find other way to fix the test?
it's a huge change on the maintenance 1.x branch... it's a risky change... if this is a go... it needs to be on master. (I wouldn't ever cherry-pick something this size into 1.x anyways).
---
[GitHub] activemq-artemis pull request #1761: [ARTEMIS-1527] - [Artemis Testsuite] Ac...
Posted by JiriOndrusek <gi...@git.apache.org>.
Github user JiriOndrusek closed the pull request at:
https://github.com/apache/activemq-artemis/pull/1761
---
[GitHub] activemq-artemis pull request #1761: [ARTEMIS-1527] - [Artemis Testsuite] Ac...
Posted by JiriOndrusek <gi...@git.apache.org>.
Github user JiriOndrusek commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/1761#discussion_r160421530
--- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ClientSessionFactoryImpl.java ---
@@ -990,17 +1111,26 @@ public CloseRunnable(RemotingConnection conn, String scaleDownTargetNodeID) {
// can cause reconnect loop
@Override
public void run() {
- try {
- CLOSE_RUNNABLES.add(this);
- if (scaleDownTargetNodeID == null) {
- conn.fail(ActiveMQClientMessageBundle.BUNDLE.disconnected());
- } else {
- conn.fail(ActiveMQClientMessageBundle.BUNDLE.disconnected(), scaleDownTargetNodeID);
- }
- } finally {
- CLOSE_RUNNABLES.remove(this);
+ CLOSE_RUNNABLES.add(this);
+ CountDownLatch latch;
+ if (scaleDownTargetNodeID == null) {
+ latch = conn.fail(ActiveMQClientMessageBundle.BUNDLE.disconnected());
+ } else {
+ latch = conn.fail(ActiveMQClientMessageBundle.BUNDLE.disconnected(), scaleDownTargetNodeID);
}
+ //TODO(jondruse) is there constant or property usable for this kind of wait interval?
--- End diff --
I have too use "constant" for repetitive checking, whether action is already finished. May I use some generic timeout used in project (or define new attribute, ...)
---