[jira] [Commented] (ARTEMIS-1607) OpenWire is sending responses too early with durable messages
[ https://issues.apache.org/jira/browse/ARTEMIS-1607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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
[ https://issues.apache.org/jira/browse/ARTEMIS-1607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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
[ https://issues.apache.org/jira/browse/ARTEMIS-1607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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
[ https://issues.apache.org/jira/browse/ARTEMIS-1607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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
[ https://issues.apache.org/jira/browse/ARTEMIS-1607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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
[ https://issues.apache.org/jira/browse/ARTEMIS-1607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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
[ https://issues.apache.org/jira/browse/ARTEMIS-1607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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: Fran
[jira] [Commented] (ARTEMIS-1607) OpenWire is sending responses too early with durable messages
[ https://issues.apache.org/jira/browse/ARTEMIS-1607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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
[ https://issues.apache.org/jira/browse/ARTEMIS-1607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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
[ https://issues.apache.org/jira/browse/ARTEMIS-1607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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
[ https://issues.apache.org/jira/browse/ARTEMIS-1607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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
[ https://issues.apache.org/jira/browse/ARTEMIS-1607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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)