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