[GitHub] activemq-artemis pull request #2090: ARTEMIS-1868 Openwire doesn't add deliv...

2018-05-21 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/activemq-artemis/pull/2090


---


[GitHub] activemq-artemis pull request #2090: ARTEMIS-1868 Openwire doesn't add deliv...

2018-05-16 Thread gaohoward
Github user gaohoward commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/2090#discussion_r188604772
  
--- Diff: 
artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/amq/AMQSession.java
 ---
@@ -308,6 +312,8 @@ public int sendMessage(MessageReference reference,
   ServerConsumer consumer,
   int deliveryCount) {
   AMQConsumer theConsumer = (AMQConsumer) consumer.getProtocolData();
+  //clear up possible rolledback ids.
+  rollbackedIds.remove(message.getMessageID());
--- End diff --

Yes I know this adds some costs. But so far I couldn't have a better 
solution.


---


[GitHub] activemq-artemis pull request #2090: ARTEMIS-1868 Openwire doesn't add deliv...

2018-05-16 Thread gaohoward
Github user gaohoward commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/2090#discussion_r188604794
  
--- Diff: 
artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/amq/AMQSession.java
 ---
@@ -95,6 +97,8 @@
 
private final SimpleString clientId;
 
+   private final Set rollbackedIds = new ConcurrentHashSet<>();
--- End diff --

This I can do. Thanks!


---


[GitHub] activemq-artemis pull request #2090: ARTEMIS-1868 Openwire doesn't add deliv...

2018-05-16 Thread franz1981
Github user franz1981 commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/2090#discussion_r188532813
  
--- Diff: 
artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/amq/AMQSession.java
 ---
@@ -95,6 +97,8 @@
 
private final SimpleString clientId;
 
+   private final Set rollbackedIds = new ConcurrentHashSet<>();
--- End diff --

It could belong to `AMQConsumer` instead of here? That will make it less 
contended, because owned exclusively by the consumer 


---


[GitHub] activemq-artemis pull request #2090: ARTEMIS-1868 Openwire doesn't add deliv...

2018-05-16 Thread franz1981
Github user franz1981 commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/2090#discussion_r188530577
  
--- Diff: 
artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/amq/AMQSession.java
 ---
@@ -308,6 +312,8 @@ public int sendMessage(MessageReference reference,
   ServerConsumer consumer,
   int deliveryCount) {
   AMQConsumer theConsumer = (AMQConsumer) consumer.getProtocolData();
+  //clear up possible rolledback ids.
+  rollbackedIds.remove(message.getMessageID());
--- End diff --

That's in the hot path of any send operation and `rollbackIds` would be 
contended by all the producers/consumers using the same AMQSession and it 
create a Long instance on any call of it. There are other ways to implement it?


---


[GitHub] activemq-artemis pull request #2090: ARTEMIS-1868 Openwire doesn't add deliv...

2018-05-16 Thread gaohoward
GitHub user gaohoward opened a pull request:

https://github.com/apache/activemq-artemis/pull/2090

ARTEMIS-1868 Openwire doesn't add delivery count in client ack mode

If a client ack mode consumer receives a message and closes without
acking it, the redelivery of the message won't set the redelivery
flag (JMSRedelivered) because it doesn't increment the delivery count
when message is cancelled back to queue.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/gaohoward/activemq-artemis b_ent1497

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/2090.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 #2090


commit e0af1e56d9dd8051bcd6b3f1316b00f201afe5e9
Author: Howard Gao 
Date:   2018-05-16T03:14:48Z

ARTEMIS-1868 Openwire doesn't add delivery count in client ack mode

If a client ack mode consumer receives a message and closes without
acking it, the redelivery of the message won't set the redelivery
flag (JMSRedelivered) because it doesn't increment the delivery count
when message is cancelled back to queue.




---