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

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



bq.  On 2011-11-15 16:41:28, Gordon Sim 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 126
bq.  > <https://reviews.apache.org/r/2832/diff/1/?file=58393#file58393line126>
bq.  >
bq.  >     What case(s) is this code required for? You are releasing a message 
you have just received, right? When is that required?
bq.  
bq.  rajith attapattu wrote:
bq.      See the above for an explanation for why this is needed.
bq.  
bq.  Gordon Sim wrote:
bq.      You mean this is here because of the lack of synchronization with the 
dispatcher thread? If so that seems a little nasty to me... anyway to do this 
more cleanly?
bq.  
bq.  rajith attapattu wrote:
bq.      That is precisely the reason. This also makes the sync call redundant. 
I started with the sync() and realized that it wasn't sufficient, hence added 
this.
bq.      As explained above, I'm not sure if there is a reasonable way to 
synchronize with the message delivery thread.
bq.      
bq.      One possible approach might be is to do something like the 
syncDispatchQueue() method. Where we push a certain marker message into the 
queue and then we get that we know there are no more messages in the pipeline. 
But I'm concerned about the safety and feasibility of such an approach.
bq.      
bq.      Robbie I believe is one person who have looked at this code more 
extensively in the last little while. So waiting to hear from him about his 
ideas as well. I'm open to suggestions on this area. Lets see if we can 
collectively figure out a better solution.
bq.  
bq.  Robbie Gemmell wrote:
bq.      (just noticed I didnt press publish yesterday morning on this...oops)
bq.      
bq.      > One possible approach might be is to do something like the 
syncDispatchQueue() method.
bq.      
bq.      This is exactly the comment I was going to make. Its not the nicest 
thing in the world, but I think its better than holding yet more locks. 
Ensuring that the broker has finished sending you messages on the stopped 
session and then having the Dispatcher do the work and tell you that there isnt 
anything left to deliver seems the easiest to reason about, and we already do 
that elsewhere so reusing the idea seems like the way to go.

Let me work this out and see how it goes.


- rajith


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2832/#review3264
-----------------------------------------------------------


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

Reply via email to