You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by mtaylor <gi...@git.apache.org> on 2018/06/01 16:09:29 UTC
[GitHub] activemq-artemis pull request #2119: Artemis 1902
GitHub user mtaylor opened a pull request:
https://github.com/apache/activemq-artemis/pull/2119
Artemis 1902
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mtaylor/activemq-artemis ARTEMIS-1902
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/2119.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 #2119
----
commit e64986fcc0f05e6ed10816ca5e0718766e4dba46
Author: Martyn Taylor <mt...@...>
Date: 2018-06-01T15:33:18Z
ARTEMIS-1902 Add AMQP redistributor stop test
commit 711ec25c6daff8ed7649d36c44821893147ec155
Author: Martyn Taylor <mt...@...>
Date: 2018-06-01T16:04:50Z
ARTEMIS-1902 Ensure ServerConsumer close done once
Calling close multiple times on ServerConsumer can result in multiple
notifications being routed around the cluster. This causes cluster
topology info to become skewed. Which affects a number of components
such as message redistribution, metrics and can eventually cause OOM
should multiple queues be redistributing at the same time.
----
---
[GitHub] activemq-artemis issue #2119: Artemis 1902
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/2119
@mtaylor I know I have in the past asked to separate test and fix.. but I went back on that.. as it's easier to cherry-pick.. and the gitreport would make a better correlation.
You ok if we squash fix and test together on a single commit?
---
[GitHub] activemq-artemis issue #2119: Artemis 1902
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/2119
The git release report makes it easier to correlate code added versus test added.
---
[GitHub] activemq-artemis issue #2119: Artemis 1902
Posted by mtaylor <gi...@git.apache.org>.
Github user mtaylor commented on the issue:
https://github.com/apache/activemq-artemis/pull/2119
@clebertsuconic Feel free to squash the commits once I've updated with @michaelandrepearce feedback. I don't really care. However, cherry-picking should be straight forward, since both commits have the JIRA, and this PR with the commits is also posted in the JIRA.
---
[GitHub] activemq-artemis pull request #2119: Artemis 1902
Posted by mtaylor <gi...@git.apache.org>.
Github user mtaylor commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2119#discussion_r192558538
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/QueueInfo.java ---
@@ -107,7 +108,11 @@ public void incrementConsumers() {
}
public void decrementConsumers() {
- numberOfConsumers--;
+ if (numberOfConsumers > 0) {
--- End diff --
@michaelandrepearce +1. I will make this an AtomicInteger cheers.
---
[GitHub] activemq-artemis pull request #2119: Artemis 1902
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/2119#discussion_r192559076
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/QueueInfo.java ---
@@ -107,7 +108,11 @@ public void incrementConsumers() {
}
public void decrementConsumers() {
- numberOfConsumers--;
+ if (numberOfConsumers > 0) {
--- End diff --
The atomic updater would be better then atomic integer. I think that’s what Michael is referring.
---
[GitHub] activemq-artemis pull request #2119: Artemis 1902
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/2119#discussion_r192514909
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/QueueInfo.java ---
@@ -107,7 +108,11 @@ public void incrementConsumers() {
}
public void decrementConsumers() {
- numberOfConsumers--;
+ if (numberOfConsumers > 0) {
--- End diff --
This isnt being atomically updated so still possible two decrements at the same time can cause the number to go negative incorrectly.
To avoid this ideally should syncronize any update to the field or make changes to the field using an atomic updater
---
[GitHub] activemq-artemis pull request #2119: Artemis 1902
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/2119
---