6 Replies Latest reply on Jul 27, 2011 9:35 AM by rcernich

    Generalize/improve synchronous IN_OUT exchange handling

    tfennelly

      In a few places in the code we're using Thread.sleep() in a loop to wait for the OUT of an IN_OUT Exchange.  This is obviously not great and there's a JIRA for it: https://issues.jboss.org/browse/SWITCHYARD-312

       

      Within the current codebase, the easiest way I could think of doing it was to create a SynchronousInOutHandler class on which you can call a waitForOut(timeout) method.

       

       

      I think maybe a better way would be to have the details built into the ExchangeContract in some way, with the ExchangeImpl.send() method using these contract details to wait for the response when invoked.  I looked at doing this and abandoned it... I think the API changes might freak Keith out a bit

        • 1. Re: Generalize/improve synchronous IN_OUT exchange handling
          rcernich

          Hey Tom,

           

          Did you think about using a barrier (CountDownLatch) instead of the loop?  You could keep the current implementation, replacing waitForResponse() with a latch.await(timeout).  Instead of having RESPONSE.set(), you could have latch.countDown().

           

          Just a thought,

          Rob

          • 2. Re: Generalize/improve synchronous IN_OUT exchange handling
            kcbabo

            I think this is a definite improvement over what we have now.  I was initially thinking about a wrapper/delegate that provided a blocking send method which waited for a reply, but I think what you have done is better.  A few comments:

             

            1) When using "waitForOut", clients have to be prepared for null if a reply is not received in the allotted time window.  I noticed some component code which calls methods on the returned message without a null check first.

             

            2) Related to (1), we should consider what is to be done with messages are received after the timeout has expired.  I don't think it's part of this particular JIRA, but if we are going to give up on a reply after a certain amount of time, we want to make sure that any replies after that point are routed to a location that can be tracked/audited.

             

            3) The default timeout is 15 secs.  Do we need a default timeout or should no timeout = wait indefinitely?  Perhaps this is a configurable property at some point?  It would be pretty cool if we had the ability to have application or service-scoped context properties which allowed the reply timeout to be customized.  Again, this is a separate JIRA, but worth considering.

            • 3. Re: Generalize/improve synchronous IN_OUT exchange handling
              tfennelly

              Hey Rob.

               

              No... hadn't thought of doing it that way.  TBH... I just lifted and generalized the code I had for doing this in the Bean components.... I generalized it as a common handler in the runtime component.

               

              Do you see an issue with how I did it?

              • 4. Re: Generalize/improve synchronous IN_OUT exchange handling
                tfennelly

                Keith Babo wrote:

                 

                1) When using "waitForOut", clients have to be prepared for null if a reply is not received in the allotted time window.  I noticed some component code which calls methods on the returned message without a null check first.

                 

                Good point!!  I think we should throw a DeliveryException (subtype of SwitchYardException) if this happens?  I think it's important that the calling code properly acknowledge a timeout.

                 

                 

                Keith Babo wrote:

                 

                2) Related to (1), we should consider what is to be done with messages are received after the timeout has expired.  I don't think it's part of this particular JIRA, but if we are going to give up on a reply after a certain amount of time, we want to make sure that any replies after that point are routed to a location that can be tracked/audited.

                 

                Right... I'll create another JIRA for this.  For now, I'll have the handler log this.

                Keith Babo wrote:

                 

                3) The default timeout is 15 secs.  Do we need a default timeout or should no timeout = wait indefinitely?  Perhaps this is a configurable property at some point?  It would be pretty cool if we had the ability to have application or service-scoped context properties which allowed the reply timeout to be customized.  Again, this is a separate JIRA, but worth considering.

                 

                I think having a timeout is a good thing.  If we don't, then we run the risk of consuming all threads and crashing the whole container if something is screwed up in one of the services/components.  No?

                • 5. Re: Generalize/improve synchronous IN_OUT exchange handling
                  kcbabo

                   

                  Good point!!  I think we should throw a DeliveryException (subtype of SwitchYardException) if this happens?  I think it's important that the calling code properly acknowledge a timeout.

                   

                  We can go that route instead of returning null.   Probably makes thing clearer from a client perspective and from an implementation standpoint.

                   

                  I think having a timeout is a good thing.  If we don't, then we run the risk of consuming all threads and crashing the whole container if something is screwed up in one of the services/components.  No?

                  Definitely agree that having a timeout is a good thing, but I would prefer for it to be configurable and 15 seconds is too low for a default value, IMHO.  If you can create a JIRA for making this configurable via properties, I will take a swing at making it work.  My guess is that the initial implementation could simply utilize the domain-level properties support we currently have.

                  • 6. Re: Generalize/improve synchronous IN_OUT exchange handling
                    rcernich

                    Hey Tom,

                     

                    No... hadn't thought of doing it that way.  TBH... I just lifted and generalized the code I had for doing this in the Bean components.... I generalized it as a common handler in the runtime component.

                     

                    Do you see an issue with how I did it?

                    Nope.  I think I altogether missed the point of what you were trying to do and was simply looking at it from the perspective of simply fixing that one class.

                     

                    Sorry for the unhelpful suggestion.

                     

                    Best,

                    Rob