[
https://issues.apache.org/jira/browse/QPID-3602?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13152408#comment-13152408
]
[email protected] 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.
Your observation is correct, this deals with messages that were already
received by the application.
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.
Let me pull in the change and see how I could integrate it with some cleanup.
Starting the flusher thread only for DUPS_OK was overdue :)
Do we have any existing tests that test this specific aspect of QueueBrowsers ?
bq. On 2011-11-17 21:22:31, Robbie Gemmell wrote:
bq. >
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java,
line 194
bq. > <https://reviews.apache.org/r/2853/diff/1/?file=58798#file58798line194>
bq. >
bq. > another whitespace error
:( this is killing me.
bq. On 2011-11-17 21:22:31, Robbie Gemmell wrote:
bq. >
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java,
line 284
bq. > <https://reviews.apache.org/r/2853/diff/1/?file=58798#file58798line284>
bq. >
bq. > I'll stop after this one...another whitespace error :)
Have mercy on me pls :D
bq. On 2011-11-17 21:22:31, Robbie Gemmell wrote:
bq. >
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java,
lines 188-202
bq. > <https://reviews.apache.org/r/2853/diff/1/?file=58799#file58799line188>
bq. >
bq. > The change to this method mean it no longer needs to exist and
should be removed.
Agreed. Left it there to better illustrate the diff. Will get rid of this
before committing.
bq. On 2011-11-17 21:22:31, Robbie Gemmell wrote:
bq. >
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java,
lines 441-444
bq. > <https://reviews.apache.org/r/2853/diff/1/?file=58799#file58799line441>
bq. >
bq. > Do we need to do this anymore considering that it just sends a
completion, given the processCompletions call added on L420 ?
I think you are right good catch !
But let me verify this.
bq. On 2011-11-17 21:22:31, Robbie Gemmell wrote:
bq. >
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/prefetch/PrefetchBehaviourTest.java,
line 173
bq. > <https://reviews.apache.org/r/2853/diff/1/?file=58800#file58800line173>
bq. >
bq. > Some onMessage() tests would be good, given recent disparity between
its behaviour and receive() for things like this.
Definitely this is in the cards for sure!
I'm going to spend some time to see how best I can reuse the test code to make
it more compact and test more combinations/variations.
- rajith
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2853/#review3325
-----------------------------------------------------------
On 2011-11-16 18:31:12, rajith attapattu wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2853/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-11-16 18:31:12)
bq.
bq.
bq. Review request for qpid, Gordon Sim, Robbie Gemmell, Weston Price, Keith
Wall, and Oleksandr Rudyy.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. This patch is for both QPID-3612 & QPID-3613. Please note QPID-3602 is the
main JIRA for credit related issues.
bq.
bq. Message completions are now handled independent of the Message acks.
bq.
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.
bq. 2. Cleanedup the acking process in AMQSession_0_10.java to make acking
independent of completions.
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.
bq.
bq. This addresses bug QPID-3602.
bq. https://issues.apache.org/jira/browse/QPID-3602
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
1202779
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java
1202779
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/prefetch/PrefetchBehaviourTest.java
1202779
bq.
bq. Diff: https://reviews.apache.org/r/2853/diff
bq.
bq.
bq. Testing
bq. -------
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.
bq.
bq. Thanks,
bq.
bq. rajith
bq.
bq.
> 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 basis.
--
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:[email protected]
