qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "jiraposter@reviews.apache.org (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (QPID-3604) If the connection is stopped the client should release all it's messages in the prefetch buffer
Date Thu, 17 Nov 2011 22:01:55 GMT

    [ https://issues.apache.org/jira/browse/QPID-3604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13152384#comment-13152384
] 

jiraposter@reviews.apache.org commented on QPID-3604:
-----------------------------------------------------



bq.  On 2011-11-16 09:38:21, Keith Wall wrote:
bq.  > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java,
line 128
bq.  > <https://reviews.apache.org/r/2832/diff/1/?file=58393#file58393line128>
bq.  >
bq.  >     Could this make use of AMQSession#rejectMessage?
bq.  >     
bq.  >     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?
bq.  
bq.  rajith attapattu wrote:
bq.      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.
bq.      
bq.      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.
bq.      
bq.      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.
bq.      
bq.      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.
bq.  
bq.  rajith attapattu wrote:
bq.      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.
bq.
bq.  
bq.  Gordon Sim wrote:
bq.      Re: "the broker will mark all released messages as redelivered. So what the client
sets doesn't really matter."
bq.      
bq.      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 is correct. So in that case Keith we will have to have re-evaluate the way we set the
REDELIVERY flag.
For the time being I prefer to have a separate release method.


- 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:
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-subscribe@qpid.apache.org


Mime
View raw message