[GitHub] activemq-artemis pull request #1687: ARTEMIS-1535 - settle delivery in same ...

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1687: ARTEMIS-1535 - settle delivery in same ...

clebertsuconic-3
GitHub user andytaylor opened a pull request:

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

    ARTEMIS-1535 - settle delivery in same lock as sending the disposition

    https://issues.apache.org/jira/browse/ARTEMIS-1535

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

    $ git pull https://github.com/andytaylor/activemq-artemis ARTEMIS-1535

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

    https://github.com/apache/activemq-artemis/pull/1687.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 #1687
   
----
commit 21e572c5816850d6c72b82d62e80114aec363a0b
Author: Andy Taylor <[hidden email]>
Date:   2017-12-05T12:12:54Z

    ARTEMIS-1535 - settle delivery in same lock as sending the disposition
   
    https://issues.apache.org/jira/browse/ARTEMIS-1535

----


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1687: ARTEMIS-1535 - settle delivery in same lock as...

clebertsuconic-3
Github user andytaylor commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1687
 
    don't merge this yet


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1687: ARTEMIS-1535 - settle delivery in same lock as...

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user tabish121 commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1687
 
    Is it true that there's nothing that will be calling the IOCallback onError method?  If that isn't the case then it seems like there's a possible case here where the Declare or Discharge is never answered as in the error case no disposition or settlement is done which would leave a client dangling awaiting a response.


---
Reply | Threaded
Open this post in threaded view
|

Re: [GitHub] activemq-artemis issue #1687: ARTEMIS-1535 - settle delivery in same lock as...

clebertsuconic
It’s cslled upon IO errors but there is also the critical listener that
would shutdown the broker before. So I’m not sure the error is very useful
On this context.

On Tue, Dec 5, 2017 at 9:53 AM tabish121 <[hidden email]> wrote:

> Github user tabish121 commented on the issue:
>
>     https://github.com/apache/activemq-artemis/pull/1687
>
>     Is it true that there's nothing that will be calling the IOCallback
> onError method?  If that isn't the case then it seems like there's a
> possible case here where the Declare or Discharge is never answered as in
> the error case no disposition or settlement is done which would leave a
> client dangling awaiting a response.
>
>
> ---
>
--
Clebert Suconic
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1687: ARTEMIS-1535 - settle delivery in same lock as...

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user andytaylor commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1687
 
    @clebertsuconic what do you think, should we do something in onError at least fro discharge?


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1687: ARTEMIS-1535 - settle delivery in same lock as...

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1687
 
    @andytaylor are you in a hurry to merge this? if so, I will take a look later today.
   
   
    But usually onError is not very useful. I will need to look at the solution closely.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1687: ARTEMIS-1535 - settle delivery in same lock as...

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1687
 
    @andytaylor @tabish121 onError isn't needed on this case... IOCriticalListener would kick in before (which is kicked after that method somewhere through the native interface on AIO).
   
   
    @andytaylor I think your changes here are fine. It would be nice to see a test.. but it's fine..
   
   
    I think it's ok to merge this, assuming you ran the AMQP tests on the testsuite.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1687: ARTEMIS-1535 - settle delivery in same ...

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user asfgit closed the pull request at:

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


---