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

Robbie Gemmell edited comment on QPID-3604 at 1/25/12 5:13 PM:
---------------------------------------------------------------

I'm still not entirely convinced of the need to hold the lock. The only seeming 
reason to do so would be to stop messages still in the dispatch queue (or any 
arriving after the stop completes, which shouldnt occur) from being moved by 
the Dispatcher into the consumer queues while you are draining them. If that 
were the case then it doesnt seem sufficient, because as soon as you think you 
have drained the consumer queues and move on to release that lock, any such 
messages still remaining in the dispatch queue can then potentially be put into 
the consumer queues before you hit the 'drainDispatchQueue' invocation.

Messages shouldnt arrive after stop() returns, at which point the dispatch 
queue can be drained to ensure no more messages are able to be delivered into 
the consumer queues, and after that the consumer queues can be safely drained 
without the lock because the Dispatcher has nothing to do.

{noformat}
+    @Override
+    void stop() throws AMQException
+    {
+        super.stop();
+        synchronized (getMessageDeliveryLock())
+        {
+               for (BasicMessageConsumer consumer : _consumers.values())
+               {
+                   List<Long> tags = 
consumer.drainReceiverQueueAndRetrieveDeliveryTags();
+                   _prefetchedMessageTags.addAll(tags);
+               }
+        }
+        _usingDispatcherForCleanup = true;
+        drainDispatchQueue();
+        _usingDispatcherForCleanup = false;
{noformat}
                
      was (Author: gemmellr):
    I'm still not entirely convinced of the need to hold the lock. The only 
seeming reason to do so would be to stop messages still in the dispatch queue 
(or any arriving after the stop completes, which shouldnt occur) from being 
moved by the Dispatcher into the consumer queues while you are draining them. 
If that were the case then it doesnt seem sufficient, because as soon as you 
think you have drained the consumer queues and move on to release that lock, 
any such messages still remaining in the dispatch queue can then potentially be 
put into the consumer queues before you hit the 'drainDispatchQueue' invocation.

Messages shouldnt arrive after stop() returns, at which point the dispatch 
queue can be drained to ensure no more messages are able to be delivered into 
the consumer queues, and after that the consumer queues can be safely drained 
without the lock because the Dispatcher has nothing to do.


+    @Override
+    void stop() throws AMQException
+    {
+        super.stop();
+        synchronized (getMessageDeliveryLock())
+        {
+               for (BasicMessageConsumer consumer : _consumers.values())
+               {
+                   List<Long> tags = 
consumer.drainReceiverQueueAndRetrieveDeliveryTags();
+                   _prefetchedMessageTags.addAll(tags);
+               }
+        }
+        _usingDispatcherForCleanup = true;
+        drainDispatchQueue();
+        _usingDispatcherForCleanup = false;

                  
> 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