> > > > On 2011-11-16 09:38:21, Keith Wall wrote: > > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java, > > > line 128 > > > <https://reviews.apache.org/r/2832/diff/1/?file=58393#file58393line128> > > > > > > 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? > > > > rajith attapattu wrote: > > 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. > > > > rajith attapattu wrote: > > 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. > > > > 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
Rajith - mea culpa, you specifically asked me about this behavior yesterday, and I missed the fact that Gordon points out above: the client can optionally tell the broker -not- to set the redelivered-flag when it releases the message. Sorry, -K > > > ----------------------------------------------------------- > 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: > > > > ----------------------------------------------------------- > > This is an automatically generated e-mail. To reply, visit: > > https://reviews.apache.org/r/2832/ > > ----------------------------------------------------------- > > > > (Updated 2011-11-15 15:36:36) > > > > > > 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 > > > > > > --------------------------------------------------------------------- Apache Qpid - AMQP Messaging Implementation Project: http://qpid.apache.org Use/Interact: mailto:[email protected]
