You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2020/03/04 08:20:32 UTC
[GitHub] [activemq-artemis] brusdev opened a new pull request #2998:
ARTEMIS-2641 Openwire client runs out of credits after reconnection
brusdev opened a new pull request #2998: ARTEMIS-2641 Openwire client runs out of credits after reconnection
URL: https://github.com/apache/activemq-artemis/pull/2998
Clear the messagePullHandler on setting prefetchSize.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull
request #2998: ARTEMIS-2641 Openwire client runs out of credits after
reconnection
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #2998: ARTEMIS-2641 Openwire client runs out of credits after reconnection
URL: https://github.com/apache/activemq-artemis/pull/2998#discussion_r387525491
##########
File path: artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/amq/AMQConsumer.java
##########
@@ -380,6 +383,11 @@ public void setPrefetchSize(int prefetchSize) {
this.prefetchSize = prefetchSize;
this.currentWindow.set(prefetchSize);
this.info.setPrefetchSize(prefetchSize);
+ if (this.prefetchSize == 0) {
+ messagePullHandler.compareAndSet(null, new MessagePullHandler());
Review comment:
what do you mean if it wont succeed? if its not null it means it already had a pullhandler set, no need to assign a new one.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] franz1981 commented on a change in pull request
#2998: ARTEMIS-2641 Openwire client runs out of credits after reconnection
Posted by GitBox <gi...@apache.org>.
franz1981 commented on a change in pull request #2998: ARTEMIS-2641 Openwire client runs out of credits after reconnection
URL: https://github.com/apache/activemq-artemis/pull/2998#discussion_r387528013
##########
File path: artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/amq/AMQConsumer.java
##########
@@ -380,6 +383,11 @@ public void setPrefetchSize(int prefetchSize) {
this.prefetchSize = prefetchSize;
this.currentWindow.set(prefetchSize);
this.info.setPrefetchSize(prefetchSize);
+ if (this.prefetchSize == 0) {
+ messagePullHandler.compareAndSet(null, new MessagePullHandler());
Review comment:
that can happen if:
1. there is a racing thread doing it
2. the value isn't null
in both cases probably logging some trace/debug is ok if we expect it to happen?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] franz1981 commented on a change in pull request
#2998: ARTEMIS-2641 Openwire client runs out of credits after reconnection
Posted by GitBox <gi...@apache.org>.
franz1981 commented on a change in pull request #2998: ARTEMIS-2641 Openwire client runs out of credits after reconnection
URL: https://github.com/apache/activemq-artemis/pull/2998#discussion_r387530237
##########
File path: artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/amq/AMQConsumer.java
##########
@@ -380,6 +383,11 @@ public void setPrefetchSize(int prefetchSize) {
this.prefetchSize = prefetchSize;
this.currentWindow.set(prefetchSize);
this.info.setPrefetchSize(prefetchSize);
+ if (this.prefetchSize == 0) {
+ messagePullHandler.compareAndSet(null, new MessagePullHandler());
Review comment:
:+1: thanks for the explanation bud :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] franz1981 commented on a change in pull request
#2998: ARTEMIS-2641 Openwire client runs out of credits after reconnection
Posted by GitBox <gi...@apache.org>.
franz1981 commented on a change in pull request #2998: ARTEMIS-2641 Openwire client runs out of credits after reconnection
URL: https://github.com/apache/activemq-artemis/pull/2998#discussion_r387523995
##########
File path: artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/amq/AMQConsumer.java
##########
@@ -380,6 +383,11 @@ public void setPrefetchSize(int prefetchSize) {
this.prefetchSize = prefetchSize;
this.currentWindow.set(prefetchSize);
this.info.setPrefetchSize(prefetchSize);
+ if (this.prefetchSize == 0) {
+ messagePullHandler.compareAndSet(null, new MessagePullHandler());
Review comment:
@michaelandrepearce any suggestions?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull
request #2998: ARTEMIS-2641 Openwire client runs out of credits after
reconnection
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #2998: ARTEMIS-2641 Openwire client runs out of credits after reconnection
URL: https://github.com/apache/activemq-artemis/pull/2998#discussion_r387529583
##########
File path: artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/amq/AMQConsumer.java
##########
@@ -380,6 +383,11 @@ public void setPrefetchSize(int prefetchSize) {
this.prefetchSize = prefetchSize;
this.currentWindow.set(prefetchSize);
this.info.setPrefetchSize(prefetchSize);
+ if (this.prefetchSize == 0) {
+ messagePullHandler.compareAndSet(null, new MessagePullHandler());
Review comment:
if you really want a log i would add it as a trace, i dont think its needed tbh i think it just clutter the code base.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] franz1981 commented on a change in pull request
#2998: ARTEMIS-2641 Openwire client runs out of credits after reconnection
Posted by GitBox <gi...@apache.org>.
franz1981 commented on a change in pull request #2998: ARTEMIS-2641 Openwire client runs out of credits after reconnection
URL: https://github.com/apache/activemq-artemis/pull/2998#discussion_r387523649
##########
File path: artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/amq/AMQConsumer.java
##########
@@ -380,6 +383,11 @@ public void setPrefetchSize(int prefetchSize) {
this.prefetchSize = prefetchSize;
this.currentWindow.set(prefetchSize);
this.info.setPrefetchSize(prefetchSize);
+ if (this.prefetchSize == 0) {
+ messagePullHandler.compareAndSet(null, new MessagePullHandler());
Review comment:
what happen if it won't succeed?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull
request #2998: ARTEMIS-2641 Openwire client runs out of credits after
reconnection
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #2998: ARTEMIS-2641 Openwire client runs out of credits after reconnection
URL: https://github.com/apache/activemq-artemis/pull/2998#discussion_r387531710
##########
File path: artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/amq/AMQConsumer.java
##########
@@ -380,6 +383,11 @@ public void setPrefetchSize(int prefetchSize) {
this.prefetchSize = prefetchSize;
this.currentWindow.set(prefetchSize);
this.info.setPrefetchSize(prefetchSize);
+ if (this.prefetchSize == 0) {
+ messagePullHandler.compareAndSet(null, new MessagePullHandler());
Review comment:
> Anyway It's fine to me; if there are no objections I'm going to merge this ;)
no objections here, please go ahead....
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] asfgit merged pull request #2998: ARTEMIS-2641
Openwire client runs out of credits after reconnection
Posted by GitBox <gi...@apache.org>.
asfgit merged pull request #2998: ARTEMIS-2641 Openwire client runs out of credits after reconnection
URL: https://github.com/apache/activemq-artemis/pull/2998
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] franz1981 commented on a change in pull request
#2998: ARTEMIS-2641 Openwire client runs out of credits after reconnection
Posted by GitBox <gi...@apache.org>.
franz1981 commented on a change in pull request #2998: ARTEMIS-2641 Openwire client runs out of credits after reconnection
URL: https://github.com/apache/activemq-artemis/pull/2998#discussion_r387528752
##########
File path: artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/amq/AMQConsumer.java
##########
@@ -380,6 +383,11 @@ public void setPrefetchSize(int prefetchSize) {
this.prefetchSize = prefetchSize;
this.currentWindow.set(prefetchSize);
this.info.setPrefetchSize(prefetchSize);
+ if (this.prefetchSize == 0) {
+ messagePullHandler.compareAndSet(null, new MessagePullHandler());
Review comment:
Anyway It's fine to me; if there are no objections I'm going to merge this ;)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] asfgit merged pull request #2998: ARTEMIS-2641
Openwire client runs out of credits after reconnection
Posted by GitBox <gi...@apache.org>.
asfgit merged pull request #2998: ARTEMIS-2641 Openwire client runs out of credits after reconnection
URL: https://github.com/apache/activemq-artemis/pull/2998
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] brusdev commented on a change in pull request
#2998: ARTEMIS-2641 Openwire client runs out of credits after reconnection
Posted by GitBox <gi...@apache.org>.
brusdev commented on a change in pull request #2998: ARTEMIS-2641 Openwire client runs out of credits after reconnection
URL: https://github.com/apache/activemq-artemis/pull/2998#discussion_r387529855
##########
File path: artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/amq/AMQConsumer.java
##########
@@ -380,6 +383,11 @@ public void setPrefetchSize(int prefetchSize) {
this.prefetchSize = prefetchSize;
this.currentWindow.set(prefetchSize);
this.info.setPrefetchSize(prefetchSize);
+ if (this.prefetchSize == 0) {
+ messagePullHandler.compareAndSet(null, new MessagePullHandler());
Review comment:
no need to set messagePullHandler value if it isn't null
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] michaelpearce-gain commented on issue #2998:
ARTEMIS-2641 Openwire client runs out of credits after reconnection
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on issue #2998: ARTEMIS-2641 Openwire client runs out of credits after reconnection
URL: https://github.com/apache/activemq-artemis/pull/2998#issuecomment-594401738
LGTM (obviously ;) ), nice test, that was the only bit i was struggling with how to think to implement.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull
request #2998: ARTEMIS-2641 Openwire client runs out of credits after
reconnection
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #2998: ARTEMIS-2641 Openwire client runs out of credits after reconnection
URL: https://github.com/apache/activemq-artemis/pull/2998#discussion_r387529034
##########
File path: artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/amq/AMQConsumer.java
##########
@@ -380,6 +383,11 @@ public void setPrefetchSize(int prefetchSize) {
this.prefetchSize = prefetchSize;
this.currentWindow.set(prefetchSize);
this.info.setPrefetchSize(prefetchSize);
+ if (this.prefetchSize == 0) {
+ messagePullHandler.compareAndSet(null, new MessagePullHandler());
Review comment:
if its not null, we dont want to change it. e.g. the case there is client was already at prefetch 0 and simply sent a second update which set it to the same size, as such you want to leave the original pullhandler.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services