message expiry

[ataylor]

Ive implemented a message expirer that basically iterates through each queue and expires any messages. The problem is that because I've used the iterator, which is fail fast, the method has to be synchronized on queue which is, as Tim correctly pointed out, not very scalable.

An alternative would be be to use the getAll() method on the PriorityLinkedList, which basically makes a copy, and iterate through this. We would then only need to synchronize on removing any expired messages. If the message has been consumed since checking then we just ignore as the client will take care of expiring it.

Another alternative would be to maintain another concurrentset of the messages and iterate over these instead.

thoughts?

[timfox]

I don't like the idea of copying.

I'd recommend for now doing something like the following,

Maintain a ConcurrentHashSet of refs in the queue as well as the normal PriorityLinkedList.

As ref is added to queue, if it has a non zero expiry time then add it to that set too. On acknowledge remove from set too.

Then the expiry thread can iterate that set without any problems.

[ataylor]

Ok, I'll also run some perf comparisons to see what effect it has

[ataylor]

would you expect the message order to be maintained in the dlq's if a number of messages expire at the same time?

[timfox]

A few comments on the latest commit:

1) Why do we need

if (ref.getMessage().getExpiration() != 0)
 {
 expiringMessageReferences.addIfAbsent(ref);
 }

In method queueimpl::addlistfirst?

AddListFirst happens at rollback in which case the ref should already be in that set

2) QueueImpl:expireMessages. Why loop through the set twice - this can be done in a single iteration

3) don't type the LHS as a ConcurrentHashSet -bad practice

private final ConcurrentHashSet<MessageReference> expiringMessageReferences = new ConcurrentHashSet<MessageReference>();

4) If MessageExpiryRunner is put in postoffice (which seems a more natural place for it) then you don't need to add the getQueues method to PostOffice,

[ataylor]

ok, no probs, i'll sort it

[ataylor]

Ive done some performance testing with these changes and theres a 4% drop in throughput when every single message sent has the expiry time set.