[jira] [Commented] (ARTEMIS-1607) OpenWire is sending responses too early with durable messages

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16336076#comment-16336076
 ] 

ASF GitHub Bot commented on ARTEMIS-1607:
-

Github user asfgit closed the pull request at:

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


> OpenWire is sending responses too early with durable messages
> -
>
> Key: ARTEMIS-1607
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1607
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1607) OpenWire is sending responses too early with durable messages

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16336075#comment-16336075
 ] 

ASF subversion and git services commented on ARTEMIS-1607:
--

Commit 56d68f0b723ccbca8ec4ffbc26a9c1d0be9c0002 in activemq-artemis's branch 
refs/heads/master from [~nigro@gmail.com]
[ https://git-wip-us.apache.org/repos/asf?p=activemq-artemis.git;h=56d68f0 ]

ARTEMIS-1607 OpenWire is sending responses too early with durable messages

AMQSession is sending response back without waiting past I/O tasks on 
StorageManager to be finished


> OpenWire is sending responses too early with durable messages
> -
>
> Key: ARTEMIS-1607
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1607
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1607) OpenWire is sending responses too early with durable messages

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16336015#comment-16336015
 ] 

ASF GitHub Bot commented on ARTEMIS-1607:
-

Github user franz1981 commented on the issue:

https://github.com/apache/activemq-artemis/pull/1801
  
@clebertsuconic I've reverted with the original not blocking version, but 
avoiding calling the shutdown, makes sense?
I'm just worried about racing accesses to the connection by different 
threads...


> OpenWire is sending responses too early with durable messages
> -
>
> Key: ARTEMIS-1607
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1607
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1607) OpenWire is sending responses too early with durable messages

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16335169#comment-16335169
 ] 

ASF GitHub Bot commented on ARTEMIS-1607:
-

Github user franz1981 commented on the issue:

https://github.com/apache/activemq-artemis/pull/1801
  
@clebertsuconic Np I've understood what you mean :) 
I've pushed another commit with another solution less "reactive" but more 
simlar to the original code: let me know if it seems better and I will squash 
the commits into this last one :+1: 


> OpenWire is sending responses too early with durable messages
> -
>
> Key: ARTEMIS-1607
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1607
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1607) OpenWire is sending responses too early with durable messages

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16335101#comment-16335101
 ] 

ASF GitHub Bot commented on ARTEMIS-1607:
-

Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/1801
  
@franz1981 typo on my last message.. I actually meant the autoRead stuff.


> OpenWire is sending responses too early with durable messages
> -
>
> Key: ARTEMIS-1607
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1607
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1607) OpenWire is sending responses too early with durable messages

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16335080#comment-16335080
 ] 

ASF GitHub Bot commented on ARTEMIS-1607:
-

Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/1801
  
@franz1981 I told you it was ok to do it this way.. but I'm a bit concerned 
by the use of setResponse(true) and false through this.

is there a way to return the packet like it used to be done before? can you 
check the setRespond(true) and false usages? I'm concerned of a possible race 
making the server unavailable eventually.


> OpenWire is sending responses too early with durable messages
> -
>
> Key: ARTEMIS-1607
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1607
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1607) OpenWire is sending responses too early with durable messages

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16334882#comment-16334882
 ] 

ASF GitHub Bot commented on ARTEMIS-1607:
-

Github user clebertsuconic commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1801#discussion_r163065589
  
--- Diff: 
artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/amq/AMQSession.java
 ---
@@ -393,26 +395,47 @@ public void send(final ProducerInfo producerInfo,
  
this.connection.getContext().setDontSendReponse(false);
  connection.sendException(exceptionToSend);
   } else {
- if (sendProducerAck) {
-try {
-   ProducerAck ack = new 
ProducerAck(producerInfo.getProducerId(), messageSend.getSize());
-   connection.dispatchAsync(ack);
-} catch (Exception e) {
-   
this.connection.getContext().setDontSendReponse(false);
-   
ActiveMQServerLogger.LOGGER.warn(e.getMessage(), e);
-   connection.sendException(e);
+ 
server.getStorageManager().afterCompleteOperations(new IOCallback() {
+@Override
+public void done() {
+   if (sendProducerAck) {
+  try {
+ ProducerAck ack = new 
ProducerAck(producerInfo.getProducerId(), messageSend.getSize());
+ connection.dispatchAsync(ack);
+  } catch (Exception e) {
+ 
connection.getContext().setDontSendReponse(false);
+ 
ActiveMQServerLogger.LOGGER.warn(e.getMessage(), e);
+ connection.sendException(e);
+  }
+   } else {
+  
connection.getContext().setDontSendReponse(false);
+  try {
+ Response response = new Response();
+ 
response.setCorrelationId(messageSend.getCommandId());
+ connection.dispatchAsync(response);
+  } catch (Exception e) {
+ 
ActiveMQServerLogger.LOGGER.warn(e.getMessage(), e);
+ connection.sendException(e);
+  }
+   }
 }
- } else {
-connection.getContext().setDontSendReponse(false);
-try {
-   Response response = new Response();
-   
response.setCorrelationId(messageSend.getCommandId());
-   connection.dispatchAsync(response);
-} catch (Exception e) {
-   
ActiveMQServerLogger.LOGGER.warn(e.getMessage(), e);
-   connection.sendException(e);
+
+@Override
+public void onError(int errorCode, String 
errorMessage) {
+   //failing here is severe and IO related
+   final Throwable criticalError = new 
IOException(errorMessage);
+   try {
+  //it is handled async and hopefully will be 
sent before the critical error shutdown the broker:
+  //it helps to fail fast the clients on 
critical errors
+  connection.serviceException(criticalError);
+   } catch (Exception e) {
+  ActiveMQServerLogger.LOGGER.debug(e);
+   } finally {
+  //it needs to be called ASAP: the broker 
isn't in a safe state
+  
server.getStorageManager().criticalError(criticalError);
--- End diff --

this is probably also called earlier by the IO layer.. but it doesn't hurt 
here.


> OpenWire is sending responses too early with durable messages
> -
>
> Key: ARTEMIS-1607
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1607
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>

[jira] [Commented] (ARTEMIS-1607) OpenWire is sending responses too early with durable messages

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16334709#comment-16334709
 ] 

ASF GitHub Bot commented on ARTEMIS-1607:
-

Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/1801
  
The errors on the completion are critical.  They are IOerrors.  I think the 
critical exception will be called anyways. 


> OpenWire is sending responses too early with durable messages
> -
>
> Key: ARTEMIS-1607
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1607
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1607) OpenWire is sending responses too early with durable messages

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16334624#comment-16334624
 ] 

ASF GitHub Bot commented on ARTEMIS-1607:
-

Github user franz1981 commented on the issue:

https://github.com/apache/activemq-artemis/pull/1801
  
@gtully Good point! I've added an async send of error that finally will end 
with the critical error as before: I hope (@clebertsuconic could help confirm 
that) that it is safe enough.
There could be the chance that the exception won't be never send back to 
the clients because the critical error handling will shutdown the server before 
the send will happen!


> OpenWire is sending responses too early with durable messages
> -
>
> Key: ARTEMIS-1607
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1607
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1607) OpenWire is sending responses too early with durable messages

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16334539#comment-16334539
 ] 

ASF GitHub Bot commented on ARTEMIS-1607:
-

Github user gtully commented on the issue:

https://github.com/apache/activemq-artemis/pull/1801
  
think any error from the io completion callback should result in 
connection.sendException(...);


> OpenWire is sending responses too early with durable messages
> -
>
> Key: ARTEMIS-1607
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1607
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1607) OpenWire is sending responses too early with durable messages

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16334451#comment-16334451
 ] 

ASF GitHub Bot commented on ARTEMIS-1607:
-

Github user franz1981 commented on the issue:

https://github.com/apache/activemq-artemis/pull/1801
  
Possibly I will add a test but is tricky and I need to run CI on it too 
:+1: 


> OpenWire is sending responses too early with durable messages
> -
>
> Key: ARTEMIS-1607
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1607
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1607) OpenWire is sending responses too early with durable messages

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16334447#comment-16334447
 ] 

ASF GitHub Bot commented on ARTEMIS-1607:
-

GitHub user franz1981 opened a pull request:

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

ARTEMIS-1607 OpenWire is sending responses too early with durable messages

AMQSession is sending response back without waiting past I/O tasks on 
StorageManager to be finished

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

$ git pull https://github.com/franz1981/activemq-artemis open_wire_fix

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

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


commit 6ce9bee89e24cc865509f8ba26f9261ec4e5a106
Author: Michael André Pearce 
Date:   2018-01-13T19:47:58Z

ARTEMIS-1606 - Change AddressInfo RoutingType Set to use EnumSet

Change all use from Set to EnumSet
Deprecating any old exposed interfaces but keeping for back compatibility.
Address info to avoid iterator on getRoutingType hotpath, like wise can be 
avoided where single RoutingType is passed in.

commit e2683e11068a45a9dce08d806dfeb295cf91fd71
Author: Francesco Nigro 
Date:   2018-01-22T15:06:48Z

ARTEMIS-1607 OpenWire is sending responses too early with durable messages

AMQSession is sending response back without waiting past I/O tasks on 
StorageManager to be finished




> OpenWire is sending responses too early with durable messages
> -
>
> Key: ARTEMIS-1607
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1607
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)