[ https://issues.apache.org/jira/browse/QPID-3604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13150863#comment-13150863 ]
jirapos...@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-subscr...@qpid.apache.org