6 Replies Latest reply on Dec 2, 2008 11:35 AM by Andy Taylor

    message expiry

    Andy Taylor Master

      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?

        • 1. Re: message expiry
          Tim Fox Master

          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.

          • 2. Re: message expiry
            Andy Taylor Master

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

            • 3. Re: message expiry
              Andy Taylor Master

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

              • 4. Re: message expiry
                Tim Fox Master

                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,



                • 5. Re: message expiry
                  Andy Taylor Master

                  ok, no probs, i'll sort it

                  • 6. Re: message expiry
                    Andy Taylor Master

                    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.