-
1. Re: Generalize/improve synchronous IN_OUT exchange handling
rcernich Jul 25, 2011 7:32 PM (in response to tfennelly)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 Jul 26, 2011 10:02 AM (in response to tfennelly)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 Jul 27, 2011 6:06 AM (in response to rcernich)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 Jul 27, 2011 6:12 AM (in response to kcbabo)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 Jul 27, 2011 7:19 AM (in response to tfennelly)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 Jul 27, 2011 9:35 AM (in response to tfennelly)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