9 Replies Latest reply on Nov 29, 2005 7:50 AM by timfox

    Simplifying concept of delivery

    timfox

      In our current use of Channels for implementing JMS destinations, Channels are used in the following places-

      1. To implement durable topic subscriptions - in which case a recoverable channel is used.
      2. To implement non-durable top subcriptions - in which case a non-recoverable channel is used.
      3. To implement a jms queue - in which case a recoverable channel is used.

      When a message arrives at a Channel in any of these cases, it is only ever delivered to (at most) one receiver of the Channel at any one time.

      I.e. for any particular Channel, at any one time, for a particular message, it is in the process of being delivered to one or zero receivers.

      Currently we use the Delivery class to model the concept of a message in the state of delivery, and we support a Channel having multiple delivery instances at the same time for a particular message.

      If we assume that a Channel *never* needs to have more than one delivery at any one time for a particular message then I believe we can simplify this signifcantly.

      The state of delivery can then just be modelled by a simple flag on the message state in the channel. (Or something similar).

      This also means we would not need a separate database table for deliveries, thus increasing performance and simplifying transaction management.

      It also means we would not have to maintain a list of deliveries in the ServerConsumerDelegate which would simplify the code significantly.

      In order to tackle the possible race condition where an acknowledgement could arrive before the state has been marked as "being delivered" we could keep the Delivery class but make sure that in it's constructor it calls back into the channel to set the state accordingly.

      Wdyt?

        • 1. Re: Simplifying concept of delivery
          ovidiu.feodorov

          The race condition you mentioned is one of the reasons we have Deliveries. See http://www.jboss.org/index.html?module=bb&op=viewtopic&t=65108,

          • 2. Re: Simplifying concept of delivery
            ovidiu.feodorov
            • 3. Re: Simplifying concept of delivery
              timfox

              I believe the proposed solution solves the race condition problem too, but without the serverconsumerdelegate having to maintain a list of deliveries.

              • 4. Re: Simplifying concept of delivery
                timfox

                Sorry I should make myself clearer here. :)

                As we know, there's a possible race condition whereby an acknowledgement could arrive before the channel knew the message had been delivered.

                Curently we're dealing with this by having a Delivery object that's created by the receiver then passed back in the return value of the handle() method.

                The delivery is then added to a list of deliveries maintained by the receiver, before the receiver actually delivers the message.

                When an ack (or a cancel) is required the delivery is looked up from the list then this is done on the Delivery object itself.

                This is all great, but the down side is it requires each receiver to maintain it's own list of deliveries, removing/adding them as necessary.

                My suggestion, is instead of the receiver managing the list of deliveries, the channel should handle it.

                So how do we tackle the race condition?

                One way of doing this would be to put code in the constructor of the Delivery object that calls into the channel and registers itself.

                Then we know, by the time the Delivery is constructed the Channel knows about it, and a race cannot occur.

                • 5. Re: Simplifying concept of delivery
                  ovidiu.feodorov

                   

                  I believe the proposed solution solves the race condition problem too, but without the serverconsumerdelegate having to maintain a list of deliveries.


                  Now that we decided the server consumer delegate does not accept more than one message at a time ("JBoss Messaging - Issues with client side message buffering." http://www.jboss.org/index.html?module=bb&op=viewtopic&t=71350), the consumer-side list of deliveries is redundant indeed. However, this issue is orthogonal to whether we use the concept of delivery or not.

                  How about a generic receiver that can accept more than one message at the same time? It needs to acknowledge them to the channel sooner or later, and it has to choices: either use the message ID for that, but in this case we get back from where we started, or use the concept of "delivery identity" and it's embodiment, the Delivery instance.

                  My suggestion, is instead of the receiver managing the list of deliveries, the channel should handle it.


                  The channel does maintain a list of deliveries anyway, otherwise it won't know whether the argument of DeliveryObserver.acknowledge(Delivery d, ...) is a known delivery or not.


                  • 6. Re: Simplifying concept of delivery
                    timfox

                    I understand that the current way of maintaining deliveries in the receiver solves the race condition problem, but it does so with some disadvantages:

                    1) Significant (many lines) extra code in the receiver to deal with these deliveries. Each receiver has to deal with this.

                    2) Extra lookups. Requires at least one lookup to find the delivery in the list, then, in most cases the channel will do another lookup to find the delivery in it's own list. Probably not a huge deal, but something we should avoid if we can.

                    My point is there is a much simpler way of getting the right semantics and also solving the race condition problem but without the overhead of maintaining a list of deliveries in the receiver.

                    The idea is that there is a simple callback interface into the Channel.

                    When a message arrives at a receiver.handle() method, the receiver makes sure the channel knows about the delivery _before_ it actually sends the mesage to the client by calling the callback interface.

                    The call does not return until the channel has stored the delivery in it's internal state.

                    Therefore, we can guarantee an ack can not come in before the channel knows about the delivery.

                    This means we solve the problem with one line of code in the receiver as opposed to over 100 lines as is currently the case, (also we don't have the extra lookup).

                    To be more explicit, here's an example of a handle() method in a ServerConsumerDelegate, only the pertinent parts have been retained:

                    public Delivery handle(DeliveryObserver observer, Routable reference, Transaction tx)
                     {
                    
                     SimpleDelivery del = new SimpleDelivery(...);
                    
                     channel.informChannelOfDelivery(del); //callback - new line of code
                    
                     //Now queue message for asynch deliver (as is currently the case).
                    
                    }
                    
                    



                    (I do not actually recommend having a method called "informChannelOfDelivery" - the above serves for illustration only.)







                    • 7. Re: Simplifying concept of delivery
                      ovidiu.feodorov

                      Proposal: The current interface works. It may be not the perfect one, but it solves our problems, so let's shelve this discussion for the time being and re-open it some time after 1.0 is released, when we'll (possibly) have a different perspective on things.

                      The issue is tracked by http://jira.jboss.org/jira/browse/JBMESSAGING-139

                      • 8. Re: Simplifying concept of delivery
                        timfox

                         

                        "ovidiu.feodorov@jboss.com" wrote:
                        Proposal: The current interface works. It may be not the perfect one, but it solves our problems, so let's shelve this discussion for the time being and re-open it some time after 1.0 is released, when we'll (possibly) have a different perspective on things.

                        The issue is tracked by http://jira.jboss.org/jira/browse/JBMESSAGING-139


                        Agreed :)

                        Now that we're limiting the JMS receiver to one delivery at a time, this isn't such a big issue now anyway.

                        • 9. Re: Simplifying concept of delivery
                          timfox

                          As discussed with Ovidiu - some more thoughts on this.

                          There seems to be a fairly straightforward way of getting rid of the list of deliveries in the ServerConsumerDelegate (this would make replication easier for one thing) *without any* changes to the Channel, DeliveryObserver, or Delivery interfaces. :)

                          To do this we need to modify SimpleDelivery to have constructor that takes a message_id and a delivery observer as params.

                          When an ack/cancel comes in we simply create an instance of SimpleDelivery with the message id and the delivery observer, and immediately call acknowledge()/cancel() on it.

                          This causes DeliveryObserver.cancel() /acknowledge() to be called.

                          The channel then looks up the delivery in it's list and cancels()/acks() it.

                          If the delivery is not found in the channel list - (this is the race condition where the ack can come in before the delivery), then it just adds itself with state "already_acked" and when the actual delivery comes in they just cancel each other out.

                          That's it.