|
[
Permlink
| « Hide
]
Celso Pinto added a comment - 30/Jun/08 05:00 PM
This test uncovers two messages being dispatched on client ACK
I've attached a patch for tests that displays the error described on #1. Digging through the code, I found it is related with the following piece of code in activemq-core/src/main/java/org/apache/activemq/broker/region/PrefetchSubscription.java at line 230:
prefetchExtension = Math.max(prefetchExtension, index + 1); This code is broken because when the first message is dispatched to the client, prefetchExtension isn't incremented so when the client ACKs the first message, index is incremented to 1 and Math.max() returns 2 so two messages are dispatched. Can anyone please explain what prefetchExtension is supposed to do and how changing it impacts the rest of the code? I've asked on IRC and received no replies, same thing on activemq-devel mailing list. I've committed a change (along with a test case) regarding the prefetch size problem. I've changed "index + 1" with "index", since it is being incremented earlier. All tests are passing. If anyone else can take a look at this as well it would be great.
The problem with dispatch stopping after transaction abort was due to the fact that messages were acked at the moment they were dispatched and we didn't have any logic to redeliver them after the rollback. I implemented some additional logic (Committed revision 738904), that acks messages received in the transaction on commit, and redeliver them after the abort. This helps in this usecase, as you can see in the StompTest.testPrefetchSize().
I still can imagine certain scenarios where this would not work well, mostly because we dispatch messages as they come to the transport and don't care whether some of already dispatched messages should be redelivered before, but this requires quite a big refactoring, so I leave it for the future enhancements. It seems that acking a messages before an abort, still makes the client hang on the next receive.
pushing this out to 5.4.0 as more work is needed
I just wanted to offer a bit of feedback on the current state of this as of the 20090620 snapshot (which includes Dejan's 738904 revision). For message redelivery, when aborting the transaction, the redelivery policy is not currently honored. This results in an infinite number of immediate retries to deliver the unacknowledged messages.
Just looked at the test case again and it seems that this is working fine now (modified test case proves it).
As for the redelivery policy, it is not implemented for Stomp. I created another issue for this: https://issues.apache.org/activemq/browse/AMQ-2345 The final comment on this issue: redelivery of messages after abort is not supported by Stomp protocol (thanks Hiram for the tip), so I reverted some of the delivery logic that was implemented for this issue. The test case is StompTest.testPrefetchSize() is now modified to reflect these changes and some fixes are implemented to make it work. Please take a look at http://activemq.apache.org/how-do-i-unack-the-message-with-stomp.html
I'm sorry but that is just wrong, if the consumer dies out of an unexpected/unhandled error (think OOM) then the message is gone for good. The STOMP protocol does define transactions, it just doesn't specify how a consumer transaction must work. I personally see that as an oversight of the specification. You should do The Right Thing and rollback the messages if the transaction is a) explicitly aborted or b) the client connection is dropped, independently of the lack of completeness in the STOMP specification, anything else is unnacceptable and unexpected behavior of the message broker.
Hi Celso,
I think you're confusing transaction rollback and death of consumer. In case when consumer dies, the broker will detect it and will append all messages that are dispatched but not acknowledged by that consumer (all the messages you have acked in the transaction that hasn't been committed) to the end of the dispatch list. So these messages will be redelivered again to another consumer eventually. This works now in the same way for JMS and Stomp clients. But when we rollback the transaction, we say to the broker that we haven't yet consumed messages that were delivered to us. So the broker will wait for those messages to be acked before sending more messages. This also works in the same way for Java and Stomp clients, it's just that JMS Java client has built-in logic that will try to redeliver those messages first (locally) when you start a new transaction. You can also implement the same logic in your Stomp client (or application). The whole point is that redelivery of messages after the rollback, is the task for the client, since there's no need for the broker to resend those messages over the network again. Hope this helps, |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||