You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by "Rajith Attapattu (Created) (JIRA)" <ji...@apache.org> on 2011/11/09 22:23:52 UTC
[jira] [Created] (QPID-3604) If the connection is stopped the
client should release all it's messages in the prefetch buffer
If the connection is stopped the client should release all it's messages in the prefetch buffer
-----------------------------------------------------------------------------------------------
Key: QPID-3604
URL: https://issues.apache.org/jira/browse/QPID-3604
Project: Qpid
Issue Type: Bug
Components: Java Client
Affects Versions: 0.14
Reporter: Rajith Attapattu
Assignee: Rajith Attapattu
Fix For: 0.15
When connection.stop() is called, the JMS client should release all it's messages in the prefetch buffer.
For all we know, the connection may never be started (depending on application logic) and those messages will be stuck on the prefetch buffer. Releasing it will allow another consumer to get them (in the case of a shared queue case).
Another less severe but nevertheless an undesirable side affect of this is the client getting more messages than required by the capacity or prefetch arguments. See QPID-3602
This may not be a big issue if the client is prefetching a few messages, but if prefetching something like 5000 messages, this could potentially cause a lethal spike in the clients memory usage.
Even in low capacity/prefetch values, if the messages are large (say in the mega byte range) this could potentially put the client under memory pressure.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3604) If the connection is stopped the
client should release all it's messages in the prefetch buffer
Posted by "Rajith Attapattu (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13197009#comment-13197009 ]
Rajith Attapattu commented on QPID-3604:
----------------------------------------
Robbie, I've now changed the order and removed the lock.
All tests pass on both the cpp profile and the java 0-10 profile.
> If the connection is stopped the client should release all it's messages in the prefetch buffer
> -----------------------------------------------------------------------------------------------
>
> Key: QPID-3604
> URL: https://issues.apache.org/jira/browse/QPID-3604
> Project: Qpid
> Issue Type: Bug
> Components: Java Client
> Affects Versions: 0.14
> Reporter: Rajith Attapattu
> Assignee: Rajith Attapattu
> Fix For: 0.15
>
>
> When connection.stop() is called, the JMS client should release all it's messages in the prefetch buffer.
> For all we know, the connection may never be started (depending on application logic) and those messages will be stuck on the prefetch buffer. Releasing it will allow another consumer to get them (in the case of a shared queue case).
> Another less severe but nevertheless an undesirable side affect of this is the client getting more messages than required by the capacity or prefetch arguments. See QPID-3602
> This may not be a big issue if the client is prefetching a few messages, but if prefetching something like 5000 messages, this could potentially cause a lethal spike in the clients memory usage.
> Even in low capacity/prefetch values, if the messages are large (say in the mega byte range) this could potentially put the client under memory pressure.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3604) If the connection is stopped the
client should release all it's messages in the prefetch buffer
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13152384#comment-13152384 ]
jiraposter@reviews.apache.org commented on QPID-3604:
-----------------------------------------------------
bq. On 2011-11-16 09:38:21, Keith Wall wrote:
bq. > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java, line 128
bq. > <https://reviews.apache.org/r/2832/diff/1/?file=58393#file58393line128>
bq. >
bq. > Could this make use of AMQSession#rejectMessage?
bq. >
bq. > I wonder also if this logic sit better in AMQSession#notifyConsumer(). It already rejects messages if the consumer is closed. Could it not also reject messages if the connection is no longer started?
bq.
bq. rajith attapattu wrote:
bq. Keith if you look at the rejectMessage method, it sets the redelivery option. In this case we should not be setting the redelivery option bcos the the application did not even see the message.
bq.
bq. I think we need to make a clear distinction btw reject and this case. If we are rejecting a message then we need to set redelivery. In other words the application had a look at it but decided not to use it. However in JMS you can't reject a message. So I'm not sure if setting redelivery in the rejectMessage is correct either.
bq.
bq. IMO the only time we should mark a message redelivered is when the application has seen a message but has not yet acknowledged. Ex consuming a bunch of messages in CLIENT_ACK and closing the consumer without acking any of the messages.
bq.
bq. Messages in the prefetch buffer should not be marked redelivered. I see there a few places where the rejectMessage method being used, and I don't think this is correct. Ex when we set a MessageListener we remove all messages in the internal queue and release them by setting the redelivery option.
bq.
bq. rajith attapattu wrote:
bq. Actually disregard the above comment. I totally forgot that the broker will mark all released messages as redelivered. So what the client sets doesn't really matter.
bq.
bq.
bq. Gordon Sim wrote:
bq. Re: "the broker will mark all released messages as redelivered. So what the client sets doesn't really matter."
bq.
bq. That is not the case. The broker does what the client tells it to via the set-redelivered field of the message-release command.
Gordon is correct. So in that case Keith we will have to have re-evaluate the way we set the REDELIVERY flag.
For the time being I prefer to have a separate release method.
- rajith
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2832/#review3294
-----------------------------------------------------------
On 2011-11-15 15:36:36, rajith attapattu wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2832/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-11-15 15:36:36)
bq.
bq.
bq. Review request for qpid, Gordon Sim, Robbie Gemmell, Weston Price, and Oleksandr Rudyy.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. This attempts to fix one of the issues related to the handling of Message credits. See QPID-3602 for an overall picture of the various issues.
bq.
bq. This particular patch does the following.
bq. 1. When the connection is stopped, it sends message.stop() & releases all messages in the prefetch buffer.
bq. 2. It will also release any messages (that were in flight) that comes after the connection is stopped. (*)
bq.
bq. (*) This interferes with the immediate_prefetch feature. However I don't know if immediate prefetch is really required in the 0-10 path.
bq.
bq. As always comments, suggestions & criticisms are equally welcomed.
bq.
bq.
bq. This addresses bug QPID-3604.
bq. https://issues.apache.org/jira/browse/QPID-3604
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/prefetch/PrefetchBehaviourTest.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java 1202228
bq.
bq. Diff: https://reviews.apache.org/r/2832/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. See PrefetchBehaviourTest#testConnectionStop for more details.
bq.
bq.
bq. Thanks,
bq.
bq. rajith
bq.
bq.
> If the connection is stopped the client should release all it's messages in the prefetch buffer
> -----------------------------------------------------------------------------------------------
>
> Key: QPID-3604
> URL: https://issues.apache.org/jira/browse/QPID-3604
> Project: Qpid
> Issue Type: Bug
> Components: Java Client
> Affects Versions: 0.14
> Reporter: Rajith Attapattu
> Assignee: Rajith Attapattu
> Fix For: 0.15
>
>
> When connection.stop() is called, the JMS client should release all it's messages in the prefetch buffer.
> For all we know, the connection may never be started (depending on application logic) and those messages will be stuck on the prefetch buffer. Releasing it will allow another consumer to get them (in the case of a shared queue case).
> Another less severe but nevertheless an undesirable side affect of this is the client getting more messages than required by the capacity or prefetch arguments. See QPID-3602
> This may not be a big issue if the client is prefetching a few messages, but if prefetching something like 5000 messages, this could potentially cause a lethal spike in the clients memory usage.
> Even in low capacity/prefetch values, if the messages are large (say in the mega byte range) this could potentially put the client under memory pressure.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3604) If the connection is stopped the
client should release all it's messages in the prefetch buffer
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13151584#comment-13151584 ]
jiraposter@reviews.apache.org commented on QPID-3604:
-----------------------------------------------------
bq. On 2011-11-16 09:38:21, Keith Wall wrote:
bq. > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java, line 128
bq. > <https://reviews.apache.org/r/2832/diff/1/?file=58393#file58393line128>
bq. >
bq. > Could this make use of AMQSession#rejectMessage?
bq. >
bq. > I wonder also if this logic sit better in AMQSession#notifyConsumer(). It already rejects messages if the consumer is closed. Could it not also reject messages if the connection is no longer started?
bq.
bq. rajith attapattu wrote:
bq. Keith if you look at the rejectMessage method, it sets the redelivery option. In this case we should not be setting the redelivery option bcos the the application did not even see the message.
bq.
bq. I think we need to make a clear distinction btw reject and this case. If we are rejecting a message then we need to set redelivery. In other words the application had a look at it but decided not to use it. However in JMS you can't reject a message. So I'm not sure if setting redelivery in the rejectMessage is correct either.
bq.
bq. IMO the only time we should mark a message redelivered is when the application has seen a message but has not yet acknowledged. Ex consuming a bunch of messages in CLIENT_ACK and closing the consumer without acking any of the messages.
bq.
bq. Messages in the prefetch buffer should not be marked redelivered. I see there a few places where the rejectMessage method being used, and I don't think this is correct. Ex when we set a MessageListener we remove all messages in the internal queue and release them by setting the redelivery option.
Actually disregard the above comment. I totally forgot that the broker will mark all released messages as redelivered. So what the client sets doesn't really matter.
- rajith
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2832/#review3294
-----------------------------------------------------------
On 2011-11-15 15:36:36, rajith attapattu wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2832/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-11-15 15:36:36)
bq.
bq.
bq. Review request for qpid, Gordon Sim, Robbie Gemmell, Weston Price, and Oleksandr Rudyy.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. This attempts to fix one of the issues related to the handling of Message credits. See QPID-3602 for an overall picture of the various issues.
bq.
bq. This particular patch does the following.
bq. 1. When the connection is stopped, it sends message.stop() & releases all messages in the prefetch buffer.
bq. 2. It will also release any messages (that were in flight) that comes after the connection is stopped. (*)
bq.
bq. (*) This interferes with the immediate_prefetch feature. However I don't know if immediate prefetch is really required in the 0-10 path.
bq.
bq. As always comments, suggestions & criticisms are equally welcomed.
bq.
bq.
bq. This addresses bug QPID-3604.
bq. https://issues.apache.org/jira/browse/QPID-3604
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/prefetch/PrefetchBehaviourTest.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java 1202228
bq.
bq. Diff: https://reviews.apache.org/r/2832/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. See PrefetchBehaviourTest#testConnectionStop for more details.
bq.
bq.
bq. Thanks,
bq.
bq. rajith
bq.
bq.
> If the connection is stopped the client should release all it's messages in the prefetch buffer
> -----------------------------------------------------------------------------------------------
>
> Key: QPID-3604
> URL: https://issues.apache.org/jira/browse/QPID-3604
> Project: Qpid
> Issue Type: Bug
> Components: Java Client
> Affects Versions: 0.14
> Reporter: Rajith Attapattu
> Assignee: Rajith Attapattu
> Fix For: 0.15
>
>
> When connection.stop() is called, the JMS client should release all it's messages in the prefetch buffer.
> For all we know, the connection may never be started (depending on application logic) and those messages will be stuck on the prefetch buffer. Releasing it will allow another consumer to get them (in the case of a shared queue case).
> Another less severe but nevertheless an undesirable side affect of this is the client getting more messages than required by the capacity or prefetch arguments. See QPID-3602
> This may not be a big issue if the client is prefetching a few messages, but if prefetching something like 5000 messages, this could potentially cause a lethal spike in the clients memory usage.
> Even in low capacity/prefetch values, if the messages are large (say in the mega byte range) this could potentially put the client under memory pressure.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3604) If the connection is stopped the
client should release all it's messages in the prefetch buffer
Posted by "Rajith Attapattu (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13193077#comment-13193077 ]
Rajith Attapattu commented on QPID-3604:
----------------------------------------
Sorry I forgot to exclude this test from the non 0-10 profiles.
I only made a fix to the 0-10 code path.
> If the connection is stopped the client should release all it's messages in the prefetch buffer
> -----------------------------------------------------------------------------------------------
>
> Key: QPID-3604
> URL: https://issues.apache.org/jira/browse/QPID-3604
> Project: Qpid
> Issue Type: Bug
> Components: Java Client
> Affects Versions: 0.14
> Reporter: Rajith Attapattu
> Assignee: Rajith Attapattu
> Fix For: 0.15
>
>
> When connection.stop() is called, the JMS client should release all it's messages in the prefetch buffer.
> For all we know, the connection may never be started (depending on application logic) and those messages will be stuck on the prefetch buffer. Releasing it will allow another consumer to get them (in the case of a shared queue case).
> Another less severe but nevertheless an undesirable side affect of this is the client getting more messages than required by the capacity or prefetch arguments. See QPID-3602
> This may not be a big issue if the client is prefetching a few messages, but if prefetching something like 5000 messages, this could potentially cause a lethal spike in the clients memory usage.
> Even in low capacity/prefetch values, if the messages are large (say in the mega byte range) this could potentially put the client under memory pressure.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3604) If the connection is stopped the
client should release all it's messages in the prefetch buffer
Posted by "Rajith Attapattu (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13184108#comment-13184108 ]
Rajith Attapattu commented on QPID-3604:
----------------------------------------
Agreed. I'm in the process of reversing it.
However I don't see some of the failures observed in jenkins on my local copy.
Until I investigated this issue further, it's best I revert it.
Thanks for your patience.
> If the connection is stopped the client should release all it's messages in the prefetch buffer
> -----------------------------------------------------------------------------------------------
>
> Key: QPID-3604
> URL: https://issues.apache.org/jira/browse/QPID-3604
> Project: Qpid
> Issue Type: Bug
> Components: Java Client
> Affects Versions: 0.14
> Reporter: Rajith Attapattu
> Assignee: Rajith Attapattu
> Fix For: 0.15
>
>
> When connection.stop() is called, the JMS client should release all it's messages in the prefetch buffer.
> For all we know, the connection may never be started (depending on application logic) and those messages will be stuck on the prefetch buffer. Releasing it will allow another consumer to get them (in the case of a shared queue case).
> Another less severe but nevertheless an undesirable side affect of this is the client getting more messages than required by the capacity or prefetch arguments. See QPID-3602
> This may not be a big issue if the client is prefetching a few messages, but if prefetching something like 5000 messages, this could potentially cause a lethal spike in the clients memory usage.
> Even in low capacity/prefetch values, if the messages are large (say in the mega byte range) this could potentially put the client under memory pressure.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
[jira] [Updated] (QPID-3604) If the connection is stopped the
client should release all it's messages in the prefetch buffer
Posted by "Robbie Gemmell (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3604?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Robbie Gemmell updated QPID-3604:
---------------------------------
Labels: possibly_complete (was: )
> If the connection is stopped the client should release all it's messages in the prefetch buffer
> -----------------------------------------------------------------------------------------------
>
> Key: QPID-3604
> URL: https://issues.apache.org/jira/browse/QPID-3604
> Project: Qpid
> Issue Type: Bug
> Components: Java Client
> Affects Versions: 0.14
> Reporter: Rajith Attapattu
> Assignee: Rajith Attapattu
> Labels: possibly_complete
> Fix For: 0.15
>
>
> When connection.stop() is called, the JMS client should release all it's messages in the prefetch buffer.
> For all we know, the connection may never be started (depending on application logic) and those messages will be stuck on the prefetch buffer. Releasing it will allow another consumer to get them (in the case of a shared queue case).
> Another less severe but nevertheless an undesirable side affect of this is the client getting more messages than required by the capacity or prefetch arguments. See QPID-3602
> This may not be a big issue if the client is prefetching a few messages, but if prefetching something like 5000 messages, this could potentially cause a lethal spike in the clients memory usage.
> Even in low capacity/prefetch values, if the messages are large (say in the mega byte range) this could potentially put the client under memory pressure.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3604) If the connection is stopped the
client should release all it's messages in the prefetch buffer
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13152385#comment-13152385 ]
jiraposter@reviews.apache.org commented on QPID-3604:
-----------------------------------------------------
bq. On 2011-11-15 16:41:28, Gordon Sim wrote:
bq. > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java, line 126
bq. > <https://reviews.apache.org/r/2832/diff/1/?file=58393#file58393line126>
bq. >
bq. > What case(s) is this code required for? You are releasing a message you have just received, right? When is that required?
bq.
bq. rajith attapattu wrote:
bq. See the above for an explanation for why this is needed.
bq.
bq. Gordon Sim wrote:
bq. You mean this is here because of the lack of synchronization with the dispatcher thread? If so that seems a little nasty to me... anyway to do this more cleanly?
bq.
bq. rajith attapattu wrote:
bq. That is precisely the reason. This also makes the sync call redundant. I started with the sync() and realized that it wasn't sufficient, hence added this.
bq. As explained above, I'm not sure if there is a reasonable way to synchronize with the message delivery thread.
bq.
bq. One possible approach might be is to do something like the syncDispatchQueue() method. Where we push a certain marker message into the queue and then we get that we know there are no more messages in the pipeline. But I'm concerned about the safety and feasibility of such an approach.
bq.
bq. Robbie I believe is one person who have looked at this code more extensively in the last little while. So waiting to hear from him about his ideas as well. I'm open to suggestions on this area. Lets see if we can collectively figure out a better solution.
bq.
bq. Robbie Gemmell wrote:
bq. (just noticed I didnt press publish yesterday morning on this...oops)
bq.
bq. > One possible approach might be is to do something like the syncDispatchQueue() method.
bq.
bq. This is exactly the comment I was going to make. Its not the nicest thing in the world, but I think its better than holding yet more locks. Ensuring that the broker has finished sending you messages on the stopped session and then having the Dispatcher do the work and tell you that there isnt anything left to deliver seems the easiest to reason about, and we already do that elsewhere so reusing the idea seems like the way to go.
Let me work this out and see how it goes.
- rajith
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2832/#review3264
-----------------------------------------------------------
On 2011-11-15 15:36:36, rajith attapattu wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2832/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-11-15 15:36:36)
bq.
bq.
bq. Review request for qpid, Gordon Sim, Robbie Gemmell, Weston Price, and Oleksandr Rudyy.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. This attempts to fix one of the issues related to the handling of Message credits. See QPID-3602 for an overall picture of the various issues.
bq.
bq. This particular patch does the following.
bq. 1. When the connection is stopped, it sends message.stop() & releases all messages in the prefetch buffer.
bq. 2. It will also release any messages (that were in flight) that comes after the connection is stopped. (*)
bq.
bq. (*) This interferes with the immediate_prefetch feature. However I don't know if immediate prefetch is really required in the 0-10 path.
bq.
bq. As always comments, suggestions & criticisms are equally welcomed.
bq.
bq.
bq. This addresses bug QPID-3604.
bq. https://issues.apache.org/jira/browse/QPID-3604
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/prefetch/PrefetchBehaviourTest.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java 1202228
bq.
bq. Diff: https://reviews.apache.org/r/2832/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. See PrefetchBehaviourTest#testConnectionStop for more details.
bq.
bq.
bq. Thanks,
bq.
bq. rajith
bq.
bq.
> If the connection is stopped the client should release all it's messages in the prefetch buffer
> -----------------------------------------------------------------------------------------------
>
> Key: QPID-3604
> URL: https://issues.apache.org/jira/browse/QPID-3604
> Project: Qpid
> Issue Type: Bug
> Components: Java Client
> Affects Versions: 0.14
> Reporter: Rajith Attapattu
> Assignee: Rajith Attapattu
> Fix For: 0.15
>
>
> When connection.stop() is called, the JMS client should release all it's messages in the prefetch buffer.
> For all we know, the connection may never be started (depending on application logic) and those messages will be stuck on the prefetch buffer. Releasing it will allow another consumer to get them (in the case of a shared queue case).
> Another less severe but nevertheless an undesirable side affect of this is the client getting more messages than required by the capacity or prefetch arguments. See QPID-3602
> This may not be a big issue if the client is prefetching a few messages, but if prefetching something like 5000 messages, this could potentially cause a lethal spike in the clients memory usage.
> Even in low capacity/prefetch values, if the messages are large (say in the mega byte range) this could potentially put the client under memory pressure.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3604) If the connection is stopped the
client should release all it's messages in the prefetch buffer
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13150560#comment-13150560 ]
jiraposter@reviews.apache.org commented on QPID-3604:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2832/
-----------------------------------------------------------
Review request for qpid, Gordon Sim, Robbie Gemmell, Weston Price, and Oleksandr Rudyy.
Summary
-------
This attempts to fix one of the issues related to the handling of Message credits. See QPID-3602 for an overall picture of the various issues.
This particular patch does the following.
1. When the connection is stopped, it sends message.stop() & releases all messages in the prefetch buffer.
2. It will also release any messages (that were in flight) that comes after the connection is stopped. (*)
(*) This interferes with the immediate_prefetch feature. However I don't know if immediate prefetch is really required in the 0-10 path.
As always comments, suggestions & criticisms are equally welcomed.
This addresses bug QPID-3604.
https://issues.apache.org/jira/browse/QPID-3604
Diffs
-----
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java 1202228
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java 1202228
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/prefetch/PrefetchBehaviourTest.java 1202228
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java 1202228
Diff: https://reviews.apache.org/r/2832/diff
Testing
-------
See PrefetchBehaviourTest#testConnectionStop for more details.
Thanks,
rajith
> If the connection is stopped the client should release all it's messages in the prefetch buffer
> -----------------------------------------------------------------------------------------------
>
> Key: QPID-3604
> URL: https://issues.apache.org/jira/browse/QPID-3604
> Project: Qpid
> Issue Type: Bug
> Components: Java Client
> Affects Versions: 0.14
> Reporter: Rajith Attapattu
> Assignee: Rajith Attapattu
> Fix For: 0.15
>
>
> When connection.stop() is called, the JMS client should release all it's messages in the prefetch buffer.
> For all we know, the connection may never be started (depending on application logic) and those messages will be stuck on the prefetch buffer. Releasing it will allow another consumer to get them (in the case of a shared queue case).
> Another less severe but nevertheless an undesirable side affect of this is the client getting more messages than required by the capacity or prefetch arguments. See QPID-3602
> This may not be a big issue if the client is prefetching a few messages, but if prefetching something like 5000 messages, this could potentially cause a lethal spike in the clients memory usage.
> Even in low capacity/prefetch values, if the messages are large (say in the mega byte range) this could potentially put the client under memory pressure.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3604) If the connection is stopped the
client should release all it's messages in the prefetch buffer
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13151958#comment-13151958 ]
jiraposter@reviews.apache.org commented on QPID-3604:
-----------------------------------------------------
bq. On 2011-11-15 16:41:28, Gordon Sim wrote:
bq. > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java, line 126
bq. > <https://reviews.apache.org/r/2832/diff/1/?file=58393#file58393line126>
bq. >
bq. > What case(s) is this code required for? You are releasing a message you have just received, right? When is that required?
bq.
bq. rajith attapattu wrote:
bq. See the above for an explanation for why this is needed.
bq.
bq. Gordon Sim wrote:
bq. You mean this is here because of the lack of synchronization with the dispatcher thread? If so that seems a little nasty to me... anyway to do this more cleanly?
bq.
bq. rajith attapattu wrote:
bq. That is precisely the reason. This also makes the sync call redundant. I started with the sync() and realized that it wasn't sufficient, hence added this.
bq. As explained above, I'm not sure if there is a reasonable way to synchronize with the message delivery thread.
bq.
bq. One possible approach might be is to do something like the syncDispatchQueue() method. Where we push a certain marker message into the queue and then we get that we know there are no more messages in the pipeline. But I'm concerned about the safety and feasibility of such an approach.
bq.
bq. Robbie I believe is one person who have looked at this code more extensively in the last little while. So waiting to hear from him about his ideas as well. I'm open to suggestions on this area. Lets see if we can collectively figure out a better solution.
(just noticed I didnt press publish yesterday morning on this...oops)
bq. One possible approach might be is to do something like the syncDispatchQueue() method.
This is exactly the comment I was going to make. Its not the nicest thing in the world, but I think its better than holding yet more locks. Ensuring that the broker has finished sending you messages on the stopped session and then having the Dispatcher do the work and tell you that there isnt anything left to deliver seems the easiest to reason about, and we already do that elsewhere so reusing the idea seems like the way to go.
- Robbie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2832/#review3264
-----------------------------------------------------------
On 2011-11-15 15:36:36, rajith attapattu wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2832/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-11-15 15:36:36)
bq.
bq.
bq. Review request for qpid, Gordon Sim, Robbie Gemmell, Weston Price, and Oleksandr Rudyy.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. This attempts to fix one of the issues related to the handling of Message credits. See QPID-3602 for an overall picture of the various issues.
bq.
bq. This particular patch does the following.
bq. 1. When the connection is stopped, it sends message.stop() & releases all messages in the prefetch buffer.
bq. 2. It will also release any messages (that were in flight) that comes after the connection is stopped. (*)
bq.
bq. (*) This interferes with the immediate_prefetch feature. However I don't know if immediate prefetch is really required in the 0-10 path.
bq.
bq. As always comments, suggestions & criticisms are equally welcomed.
bq.
bq.
bq. This addresses bug QPID-3604.
bq. https://issues.apache.org/jira/browse/QPID-3604
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/prefetch/PrefetchBehaviourTest.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java 1202228
bq.
bq. Diff: https://reviews.apache.org/r/2832/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. See PrefetchBehaviourTest#testConnectionStop for more details.
bq.
bq.
bq. Thanks,
bq.
bq. rajith
bq.
bq.
> If the connection is stopped the client should release all it's messages in the prefetch buffer
> -----------------------------------------------------------------------------------------------
>
> Key: QPID-3604
> URL: https://issues.apache.org/jira/browse/QPID-3604
> Project: Qpid
> Issue Type: Bug
> Components: Java Client
> Affects Versions: 0.14
> Reporter: Rajith Attapattu
> Assignee: Rajith Attapattu
> Fix For: 0.15
>
>
> When connection.stop() is called, the JMS client should release all it's messages in the prefetch buffer.
> For all we know, the connection may never be started (depending on application logic) and those messages will be stuck on the prefetch buffer. Releasing it will allow another consumer to get them (in the case of a shared queue case).
> Another less severe but nevertheless an undesirable side affect of this is the client getting more messages than required by the capacity or prefetch arguments. See QPID-3602
> This may not be a big issue if the client is prefetching a few messages, but if prefetching something like 5000 messages, this could potentially cause a lethal spike in the clients memory usage.
> Even in low capacity/prefetch values, if the messages are large (say in the mega byte range) this could potentially put the client under memory pressure.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3604) If the connection is stopped the
client should release all it's messages in the prefetch buffer
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13150617#comment-13150617 ]
jiraposter@reviews.apache.org commented on QPID-3604:
-----------------------------------------------------
bq. On 2011-11-15 16:41:28, Gordon Sim wrote:
bq. > I don't have a clear enough mental picture of the code you are modifying, so am really only able to ask some questions...
Thanks a lot for taking the time to review. I really appreciate it.
bq. On 2011-11-15 16:41:28, Gordon Sim wrote:
bq. > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java, line 787
bq. > <https://reviews.apache.org/r/2832/diff/1/?file=58392#file58392line787>
bq. >
bq. > Are there any other locks acquired as part of the block here? If so are there any lock ordering issues where you could be introducing a deadlock?
Not that I could think of. The message-delivery-lock is taken to ensure that no messages are being served while we start pulling them out of the queue.
In my tests so far, I haven't encountered any issues. However I plan to have more manual tests - ex. Trying to stop the connection while the message consumers are in full flight.
bq. On 2011-11-15 16:41:28, Gordon Sim wrote:
bq. > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java, line 796
bq. > <https://reviews.apache.org/r/2832/diff/1/?file=58392#file58392line796>
bq. >
bq. > You are syncing here while holding the delivery lock, could that cause any problems?
So far I haven't encountered any issues. However things like failover, session exceptions etc..could cause issues. I'm planning more thorough longer running tests.
Another thing I am considering is to not use a sync() at all. I'm not quite convinced that it's of much value here.
I've noticed that the client continues to get messages into it's queue even after the code returns from the sync call. Hence the code snippet to release any messages received after the connection is stopped. I was expecting the brokers response to the sync command to be received after the client has got all the messages that were in flight. So after I sync I could just release the messages in the queue and be done with it. But that's not the case.
It seems that the dispatcher thread takes a bit of time to process the UnprocessedMessages into the correct JMSMessages and put them onto the queue. So the sync() really doesn't add much value here.
bq. On 2011-11-15 16:41:28, Gordon Sim wrote:
bq. > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java, line 126
bq. > <https://reviews.apache.org/r/2832/diff/1/?file=58393#file58393line126>
bq. >
bq. > What case(s) is this code required for? You are releasing a message you have just received, right? When is that required?
See the above for an explanation for why this is needed.
- rajith
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2832/#review3264
-----------------------------------------------------------
On 2011-11-15 15:36:36, rajith attapattu wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2832/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-11-15 15:36:36)
bq.
bq.
bq. Review request for qpid, Gordon Sim, Robbie Gemmell, Weston Price, and Oleksandr Rudyy.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. This attempts to fix one of the issues related to the handling of Message credits. See QPID-3602 for an overall picture of the various issues.
bq.
bq. This particular patch does the following.
bq. 1. When the connection is stopped, it sends message.stop() & releases all messages in the prefetch buffer.
bq. 2. It will also release any messages (that were in flight) that comes after the connection is stopped. (*)
bq.
bq. (*) This interferes with the immediate_prefetch feature. However I don't know if immediate prefetch is really required in the 0-10 path.
bq.
bq. As always comments, suggestions & criticisms are equally welcomed.
bq.
bq.
bq. This addresses bug QPID-3604.
bq. https://issues.apache.org/jira/browse/QPID-3604
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/prefetch/PrefetchBehaviourTest.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java 1202228
bq.
bq. Diff: https://reviews.apache.org/r/2832/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. See PrefetchBehaviourTest#testConnectionStop for more details.
bq.
bq.
bq. Thanks,
bq.
bq. rajith
bq.
bq.
> If the connection is stopped the client should release all it's messages in the prefetch buffer
> -----------------------------------------------------------------------------------------------
>
> Key: QPID-3604
> URL: https://issues.apache.org/jira/browse/QPID-3604
> Project: Qpid
> Issue Type: Bug
> Components: Java Client
> Affects Versions: 0.14
> Reporter: Rajith Attapattu
> Assignee: Rajith Attapattu
> Fix For: 0.15
>
>
> When connection.stop() is called, the JMS client should release all it's messages in the prefetch buffer.
> For all we know, the connection may never be started (depending on application logic) and those messages will be stuck on the prefetch buffer. Releasing it will allow another consumer to get them (in the case of a shared queue case).
> Another less severe but nevertheless an undesirable side affect of this is the client getting more messages than required by the capacity or prefetch arguments. See QPID-3602
> This may not be a big issue if the client is prefetching a few messages, but if prefetching something like 5000 messages, this could potentially cause a lethal spike in the clients memory usage.
> Even in low capacity/prefetch values, if the messages are large (say in the mega byte range) this could potentially put the client under memory pressure.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3604) If the connection is stopped the
client should release all it's messages in the prefetch buffer
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13150596#comment-13150596 ]
jiraposter@reviews.apache.org commented on QPID-3604:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2832/#review3264
-----------------------------------------------------------
I don't have a clear enough mental picture of the code you are modifying, so am really only able to ask some questions...
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java
<https://reviews.apache.org/r/2832/#comment7291>
Are there any other locks acquired as part of the block here? If so are there any lock ordering issues where you could be introducing a deadlock?
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java
<https://reviews.apache.org/r/2832/#comment7292>
You are syncing here while holding the delivery lock, could that cause any problems?
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java
<https://reviews.apache.org/r/2832/#comment7293>
What case(s) is this code required for? You are releasing a message you have just received, right? When is that required?
- Gordon
On 2011-11-15 15:36:36, rajith attapattu wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2832/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-11-15 15:36:36)
bq.
bq.
bq. Review request for qpid, Gordon Sim, Robbie Gemmell, Weston Price, and Oleksandr Rudyy.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. This attempts to fix one of the issues related to the handling of Message credits. See QPID-3602 for an overall picture of the various issues.
bq.
bq. This particular patch does the following.
bq. 1. When the connection is stopped, it sends message.stop() & releases all messages in the prefetch buffer.
bq. 2. It will also release any messages (that were in flight) that comes after the connection is stopped. (*)
bq.
bq. (*) This interferes with the immediate_prefetch feature. However I don't know if immediate prefetch is really required in the 0-10 path.
bq.
bq. As always comments, suggestions & criticisms are equally welcomed.
bq.
bq.
bq. This addresses bug QPID-3604.
bq. https://issues.apache.org/jira/browse/QPID-3604
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/prefetch/PrefetchBehaviourTest.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java 1202228
bq.
bq. Diff: https://reviews.apache.org/r/2832/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. See PrefetchBehaviourTest#testConnectionStop for more details.
bq.
bq.
bq. Thanks,
bq.
bq. rajith
bq.
bq.
> If the connection is stopped the client should release all it's messages in the prefetch buffer
> -----------------------------------------------------------------------------------------------
>
> Key: QPID-3604
> URL: https://issues.apache.org/jira/browse/QPID-3604
> Project: Qpid
> Issue Type: Bug
> Components: Java Client
> Affects Versions: 0.14
> Reporter: Rajith Attapattu
> Assignee: Rajith Attapattu
> Fix For: 0.15
>
>
> When connection.stop() is called, the JMS client should release all it's messages in the prefetch buffer.
> For all we know, the connection may never be started (depending on application logic) and those messages will be stuck on the prefetch buffer. Releasing it will allow another consumer to get them (in the case of a shared queue case).
> Another less severe but nevertheless an undesirable side affect of this is the client getting more messages than required by the capacity or prefetch arguments. See QPID-3602
> This may not be a big issue if the client is prefetching a few messages, but if prefetching something like 5000 messages, this could potentially cause a lethal spike in the clients memory usage.
> Even in low capacity/prefetch values, if the messages are large (say in the mega byte range) this could potentially put the client under memory pressure.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3604) If the connection is stopped the
client should release all it's messages in the prefetch buffer
Posted by "Keith Wall (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13183947#comment-13183947 ]
Keith Wall commented on QPID-3604:
----------------------------------
Hi Rajith
It seems this commit is causing may failing tests for both the CPP and Java Brokers on many profiles. You can see the failures on Jenkins:
eg.
>>> org.apache.qpid.client.MessageListenerTest.testMessageListenerThrowsError 0.14 sec 1
>>> org.apache.qpid.client.failover.AddressBasedFailoverBehaviourTest.testTransactedQueueBrowserCloseWhileFailover 0.21 sec 1
>>> org.apache.qpid.client.prefetch.PrefetchBehaviourTest.testConnectionStop 1.1 sec 1
>>> org.apache.qpid.server.queue.MessageGroupQueueTest.testConsumerCloseGroupAssignment 0.11 sec 1
>>> org.apache.qpid.test.client.queue.LVQTest.testLVQQueue
Could you take a look?
> If the connection is stopped the client should release all it's messages in the prefetch buffer
> -----------------------------------------------------------------------------------------------
>
> Key: QPID-3604
> URL: https://issues.apache.org/jira/browse/QPID-3604
> Project: Qpid
> Issue Type: Bug
> Components: Java Client
> Affects Versions: 0.14
> Reporter: Rajith Attapattu
> Assignee: Rajith Attapattu
> Fix For: 0.15
>
>
> When connection.stop() is called, the JMS client should release all it's messages in the prefetch buffer.
> For all we know, the connection may never be started (depending on application logic) and those messages will be stuck on the prefetch buffer. Releasing it will allow another consumer to get them (in the case of a shared queue case).
> Another less severe but nevertheless an undesirable side affect of this is the client getting more messages than required by the capacity or prefetch arguments. See QPID-3602
> This may not be a big issue if the client is prefetching a few messages, but if prefetching something like 5000 messages, this could potentially cause a lethal spike in the clients memory usage.
> Even in low capacity/prefetch values, if the messages are large (say in the mega byte range) this could potentially put the client under memory pressure.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3604) If the connection is stopped the
client should release all it's messages in the prefetch buffer
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13151108#comment-13151108 ]
jiraposter@reviews.apache.org commented on QPID-3604:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2832/#review3294
-----------------------------------------------------------
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java
<https://reviews.apache.org/r/2832/#comment7376>
Could this make use of AMQSession#rejectMessage?
I wonder also if this logic sit better in AMQSession#notifyConsumer(). It already rejects messages if the consumer is closed. Could it not also reject messages if the connection is no longer started?
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/prefetch/PrefetchBehaviourTest.java
<https://reviews.apache.org/r/2832/#comment7374>
Could you not use the test utility method for the production of these messages?
QpidBrokerTestCase#sendMessage
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/prefetch/PrefetchBehaviourTest.java
<https://reviews.apache.org/r/2832/#comment7375>
I think using a timeout here would be preferable. IMHO we should avoid writing unit tests that can hang indefinitely in favour of those that will always fail with a useful assertion-fail.
Also typo in assertion message (once -> one).
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/prefetch/PrefetchBehaviourTest.java
<https://reviews.apache.org/r/2832/#comment7377>
You've got a couple of whitespace issues that make the patch slightly larger than need be :)
- Keith
On 2011-11-15 15:36:36, rajith attapattu wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2832/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-11-15 15:36:36)
bq.
bq.
bq. Review request for qpid, Gordon Sim, Robbie Gemmell, Weston Price, and Oleksandr Rudyy.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. This attempts to fix one of the issues related to the handling of Message credits. See QPID-3602 for an overall picture of the various issues.
bq.
bq. This particular patch does the following.
bq. 1. When the connection is stopped, it sends message.stop() & releases all messages in the prefetch buffer.
bq. 2. It will also release any messages (that were in flight) that comes after the connection is stopped. (*)
bq.
bq. (*) This interferes with the immediate_prefetch feature. However I don't know if immediate prefetch is really required in the 0-10 path.
bq.
bq. As always comments, suggestions & criticisms are equally welcomed.
bq.
bq.
bq. This addresses bug QPID-3604.
bq. https://issues.apache.org/jira/browse/QPID-3604
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/prefetch/PrefetchBehaviourTest.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java 1202228
bq.
bq. Diff: https://reviews.apache.org/r/2832/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. See PrefetchBehaviourTest#testConnectionStop for more details.
bq.
bq.
bq. Thanks,
bq.
bq. rajith
bq.
bq.
> If the connection is stopped the client should release all it's messages in the prefetch buffer
> -----------------------------------------------------------------------------------------------
>
> Key: QPID-3604
> URL: https://issues.apache.org/jira/browse/QPID-3604
> Project: Qpid
> Issue Type: Bug
> Components: Java Client
> Affects Versions: 0.14
> Reporter: Rajith Attapattu
> Assignee: Rajith Attapattu
> Fix For: 0.15
>
>
> When connection.stop() is called, the JMS client should release all it's messages in the prefetch buffer.
> For all we know, the connection may never be started (depending on application logic) and those messages will be stuck on the prefetch buffer. Releasing it will allow another consumer to get them (in the case of a shared queue case).
> Another less severe but nevertheless an undesirable side affect of this is the client getting more messages than required by the capacity or prefetch arguments. See QPID-3602
> This may not be a big issue if the client is prefetching a few messages, but if prefetching something like 5000 messages, this could potentially cause a lethal spike in the clients memory usage.
> Even in low capacity/prefetch values, if the messages are large (say in the mega byte range) this could potentially put the client under memory pressure.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
[jira] [Issue Comment Edited] (QPID-3604) If the connection is
stopped the client should release all it's messages in the prefetch buffer
Posted by "Robbie Gemmell (Issue Comment Edited) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13193128#comment-13193128 ]
Robbie Gemmell edited comment on QPID-3604 at 1/25/12 5:13 PM:
---------------------------------------------------------------
I'm still not entirely convinced of the need to hold the lock. The only seeming reason to do so would be to stop messages still in the dispatch queue (or any arriving after the stop completes, which shouldnt occur) from being moved by the Dispatcher into the consumer queues while you are draining them. If that were the case then it doesnt seem sufficient, because as soon as you think you have drained the consumer queues and move on to release that lock, any such messages still remaining in the dispatch queue can then potentially be put into the consumer queues before you hit the 'drainDispatchQueue' invocation.
Messages shouldnt arrive after stop() returns, at which point the dispatch queue can be drained to ensure no more messages are able to be delivered into the consumer queues, and after that the consumer queues can be safely drained without the lock because the Dispatcher has nothing to do.
{noformat}
+ @Override
+ void stop() throws AMQException
+ {
+ super.stop();
+ synchronized (getMessageDeliveryLock())
+ {
+ for (BasicMessageConsumer consumer : _consumers.values())
+ {
+ List<Long> tags = consumer.drainReceiverQueueAndRetrieveDeliveryTags();
+ _prefetchedMessageTags.addAll(tags);
+ }
+ }
+ _usingDispatcherForCleanup = true;
+ drainDispatchQueue();
+ _usingDispatcherForCleanup = false;
{noformat}
was (Author: gemmellr):
I'm still not entirely convinced of the need to hold the lock. The only seeming reason to do so would be to stop messages still in the dispatch queue (or any arriving after the stop completes, which shouldnt occur) from being moved by the Dispatcher into the consumer queues while you are draining them. If that were the case then it doesnt seem sufficient, because as soon as you think you have drained the consumer queues and move on to release that lock, any such messages still remaining in the dispatch queue can then potentially be put into the consumer queues before you hit the 'drainDispatchQueue' invocation.
Messages shouldnt arrive after stop() returns, at which point the dispatch queue can be drained to ensure no more messages are able to be delivered into the consumer queues, and after that the consumer queues can be safely drained without the lock because the Dispatcher has nothing to do.
+ @Override
+ void stop() throws AMQException
+ {
+ super.stop();
+ synchronized (getMessageDeliveryLock())
+ {
+ for (BasicMessageConsumer consumer : _consumers.values())
+ {
+ List<Long> tags = consumer.drainReceiverQueueAndRetrieveDeliveryTags();
+ _prefetchedMessageTags.addAll(tags);
+ }
+ }
+ _usingDispatcherForCleanup = true;
+ drainDispatchQueue();
+ _usingDispatcherForCleanup = false;
> If the connection is stopped the client should release all it's messages in the prefetch buffer
> -----------------------------------------------------------------------------------------------
>
> Key: QPID-3604
> URL: https://issues.apache.org/jira/browse/QPID-3604
> Project: Qpid
> Issue Type: Bug
> Components: Java Client
> Affects Versions: 0.14
> Reporter: Rajith Attapattu
> Assignee: Rajith Attapattu
> Fix For: 0.15
>
>
> When connection.stop() is called, the JMS client should release all it's messages in the prefetch buffer.
> For all we know, the connection may never be started (depending on application logic) and those messages will be stuck on the prefetch buffer. Releasing it will allow another consumer to get them (in the case of a shared queue case).
> Another less severe but nevertheless an undesirable side affect of this is the client getting more messages than required by the capacity or prefetch arguments. See QPID-3602
> This may not be a big issue if the client is prefetching a few messages, but if prefetching something like 5000 messages, this could potentially cause a lethal spike in the clients memory usage.
> Even in low capacity/prefetch values, if the messages are large (say in the mega byte range) this could potentially put the client under memory pressure.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3604) If the connection is stopped the
client should release all it's messages in the prefetch buffer
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13151278#comment-13151278 ]
jiraposter@reviews.apache.org commented on QPID-3604:
-----------------------------------------------------
bq. On 2011-11-16 09:38:21, Keith Wall wrote:
bq. > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java, line 128
bq. > <https://reviews.apache.org/r/2832/diff/1/?file=58393#file58393line128>
bq. >
bq. > Could this make use of AMQSession#rejectMessage?
bq. >
bq. > I wonder also if this logic sit better in AMQSession#notifyConsumer(). It already rejects messages if the consumer is closed. Could it not also reject messages if the connection is no longer started?
Keith if you look at the rejectMessage method, it sets the redelivery option. In this case we should not be setting the redelivery option bcos the the application did not even see the message.
I think we need to make a clear distinction btw reject and this case. If we are rejecting a message then we need to set redelivery. In other words the application had a look at it but decided not to use it. However in JMS you can't reject a message. So I'm not sure if setting redelivery in the rejectMessage is correct either.
IMO the only time we should mark a message redelivered is when the application has seen a message but has not yet acknowledged. Ex consuming a bunch of messages in CLIENT_ACK and closing the consumer without acking any of the messages.
Messages in the prefetch buffer should not be marked redelivered. I see there a few places where the rejectMessage method being used, and I don't think this is correct. Ex when we set a MessageListener we remove all messages in the internal queue and release them by setting the redelivery option.
bq. On 2011-11-16 09:38:21, Keith Wall wrote:
bq. > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/prefetch/PrefetchBehaviourTest.java, line 135
bq. > <https://reviews.apache.org/r/2832/diff/1/?file=58394#file58394line135>
bq. >
bq. > Could you not use the test utility method for the production of these messages?
bq. > QpidBrokerTestCase#sendMessage
that could be done.
bq. On 2011-11-16 09:38:21, Keith Wall wrote:
bq. > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/prefetch/PrefetchBehaviourTest.java, line 140
bq. > <https://reviews.apache.org/r/2832/diff/1/?file=58394#file58394line140>
bq. >
bq. > I think using a timeout here would be preferable. IMHO we should avoid writing unit tests that can hang indefinitely in favour of those that will always fail with a useful assertion-fail.
bq. >
bq. > Also typo in assertion message (once -> one).
Ah, I did have a timeout and the typo was correct (as it was pointed out by someone else too), it seems like I generated the patches before these changes.
Definitely we should always use a timeout. This will be corrected.
bq. On 2011-11-16 09:38:21, Keith Wall wrote:
bq. > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/prefetch/PrefetchBehaviourTest.java, line 152
bq. > <https://reviews.apache.org/r/2832/diff/1/?file=58394#file58394line152>
bq. >
bq. > You've got a couple of whitespace issues that make the patch slightly larger than need be :)
It seems eclipse is doing this. I tried using the AnyEdit plugging and it made matters worse. I will try to edit this on the command line before I do the final commit :)
- rajith
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2832/#review3294
-----------------------------------------------------------
On 2011-11-15 15:36:36, rajith attapattu wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2832/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-11-15 15:36:36)
bq.
bq.
bq. Review request for qpid, Gordon Sim, Robbie Gemmell, Weston Price, and Oleksandr Rudyy.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. This attempts to fix one of the issues related to the handling of Message credits. See QPID-3602 for an overall picture of the various issues.
bq.
bq. This particular patch does the following.
bq. 1. When the connection is stopped, it sends message.stop() & releases all messages in the prefetch buffer.
bq. 2. It will also release any messages (that were in flight) that comes after the connection is stopped. (*)
bq.
bq. (*) This interferes with the immediate_prefetch feature. However I don't know if immediate prefetch is really required in the 0-10 path.
bq.
bq. As always comments, suggestions & criticisms are equally welcomed.
bq.
bq.
bq. This addresses bug QPID-3604.
bq. https://issues.apache.org/jira/browse/QPID-3604
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/prefetch/PrefetchBehaviourTest.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java 1202228
bq.
bq. Diff: https://reviews.apache.org/r/2832/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. See PrefetchBehaviourTest#testConnectionStop for more details.
bq.
bq.
bq. Thanks,
bq.
bq. rajith
bq.
bq.
> If the connection is stopped the client should release all it's messages in the prefetch buffer
> -----------------------------------------------------------------------------------------------
>
> Key: QPID-3604
> URL: https://issues.apache.org/jira/browse/QPID-3604
> Project: Qpid
> Issue Type: Bug
> Components: Java Client
> Affects Versions: 0.14
> Reporter: Rajith Attapattu
> Assignee: Rajith Attapattu
> Fix For: 0.15
>
>
> When connection.stop() is called, the JMS client should release all it's messages in the prefetch buffer.
> For all we know, the connection may never be started (depending on application logic) and those messages will be stuck on the prefetch buffer. Releasing it will allow another consumer to get them (in the case of a shared queue case).
> Another less severe but nevertheless an undesirable side affect of this is the client getting more messages than required by the capacity or prefetch arguments. See QPID-3602
> This may not be a big issue if the client is prefetching a few messages, but if prefetching something like 5000 messages, this could potentially cause a lethal spike in the clients memory usage.
> Even in low capacity/prefetch values, if the messages are large (say in the mega byte range) this could potentially put the client under memory pressure.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3604) If the connection is stopped the
client should release all it's messages in the prefetch buffer
Posted by "Rajith Attapattu (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13193256#comment-13193256 ]
Rajith Attapattu commented on QPID-3604:
----------------------------------------
Robbie, I agree with you there.
The sequence should be,
1. Issue message stops and sync, waiting for the broker to confirm. At this point we can safely assume that the broker will not be sending any more messages.
2. Drain the dispatch queue
3. Drain the individual consumer queues.
So if I change the order then I agree we don't need the message MessageDeliveryLock.
Rajith
> If the connection is stopped the client should release all it's messages in the prefetch buffer
> -----------------------------------------------------------------------------------------------
>
> Key: QPID-3604
> URL: https://issues.apache.org/jira/browse/QPID-3604
> Project: Qpid
> Issue Type: Bug
> Components: Java Client
> Affects Versions: 0.14
> Reporter: Rajith Attapattu
> Assignee: Rajith Attapattu
> Fix For: 0.15
>
>
> When connection.stop() is called, the JMS client should release all it's messages in the prefetch buffer.
> For all we know, the connection may never be started (depending on application logic) and those messages will be stuck on the prefetch buffer. Releasing it will allow another consumer to get them (in the case of a shared queue case).
> Another less severe but nevertheless an undesirable side affect of this is the client getting more messages than required by the capacity or prefetch arguments. See QPID-3602
> This may not be a big issue if the client is prefetching a few messages, but if prefetching something like 5000 messages, this could potentially cause a lethal spike in the clients memory usage.
> Even in low capacity/prefetch values, if the messages are large (say in the mega byte range) this could potentially put the client under memory pressure.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3604) If the connection is stopped the
client should release all it's messages in the prefetch buffer
Posted by "Rob Godfrey (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13183953#comment-13183953 ]
Rob Godfrey commented on QPID-3604:
-----------------------------------
The commit associated with this JIRA doesn't seem to completely match the description - rather than releasing all the messages *only* when connection.stop() is called, it instead does so every time the channel is asked to suspend. While connection.stop() is one occasion when this happens, it is by no means the only one (e.g it is called every time a new consumer is registered, or if flow control is activated on a "no-ack" session).
I also note that one of the tests regularly failing after this commit is the new test added in the same commit.
Can I suggest that this change is reverted since these failures are making it difficult test whether other new work is causing failures or not.
> If the connection is stopped the client should release all it's messages in the prefetch buffer
> -----------------------------------------------------------------------------------------------
>
> Key: QPID-3604
> URL: https://issues.apache.org/jira/browse/QPID-3604
> Project: Qpid
> Issue Type: Bug
> Components: Java Client
> Affects Versions: 0.14
> Reporter: Rajith Attapattu
> Assignee: Rajith Attapattu
> Fix For: 0.15
>
>
> When connection.stop() is called, the JMS client should release all it's messages in the prefetch buffer.
> For all we know, the connection may never be started (depending on application logic) and those messages will be stuck on the prefetch buffer. Releasing it will allow another consumer to get them (in the case of a shared queue case).
> Another less severe but nevertheless an undesirable side affect of this is the client getting more messages than required by the capacity or prefetch arguments. See QPID-3602
> This may not be a big issue if the client is prefetching a few messages, but if prefetching something like 5000 messages, this could potentially cause a lethal spike in the clients memory usage.
> Even in low capacity/prefetch values, if the messages are large (say in the mega byte range) this could potentially put the client under memory pressure.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3604) If the connection is stopped the
client should release all it's messages in the prefetch buffer
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13183053#comment-13183053 ]
jiraposter@reviews.apache.org commented on QPID-3604:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2832/
-----------------------------------------------------------
(Updated 2012-01-10 03:52:44.305560)
Review request for qpid, Gordon Sim, Robbie Gemmell, Weston Price, and Oleksandr Rudyy.
Summary
-------
This attempts to fix one of the issues related to the handling of Message credits. See QPID-3602 for an overall picture of the various issues.
This particular patch does the following.
1. When the connection is stopped, it sends message.stop() & releases all messages in the prefetch buffer.
2. It will also release any messages (that were in flight) that comes after the connection is stopped. (*)
(*) This interferes with the immediate_prefetch feature. However I don't know if immediate prefetch is really required in the 0-10 path.
As always comments, suggestions & criticisms are equally welcomed.
This addresses bug QPID-3604.
https://issues.apache.org/jira/browse/QPID-3604
Diffs (updated)
-----
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession.java 1229466
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java 1229466
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/prefetch/PrefetchBehaviourTest.java 1229466
Diff: https://reviews.apache.org/r/2832/diff
Testing
-------
See PrefetchBehaviourTest#testConnectionStop for more details.
Thanks,
rajith
> If the connection is stopped the client should release all it's messages in the prefetch buffer
> -----------------------------------------------------------------------------------------------
>
> Key: QPID-3604
> URL: https://issues.apache.org/jira/browse/QPID-3604
> Project: Qpid
> Issue Type: Bug
> Components: Java Client
> Affects Versions: 0.14
> Reporter: Rajith Attapattu
> Assignee: Rajith Attapattu
> Fix For: 0.15
>
>
> When connection.stop() is called, the JMS client should release all it's messages in the prefetch buffer.
> For all we know, the connection may never be started (depending on application logic) and those messages will be stuck on the prefetch buffer. Releasing it will allow another consumer to get them (in the case of a shared queue case).
> Another less severe but nevertheless an undesirable side affect of this is the client getting more messages than required by the capacity or prefetch arguments. See QPID-3602
> This may not be a big issue if the client is prefetching a few messages, but if prefetching something like 5000 messages, this could potentially cause a lethal spike in the clients memory usage.
> Even in low capacity/prefetch values, if the messages are large (say in the mega byte range) this could potentially put the client under memory pressure.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3604) If the connection is stopped the
client should release all it's messages in the prefetch buffer
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13182735#comment-13182735 ]
jiraposter@reviews.apache.org commented on QPID-3604:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2832/
-----------------------------------------------------------
(Updated 2012-01-09 19:27:40.039273)
Review request for qpid, Gordon Sim, Robbie Gemmell, Weston Price, and Oleksandr Rudyy.
Changes
-------
This reworked patch addresses some of the concerns raised with the original patch.
The message-delivery lock is still taken to ensure that no message delivery is attempted while the 'stopping' is in progress.
IMO this is a nessacery evil.
The code now drains individual consumer queues as well as the dispatch queue (via syncDipatchQueue method) and releases both unacked and prefetched messages, while only the former being marked redelivered.
Also all of these transfers are being marked as completed to ensure credits don't dry up.
Summary
-------
This attempts to fix one of the issues related to the handling of Message credits. See QPID-3602 for an overall picture of the various issues.
This particular patch does the following.
1. When the connection is stopped, it sends message.stop() & releases all messages in the prefetch buffer.
2. It will also release any messages (that were in flight) that comes after the connection is stopped. (*)
(*) This interferes with the immediate_prefetch feature. However I don't know if immediate prefetch is really required in the 0-10 path.
As always comments, suggestions & criticisms are equally welcomed.
This addresses bug QPID-3604.
https://issues.apache.org/jira/browse/QPID-3604
Diffs (updated)
-----
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession.java 1229312
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java 1229312
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/prefetch/PrefetchBehaviourTest.java 1229312
Diff: https://reviews.apache.org/r/2832/diff
Testing
-------
See PrefetchBehaviourTest#testConnectionStop for more details.
Thanks,
rajith
> If the connection is stopped the client should release all it's messages in the prefetch buffer
> -----------------------------------------------------------------------------------------------
>
> Key: QPID-3604
> URL: https://issues.apache.org/jira/browse/QPID-3604
> Project: Qpid
> Issue Type: Bug
> Components: Java Client
> Affects Versions: 0.14
> Reporter: Rajith Attapattu
> Assignee: Rajith Attapattu
> Fix For: 0.15
>
>
> When connection.stop() is called, the JMS client should release all it's messages in the prefetch buffer.
> For all we know, the connection may never be started (depending on application logic) and those messages will be stuck on the prefetch buffer. Releasing it will allow another consumer to get them (in the case of a shared queue case).
> Another less severe but nevertheless an undesirable side affect of this is the client getting more messages than required by the capacity or prefetch arguments. See QPID-3602
> This may not be a big issue if the client is prefetching a few messages, but if prefetching something like 5000 messages, this could potentially cause a lethal spike in the clients memory usage.
> Even in low capacity/prefetch values, if the messages are large (say in the mega byte range) this could potentially put the client under memory pressure.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3604) If the connection is stopped the
client should release all it's messages in the prefetch buffer
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13150827#comment-13150827 ]
jiraposter@reviews.apache.org commented on QPID-3604:
-----------------------------------------------------
bq. On 2011-11-15 16:41:28, Gordon Sim wrote:
bq. > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java, line 787
bq. > <https://reviews.apache.org/r/2832/diff/1/?file=58392#file58392line787>
bq. >
bq. > Are there any other locks acquired as part of the block here? If so are there any lock ordering issues where you could be introducing a deadlock?
bq.
bq. rajith attapattu wrote:
bq. Not that I could think of. The message-delivery-lock is taken to ensure that no messages are being served while we start pulling them out of the queue.
bq. In my tests so far, I haven't encountered any issues. However I plan to have more manual tests - ex. Trying to stop the connection while the message consumers are in full flight.
bq.
bq. Gordon Sim wrote:
bq. What about the failover mutex? Could the release trigger a codepath that attempts to acquire that? What about an asynchronous exception occurring concurrently; would that ever need to acquire the message-delivery-lock?
Certainly possible as mentioned in the comment below. The failover and the synchronous exceptions are things that could trigger a deadlock.
Testing is the best way to eliminate these possibilities. However IMO acquiring the message-delivery-lock is a must to ensure unwanted interaction between messages delivery & releasing.
bq. On 2011-11-15 16:41:28, Gordon Sim wrote:
bq. > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java, line 796
bq. > <https://reviews.apache.org/r/2832/diff/1/?file=58392#file58392line796>
bq. >
bq. > You are syncing here while holding the delivery lock, could that cause any problems?
bq.
bq. rajith attapattu wrote:
bq. So far I haven't encountered any issues. However things like failover, session exceptions etc..could cause issues. I'm planning more thorough longer running tests.
bq. Another thing I am considering is to not use a sync() at all. I'm not quite convinced that it's of much value here.
bq.
bq. I've noticed that the client continues to get messages into it's queue even after the code returns from the sync call. Hence the code snippet to release any messages received after the connection is stopped. I was expecting the brokers response to the sync command to be received after the client has got all the messages that were in flight. So after I sync I could just release the messages in the queue and be done with it. But that's not the case.
bq.
bq. It seems that the dispatcher thread takes a bit of time to process the UnprocessedMessages into the correct JMSMessages and put them onto the queue. So the sync() really doesn't add much value here.
bq.
bq. Gordon Sim wrote:
bq. It sounds like it is necessary but not sufficient. You need to know that the stop has been processed by the broker and it will not send any further, but you also need to synchronise with the thread actually processing incoming messages(?).
I looked at it the other way :). Since it's not sufficient, it's not necessary. All though it sounds wrong, not having the sync doesn't influence the outcome of the patch at all (Bcos we release any messages we receive after the connection is stopped).
It's not ideal, but with the absence of a proper mechanism to synchronize with the message processing dispatcher thread this seems a reasonable approach.
Another reason why I shied away from attempting that was due to the nasty interactions we may have with threading. The dispatcher thread does use the message delivery lock and that route could increase the potential for a deadlock.
bq. On 2011-11-15 16:41:28, Gordon Sim wrote:
bq. > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java, line 126
bq. > <https://reviews.apache.org/r/2832/diff/1/?file=58393#file58393line126>
bq. >
bq. > What case(s) is this code required for? You are releasing a message you have just received, right? When is that required?
bq.
bq. rajith attapattu wrote:
bq. See the above for an explanation for why this is needed.
bq.
bq. Gordon Sim wrote:
bq. You mean this is here because of the lack of synchronization with the dispatcher thread? If so that seems a little nasty to me... anyway to do this more cleanly?
That is precisely the reason. This also makes the sync call redundant. I started with the sync() and realized that it wasn't sufficient, hence added this.
As explained above, I'm not sure if there is a reasonable way to synchronize with the message delivery thread.
One possible approach might be is to do something like the syncDispatchQueue() method. Where we push a certain marker message into the queue and then we get that we know there are no more messages in the pipeline. But I'm concerned about the safety and feasibility of such an approach.
Robbie I believe is one person who have looked at this code more extensively in the last little while. So waiting to hear from him about his ideas as well. I'm open to suggestions on this area. Lets see if we can collectively figure out a better solution.
- rajith
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2832/#review3264
-----------------------------------------------------------
On 2011-11-15 15:36:36, rajith attapattu wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2832/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-11-15 15:36:36)
bq.
bq.
bq. Review request for qpid, Gordon Sim, Robbie Gemmell, Weston Price, and Oleksandr Rudyy.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. This attempts to fix one of the issues related to the handling of Message credits. See QPID-3602 for an overall picture of the various issues.
bq.
bq. This particular patch does the following.
bq. 1. When the connection is stopped, it sends message.stop() & releases all messages in the prefetch buffer.
bq. 2. It will also release any messages (that were in flight) that comes after the connection is stopped. (*)
bq.
bq. (*) This interferes with the immediate_prefetch feature. However I don't know if immediate prefetch is really required in the 0-10 path.
bq.
bq. As always comments, suggestions & criticisms are equally welcomed.
bq.
bq.
bq. This addresses bug QPID-3604.
bq. https://issues.apache.org/jira/browse/QPID-3604
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/prefetch/PrefetchBehaviourTest.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java 1202228
bq.
bq. Diff: https://reviews.apache.org/r/2832/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. See PrefetchBehaviourTest#testConnectionStop for more details.
bq.
bq.
bq. Thanks,
bq.
bq. rajith
bq.
bq.
> If the connection is stopped the client should release all it's messages in the prefetch buffer
> -----------------------------------------------------------------------------------------------
>
> Key: QPID-3604
> URL: https://issues.apache.org/jira/browse/QPID-3604
> Project: Qpid
> Issue Type: Bug
> Components: Java Client
> Affects Versions: 0.14
> Reporter: Rajith Attapattu
> Assignee: Rajith Attapattu
> Fix For: 0.15
>
>
> When connection.stop() is called, the JMS client should release all it's messages in the prefetch buffer.
> For all we know, the connection may never be started (depending on application logic) and those messages will be stuck on the prefetch buffer. Releasing it will allow another consumer to get them (in the case of a shared queue case).
> Another less severe but nevertheless an undesirable side affect of this is the client getting more messages than required by the capacity or prefetch arguments. See QPID-3602
> This may not be a big issue if the client is prefetching a few messages, but if prefetching something like 5000 messages, this could potentially cause a lethal spike in the clients memory usage.
> Even in low capacity/prefetch values, if the messages are large (say in the mega byte range) this could potentially put the client under memory pressure.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3604) If the connection is stopped the
client should release all it's messages in the prefetch buffer
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13150633#comment-13150633 ]
jiraposter@reviews.apache.org commented on QPID-3604:
-----------------------------------------------------
bq. On 2011-11-15 16:41:28, Gordon Sim wrote:
bq. > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java, line 796
bq. > <https://reviews.apache.org/r/2832/diff/1/?file=58392#file58392line796>
bq. >
bq. > You are syncing here while holding the delivery lock, could that cause any problems?
bq.
bq. rajith attapattu wrote:
bq. So far I haven't encountered any issues. However things like failover, session exceptions etc..could cause issues. I'm planning more thorough longer running tests.
bq. Another thing I am considering is to not use a sync() at all. I'm not quite convinced that it's of much value here.
bq.
bq. I've noticed that the client continues to get messages into it's queue even after the code returns from the sync call. Hence the code snippet to release any messages received after the connection is stopped. I was expecting the brokers response to the sync command to be received after the client has got all the messages that were in flight. So after I sync I could just release the messages in the queue and be done with it. But that's not the case.
bq.
bq. It seems that the dispatcher thread takes a bit of time to process the UnprocessedMessages into the correct JMSMessages and put them onto the queue. So the sync() really doesn't add much value here.
It sounds like it is necessary but not sufficient. You need to know that the stop has been processed by the broker and it will not send any further, but you also need to synchronise with the thread actually processing incoming messages(?).
bq. On 2011-11-15 16:41:28, Gordon Sim wrote:
bq. > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java, line 126
bq. > <https://reviews.apache.org/r/2832/diff/1/?file=58393#file58393line126>
bq. >
bq. > What case(s) is this code required for? You are releasing a message you have just received, right? When is that required?
bq.
bq. rajith attapattu wrote:
bq. See the above for an explanation for why this is needed.
You mean this is here because of the lack of synchronization with the dispatcher thread? If so that seems a little nasty to me... anyway to do this more cleanly?
bq. On 2011-11-15 16:41:28, Gordon Sim wrote:
bq. > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java, line 787
bq. > <https://reviews.apache.org/r/2832/diff/1/?file=58392#file58392line787>
bq. >
bq. > Are there any other locks acquired as part of the block here? If so are there any lock ordering issues where you could be introducing a deadlock?
bq.
bq. rajith attapattu wrote:
bq. Not that I could think of. The message-delivery-lock is taken to ensure that no messages are being served while we start pulling them out of the queue.
bq. In my tests so far, I haven't encountered any issues. However I plan to have more manual tests - ex. Trying to stop the connection while the message consumers are in full flight.
What about the failover mutex? Could the release trigger a codepath that attempts to acquire that? What about an asynchronous exception occurring concurrently; would that ever need to acquire the message-delivery-lock?
- Gordon
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2832/#review3264
-----------------------------------------------------------
On 2011-11-15 15:36:36, rajith attapattu wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2832/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-11-15 15:36:36)
bq.
bq.
bq. Review request for qpid, Gordon Sim, Robbie Gemmell, Weston Price, and Oleksandr Rudyy.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. This attempts to fix one of the issues related to the handling of Message credits. See QPID-3602 for an overall picture of the various issues.
bq.
bq. This particular patch does the following.
bq. 1. When the connection is stopped, it sends message.stop() & releases all messages in the prefetch buffer.
bq. 2. It will also release any messages (that were in flight) that comes after the connection is stopped. (*)
bq.
bq. (*) This interferes with the immediate_prefetch feature. However I don't know if immediate prefetch is really required in the 0-10 path.
bq.
bq. As always comments, suggestions & criticisms are equally welcomed.
bq.
bq.
bq. This addresses bug QPID-3604.
bq. https://issues.apache.org/jira/browse/QPID-3604
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/prefetch/PrefetchBehaviourTest.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java 1202228
bq.
bq. Diff: https://reviews.apache.org/r/2832/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. See PrefetchBehaviourTest#testConnectionStop for more details.
bq.
bq.
bq. Thanks,
bq.
bq. rajith
bq.
bq.
> If the connection is stopped the client should release all it's messages in the prefetch buffer
> -----------------------------------------------------------------------------------------------
>
> Key: QPID-3604
> URL: https://issues.apache.org/jira/browse/QPID-3604
> Project: Qpid
> Issue Type: Bug
> Components: Java Client
> Affects Versions: 0.14
> Reporter: Rajith Attapattu
> Assignee: Rajith Attapattu
> Fix For: 0.15
>
>
> When connection.stop() is called, the JMS client should release all it's messages in the prefetch buffer.
> For all we know, the connection may never be started (depending on application logic) and those messages will be stuck on the prefetch buffer. Releasing it will allow another consumer to get them (in the case of a shared queue case).
> Another less severe but nevertheless an undesirable side affect of this is the client getting more messages than required by the capacity or prefetch arguments. See QPID-3602
> This may not be a big issue if the client is prefetching a few messages, but if prefetching something like 5000 messages, this could potentially cause a lethal spike in the clients memory usage.
> Even in low capacity/prefetch values, if the messages are large (say in the mega byte range) this could potentially put the client under memory pressure.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
[jira] [Resolved] (QPID-3604) If the connection is stopped the
client should release all it's messages in the prefetch buffer
Posted by "Rajith Attapattu (Resolved) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3604?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Rajith Attapattu resolved QPID-3604.
------------------------------------
Resolution: Fixed
> If the connection is stopped the client should release all it's messages in the prefetch buffer
> -----------------------------------------------------------------------------------------------
>
> Key: QPID-3604
> URL: https://issues.apache.org/jira/browse/QPID-3604
> Project: Qpid
> Issue Type: Bug
> Components: Java Client
> Affects Versions: 0.14
> Reporter: Rajith Attapattu
> Assignee: Rajith Attapattu
> Labels: possibly_complete
> Fix For: 0.15
>
>
> When connection.stop() is called, the JMS client should release all it's messages in the prefetch buffer.
> For all we know, the connection may never be started (depending on application logic) and those messages will be stuck on the prefetch buffer. Releasing it will allow another consumer to get them (in the case of a shared queue case).
> Another less severe but nevertheless an undesirable side affect of this is the client getting more messages than required by the capacity or prefetch arguments. See QPID-3602
> This may not be a big issue if the client is prefetching a few messages, but if prefetching something like 5000 messages, this could potentially cause a lethal spike in the clients memory usage.
> Even in low capacity/prefetch values, if the messages are large (say in the mega byte range) this could potentially put the client under memory pressure.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3604) If the connection is stopped the
client should release all it's messages in the prefetch buffer
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13151974#comment-13151974 ]
jiraposter@reviews.apache.org commented on QPID-3604:
-----------------------------------------------------
bq. On 2011-11-16 09:38:21, Keith Wall wrote:
bq. > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java, line 128
bq. > <https://reviews.apache.org/r/2832/diff/1/?file=58393#file58393line128>
bq. >
bq. > Could this make use of AMQSession#rejectMessage?
bq. >
bq. > I wonder also if this logic sit better in AMQSession#notifyConsumer(). It already rejects messages if the consumer is closed. Could it not also reject messages if the connection is no longer started?
bq.
bq. rajith attapattu wrote:
bq. Keith if you look at the rejectMessage method, it sets the redelivery option. In this case we should not be setting the redelivery option bcos the the application did not even see the message.
bq.
bq. I think we need to make a clear distinction btw reject and this case. If we are rejecting a message then we need to set redelivery. In other words the application had a look at it but decided not to use it. However in JMS you can't reject a message. So I'm not sure if setting redelivery in the rejectMessage is correct either.
bq.
bq. IMO the only time we should mark a message redelivered is when the application has seen a message but has not yet acknowledged. Ex consuming a bunch of messages in CLIENT_ACK and closing the consumer without acking any of the messages.
bq.
bq. Messages in the prefetch buffer should not be marked redelivered. I see there a few places where the rejectMessage method being used, and I don't think this is correct. Ex when we set a MessageListener we remove all messages in the internal queue and release them by setting the redelivery option.
bq.
bq. rajith attapattu wrote:
bq. Actually disregard the above comment. I totally forgot that the broker will mark all released messages as redelivered. So what the client sets doesn't really matter.
bq.
Re: "the broker will mark all released messages as redelivered. So what the client sets doesn't really matter."
That is not the case. The broker does what the client tells it to via the set-redelivered field of the message-release command.
- Gordon
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2832/#review3294
-----------------------------------------------------------
On 2011-11-15 15:36:36, rajith attapattu wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2832/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-11-15 15:36:36)
bq.
bq.
bq. Review request for qpid, Gordon Sim, Robbie Gemmell, Weston Price, and Oleksandr Rudyy.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. This attempts to fix one of the issues related to the handling of Message credits. See QPID-3602 for an overall picture of the various issues.
bq.
bq. This particular patch does the following.
bq. 1. When the connection is stopped, it sends message.stop() & releases all messages in the prefetch buffer.
bq. 2. It will also release any messages (that were in flight) that comes after the connection is stopped. (*)
bq.
bq. (*) This interferes with the immediate_prefetch feature. However I don't know if immediate prefetch is really required in the 0-10 path.
bq.
bq. As always comments, suggestions & criticisms are equally welcomed.
bq.
bq.
bq. This addresses bug QPID-3604.
bq. https://issues.apache.org/jira/browse/QPID-3604
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/prefetch/PrefetchBehaviourTest.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java 1202228
bq.
bq. Diff: https://reviews.apache.org/r/2832/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. See PrefetchBehaviourTest#testConnectionStop for more details.
bq.
bq.
bq. Thanks,
bq.
bq. rajith
bq.
bq.
> If the connection is stopped the client should release all it's messages in the prefetch buffer
> -----------------------------------------------------------------------------------------------
>
> Key: QPID-3604
> URL: https://issues.apache.org/jira/browse/QPID-3604
> Project: Qpid
> Issue Type: Bug
> Components: Java Client
> Affects Versions: 0.14
> Reporter: Rajith Attapattu
> Assignee: Rajith Attapattu
> Fix For: 0.15
>
>
> When connection.stop() is called, the JMS client should release all it's messages in the prefetch buffer.
> For all we know, the connection may never be started (depending on application logic) and those messages will be stuck on the prefetch buffer. Releasing it will allow another consumer to get them (in the case of a shared queue case).
> Another less severe but nevertheless an undesirable side affect of this is the client getting more messages than required by the capacity or prefetch arguments. See QPID-3602
> This may not be a big issue if the client is prefetching a few messages, but if prefetching something like 5000 messages, this could potentially cause a lethal spike in the clients memory usage.
> Even in low capacity/prefetch values, if the messages are large (say in the mega byte range) this could potentially put the client under memory pressure.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3604) If the connection is stopped the
client should release all it's messages in the prefetch buffer
Posted by "Rob Godfrey (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13192939#comment-13192939 ]
Rob Godfrey commented on QPID-3604:
-----------------------------------
Rajith, your latest commit seems to have broken the 0-9-1 build:
java.lang.NullPointerException
at org.apache.qpid.client.BasicMessageProducer_0_8.declareDestination(BasicMessageProducer_0_8.java:58)
at org.apache.qpid.client.BasicMessageProducer.<init>(BasicMessageProducer.java:139)
at org.apache.qpid.client.BasicMessageProducer_0_8.<init>(BasicMessageProducer_0_8.java:51)
at org.apache.qpid.client.AMQSession_0_8.createMessageProducer(AMQSession_0_8.java:478)
at org.apache.qpid.client.AMQSession_0_8.createMessageProducer(AMQSession_0_8.java:85)
at org.apache.qpid.client.AMQSession$7.execute(AMQSession.java:2679)
at org.apache.qpid.client.AMQSession$7.execute(AMQSession.java:2670)
at org.apache.qpid.client.AMQConnectionDelegate_8_0.executeRetrySupport(AMQConnectionDelegate_8_0.java:349)
at org.apache.qpid.client.AMQConnection.executeRetrySupport(AMQConnection.java:577)
at org.apache.qpid.client.failover.FailoverRetrySupport.execute(FailoverRetrySupport.java:102)
at org.apache.qpid.client.AMQSession.createProducerImpl(AMQSession.java:2668)
at org.apache.qpid.client.AMQSession.createProducer(AMQSession.java:1199)
at org.apache.qpid.client.AMQSession.createProducer(AMQSession.java:120)
at org.apache.qpid.client.prefetch.PrefetchBehaviourTest.testConnectionStop(PrefetchBehaviourTest.java:212)
at org.apache.qpid.test.utils.QpidBrokerTestCase.runBare(QpidBrokerTestCase.java:242)
at org.apache.qpid.test.utils.QpidTestCase.run(QpidTestCase.java:135)
Please run both the 0-9-1 and 0-10 builds before committing a change
> If the connection is stopped the client should release all it's messages in the prefetch buffer
> -----------------------------------------------------------------------------------------------
>
> Key: QPID-3604
> URL: https://issues.apache.org/jira/browse/QPID-3604
> Project: Qpid
> Issue Type: Bug
> Components: Java Client
> Affects Versions: 0.14
> Reporter: Rajith Attapattu
> Assignee: Rajith Attapattu
> Fix For: 0.15
>
>
> When connection.stop() is called, the JMS client should release all it's messages in the prefetch buffer.
> For all we know, the connection may never be started (depending on application logic) and those messages will be stuck on the prefetch buffer. Releasing it will allow another consumer to get them (in the case of a shared queue case).
> Another less severe but nevertheless an undesirable side affect of this is the client getting more messages than required by the capacity or prefetch arguments. See QPID-3602
> This may not be a big issue if the client is prefetching a few messages, but if prefetching something like 5000 messages, this could potentially cause a lethal spike in the clients memory usage.
> Even in low capacity/prefetch values, if the messages are large (say in the mega byte range) this could potentially put the client under memory pressure.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3604) If the connection is stopped the
client should release all it's messages in the prefetch buffer
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13150863#comment-13150863 ]
jiraposter@reviews.apache.org commented on QPID-3604:
-----------------------------------------------------
bq. On 2011-11-15 16:41:28, Gordon Sim wrote:
bq. > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java, line 787
bq. > <https://reviews.apache.org/r/2832/diff/1/?file=58392#file58392line787>
bq. >
bq. > Are there any other locks acquired as part of the block here? If so are there any lock ordering issues where you could be introducing a deadlock?
bq.
bq. rajith attapattu wrote:
bq. Not that I could think of. The message-delivery-lock is taken to ensure that no messages are being served while we start pulling them out of the queue.
bq. In my tests so far, I haven't encountered any issues. However I plan to have more manual tests - ex. Trying to stop the connection while the message consumers are in full flight.
bq.
bq. Gordon Sim wrote:
bq. What about the failover mutex? Could the release trigger a codepath that attempts to acquire that? What about an asynchronous exception occurring concurrently; would that ever need to acquire the message-delivery-lock?
bq.
bq. rajith attapattu wrote:
bq. Certainly possible as mentioned in the comment below. The failover and the synchronous exceptions are things that could trigger a deadlock.
bq. Testing is the best way to eliminate these possibilities. However IMO acquiring the message-delivery-lock is a must to ensure unwanted interaction between messages delivery & releasing.
Testing is certainly vital, however with threading issues a thorough analysis is as well given the non-deterministic nature of potential problems.
- Gordon
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2832/#review3264
-----------------------------------------------------------
On 2011-11-15 15:36:36, rajith attapattu wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2832/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-11-15 15:36:36)
bq.
bq.
bq. Review request for qpid, Gordon Sim, Robbie Gemmell, Weston Price, and Oleksandr Rudyy.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. This attempts to fix one of the issues related to the handling of Message credits. See QPID-3602 for an overall picture of the various issues.
bq.
bq. This particular patch does the following.
bq. 1. When the connection is stopped, it sends message.stop() & releases all messages in the prefetch buffer.
bq. 2. It will also release any messages (that were in flight) that comes after the connection is stopped. (*)
bq.
bq. (*) This interferes with the immediate_prefetch feature. However I don't know if immediate prefetch is really required in the 0-10 path.
bq.
bq. As always comments, suggestions & criticisms are equally welcomed.
bq.
bq.
bq. This addresses bug QPID-3604.
bq. https://issues.apache.org/jira/browse/QPID-3604
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/prefetch/PrefetchBehaviourTest.java 1202228
bq. http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java 1202228
bq.
bq. Diff: https://reviews.apache.org/r/2832/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. See PrefetchBehaviourTest#testConnectionStop for more details.
bq.
bq.
bq. Thanks,
bq.
bq. rajith
bq.
bq.
> If the connection is stopped the client should release all it's messages in the prefetch buffer
> -----------------------------------------------------------------------------------------------
>
> Key: QPID-3604
> URL: https://issues.apache.org/jira/browse/QPID-3604
> Project: Qpid
> Issue Type: Bug
> Components: Java Client
> Affects Versions: 0.14
> Reporter: Rajith Attapattu
> Assignee: Rajith Attapattu
> Fix For: 0.15
>
>
> When connection.stop() is called, the JMS client should release all it's messages in the prefetch buffer.
> For all we know, the connection may never be started (depending on application logic) and those messages will be stuck on the prefetch buffer. Releasing it will allow another consumer to get them (in the case of a shared queue case).
> Another less severe but nevertheless an undesirable side affect of this is the client getting more messages than required by the capacity or prefetch arguments. See QPID-3602
> This may not be a big issue if the client is prefetching a few messages, but if prefetching something like 5000 messages, this could potentially cause a lethal spike in the clients memory usage.
> Even in low capacity/prefetch values, if the messages are large (say in the mega byte range) this could potentially put the client under memory pressure.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3604) If the connection is stopped the
client should release all it's messages in the prefetch buffer
Posted by "Robbie Gemmell (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13193128#comment-13193128 ]
Robbie Gemmell commented on QPID-3604:
--------------------------------------
I'm still not entirely convinced of the need to hold the lock. The only seeming reason to do so would be to stop messages still in the dispatch queue (or any arriving after the stop completes, which shouldnt occur) from being moved by the Dispatcher into the consumer queues while you are draining them. If that were the case then it doesnt seem sufficient, because as soon as you think you have drained the consumer queues and move on to release that lock, any such messages still remaining in the dispatch queue can then potentially be put into the consumer queues before you hit the 'drainDispatchQueue' invocation.
Messages shouldnt arrive after stop() returns, at which point the dispatch queue can be drained to ensure no more messages are able to be delivered into the consumer queues, and after that the consumer queues can be safely drained without the lock because the Dispatcher has nothing to do.
+ @Override
+ void stop() throws AMQException
+ {
+ super.stop();
+ synchronized (getMessageDeliveryLock())
+ {
+ for (BasicMessageConsumer consumer : _consumers.values())
+ {
+ List<Long> tags = consumer.drainReceiverQueueAndRetrieveDeliveryTags();
+ _prefetchedMessageTags.addAll(tags);
+ }
+ }
+ _usingDispatcherForCleanup = true;
+ drainDispatchQueue();
+ _usingDispatcherForCleanup = false;
> If the connection is stopped the client should release all it's messages in the prefetch buffer
> -----------------------------------------------------------------------------------------------
>
> Key: QPID-3604
> URL: https://issues.apache.org/jira/browse/QPID-3604
> Project: Qpid
> Issue Type: Bug
> Components: Java Client
> Affects Versions: 0.14
> Reporter: Rajith Attapattu
> Assignee: Rajith Attapattu
> Fix For: 0.15
>
>
> When connection.stop() is called, the JMS client should release all it's messages in the prefetch buffer.
> For all we know, the connection may never be started (depending on application logic) and those messages will be stuck on the prefetch buffer. Releasing it will allow another consumer to get them (in the case of a shared queue case).
> Another less severe but nevertheless an undesirable side affect of this is the client getting more messages than required by the capacity or prefetch arguments. See QPID-3602
> This may not be a big issue if the client is prefetching a few messages, but if prefetching something like 5000 messages, this could potentially cause a lethal spike in the clients memory usage.
> Even in low capacity/prefetch values, if the messages are large (say in the mega byte range) this could potentially put the client under memory pressure.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org