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