> 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.
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: > > ----------------------------------------------------------- > 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 > >
