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-3602) Fix Consumer message credit issues in 0-10 codepath
Date Thu, 17 Nov 2011 23:01:53 GMT

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

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

bq.  On 2011-11-17 21:22:31, Robbie Gemmell wrote:
bq.  > In addition to the code-specific items already noted, I thought id mention that
it would be good if this change pulled in the rollback/recover related completion issue Keith
and I patched today. As I mentioned on the phone earlier, we went for the simplest (albeit
hacky) solution we could to make it clearer for inclusion in the release, but it would be
nice to see it cleaned up. This change looks to take care of the messages which were actually
delivered before the rollback was performed, but it still wont handle the ones which were
only prefetched.
bq.  > 
bq.  > I noticed that you stopped the flusher thread running for modes other than DUPS_OK,
which is nice. Alex questioned whether this change would affect browsers or not because they
ignore the session acknowledge mode and go with NO_ACK behaviour. Looking it over I think
it seems fine due to the processCompletions call in postDeliver(), but its possibly still
worth a check.
bq.  rajith attapattu wrote:
bq.      Your observation is correct, this deals with messages that were already received
by the application.
bq.      For recover(), we really don't need to send completions for the transfers we get
bcos we do a stop and start which re-issues the credit. But for correctness we should mark
them complete. As for rollback we definitely should mark the messages that were in the prefetch
buffer as complete so we get re-credited. 
bq.      Let me pull in the change and see how I could integrate it with some cleanup.
bq.      Starting the flusher thread only for DUPS_OK was overdue :)
bq.      Do we have any existing tests that test this specific aspect of QueueBrowsers ?
bq.  Robbie Gemmell wrote:
bq.      >For recover(), we really don't need to send completions for the transfers we
get bcos we do a stop and start which re-issues the credit.
bq.      I dont think that is correct. There is a discussion on the mailing lists at the moment
about the behavioural differences in credit window sizing between the Java and C++ brokers,
and the initial outcome looks like making the C++ broker move to doing what the Java broker
does. In that case, it is necessary for the client to complete all the commands during recover,
or the broker will simply consider that you have already used [some of] the new credit you
are issuing because you still havent completed the outstanding messages (as very explicitly
mentioned in the 0-10 spec, releasing etc a message does not complete it). If it did anything
else, you would effectively be allowing the broker to breach the requested credit window size
in certain situations (as the C++ broker currently does).

I don't think it's correct either :D , hence me mentioning "But for correctness we should
mark them complete".
Yes I was following closely that thread and I agree that the C++ broker should do the same
as the Java broker.

- rajith

This is an automatically generated e-mail. To reply, visit:

On 2011-11-16 18:31:12, rajith attapattu wrote:
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/2853/
bq.  -----------------------------------------------------------
bq.  (Updated 2011-11-16 18:31:12)
bq.  Review request for qpid, Gordon Sim, Robbie Gemmell, Weston Price, Keith Wall, and Oleksandr
bq.  Summary
bq.  -------
bq.  This patch is for both QPID-3612 & QPID-3613. Please note QPID-3602 is the main JIRA
for credit related issues.
bq.  Message completions are now handled independent of the Message acks.
bq.  1. The consumer now keeps track of credits on a per subscription basis using the RangeSet
"completions" and sends completions,
bq.     if the unCompletedCount >= _capacity/2
bq.     For capacity = 0 or capacity = 1, we use the flag "sendCompleteForEveryMsg" to send
a completion for each message.
bq.  2. Cleanedup the acking process in AMQSession_0_10.java to make acking independent of
bq.     2.1 For AUTO_ACK we send an ack after every message.
bq.     2.1 For DUPS_OK we ack if,
bq.            maxAckDelay <= 0 OR
bq.            unackedCount >= ackBatchingThreshold, where ackBatchingThreshold == prefetch/2
bq.  This addresses bug QPID-3602.
bq.      https://issues.apache.org/jira/browse/QPID-3602
bq.  Diffs
bq.  -----
bq.    http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java
bq.    http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java
bq.    http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/prefetch/PrefetchBehaviourTest.java
bq.  Diff: https://reviews.apache.org/r/2853/diff
bq.  Testing
bq.  -------
bq.  Added two basic test cases (transacted and client-ack modes) to ensure completions are
sent in time to ensure credits don't dry up.
bq.  Thanks,
bq.  rajith

> Fix Consumer message credit issues in 0-10 codepath
> ---------------------------------------------------
>                 Key: QPID-3602
>                 URL: https://issues.apache.org/jira/browse/QPID-3602
>             Project: Qpid
>          Issue Type: Bug
>          Components: Java Client
>    Affects Versions: 0.14
>            Reporter: Rajith Attapattu
>            Assignee: Rajith Attapattu
>             Fix For: 0.15
> Currently there are several issues related to message credits.
> 1. QPID-2604 - Getting more messages than required by the prefetch value.
> 2. QPID-3604 - If connection is started and stopped, the client *may* get more messages
than required by the prefetch value.
> 3. QPID-3562 - Prefetch=1 case doesn't work properly.
> 4. Prefetch-0 case doesn't work properly (well completely broken).
> 5. QPID-3612 -Message credits are affected by Command Completions and not message-acks.
However these two are intertwined in the logic causing some issues. For example when in client-ack
mode or using transactions, if the client has exhausted the credits, but is waiting for more
messages to come before it acks or commits a transaction, then the client will appear hung
(This issue is currently masked due to some of the above bugs). 
> 6. QPID-3613 Credit should be managed on a per subscription basis than on a per session

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

View raw message