Replicate Deliveries on ServerConsumerImpl

[clebert.suconic]

I wrote a Failover test that is sending 100 messages to a live node and consuming all on the live node. (i.e. I have the backup configured, but I don't fail the connection).

As I ACK the messages, I'm getting several errors on not finding the MessageId on ServerConsumerImpl::deliveringRefs.


After some investigation I found out that deliverReplicated is calling doHandle, and doHandle is checking for credits, even though this is documented:

// We call doHandle rather than handle, since we don't want to check available credits
 // This is because delivery and receive credits can be processed in different order on live
 // and backup, and otherwise we could have a situation where the delivery is replicated
 // but the credits haven't arrived yet, so the delivery gets rejected on backup
 HandleStatus handled = doHandle(ref);

private HandleStatus doHandle(final MessageReference ref) throws Exception
 {
 if (availableCredits != null && availableCredits.get() <= 0)
 {
 return HandleStatus.BUSY;
 }

 lock.lock();

 try
 {
 // If there is a pendingLargeMessage we can't take another message
 // This has to be checked inside the lock as the set to null is done inside the lock
 if (largeMessageSender != null)
 {
 return HandleStatus.BUSY;
 }

I'm getting several exceptions like:

if (handled != HandleStatus.HANDLED)
 {
 throw new IllegalStateException("Reference was not handled " + ref + " " + handled);
 }

and several others on not finding the MessageId.




Why deliverReplicated needs to actually call handle? Shouldn't instead just adding it to the list and eventually performing preCommit?

[clebert.suconic]

I've just added JustReplicationTest.


The test will pass, but you will see several exceptions on the log.

This is part of another failing condition I have on MessageChunkFailover. I expect to work more on this tomorrow afternoon (your afternoon.. my morning) if you (Tim) don't fix it before.

[timfox]

I can't run your test now since I am in the middle of a refactoring.

I notice you've set blocking to true for everything though.

Perhaps I'll have time to look later.

[clebert.suconic]

Never mind about the test. That test was setting the address on paging by accident.

But one of the issues I had on paging/chunk & clustering, was doHandle returning BUSY on a replication. Shouldn't it instead be doing what you originally wrote on that comment? (not checking for available credits?)