On Wed, Sep 9, 2009 at 11:26 PM, Hiranya Jayathilaka <hiranya...@gmail.com>wrote:
> > > On Thu, Sep 10, 2009 at 11:38 AM, Supun Kamburugamuva > <supu...@gmail.com>wrote: > >> >> >> On Wed, Sep 9, 2009 at 10:48 PM, Hiranya Jayathilaka < >> hiranya...@gmail.com> wrote: >> >>> >>> >>> On Thu, Sep 10, 2009 at 11:10 AM, Supun Kamburugamuva <supu...@gmail.com >>> > wrote: >>> >>>> >>>> >>>> On Wed, Sep 9, 2009 at 10:27 PM, Hiranya Jayathilaka < >>>> hiranya...@gmail.com> wrote: >>>> >>>>> >>>>> >>>>> On Wed, Sep 9, 2009 at 7:52 PM, Supun Kamburugamuva <supu...@gmail.com >>>>> > wrote: >>>>> >>>>>> Hi, >>>>>> I think this is the price that we have to pay for enabling >>>>>> dynamic behavior. If the ESB is under huge load and we synchronize these >>>>>> methods it won't scale anyway. >>>>>> >>>>>> Having two threads load the same resource is tolerable if one thread >>>>>> doesn't access the resource while it is initialized halfway way. >>>>>> >>>>>> One suggestion, I think if we can move the >>>>>> >>>>>> initialized = true; line 166 >>>>>> >>>>>> statement in the AbstractEndpoint.java to the bottom of the init >>>>>> method it will be more safer. >>>>>> >>>>> >>>>> What would be safer is to move the above mentioned line of code to the >>>>> bottom and make the method synchronized. The AbstractEndpoint#init method >>>>> 'generally' doesn't get called by multiple threads simultaneously and >>>>> that's >>>>> probably why it is left un-synchronized at the moment. But with dynamic >>>>> endpoints (and only with dynamic endpoints) there is a chance that this >>>>> method can get called by several threads on the same object. >>>>> >>>>> Devs WDYT? Shall we make this change? Since the method does not get >>>>> called too often by concurrent threads I don't think there would be any >>>>> performance losses here. >>>>> >>>>> >>>> Endpoint.init method is supposed to be called by a single thread and >>>> only once. So I think it is not a good design to make this method >>>> synchronized. I think the correct approach is to handle the synchronization >>>> by the caller depending on the situation. >>>> >>> >>> I don't think I got your point. Could you please explain that a little >>> more? Currently there is a possibility that multiple message context >>> instances may attempt to initialize the same endpoint object at the same >>> time. How can we synchronize at caller level to avoid that? >>> >> >> I'm suggesting something like this. >> >> public Endpoint getEndpoint(String key) { >> Object o = localEntries.get(key); >> if (o != null && o instanceof Endpoint) { >> return (Endpoint) o; >> } else { >> Endpoint e = getConfiguration().getEndpoint(key); >> synchronized (object) { >> if (!e.isInitialized()) { >> e.init(synEnv); >> } >> } >> localEntries.put(key, e); >> return e; >> } >> }, >> >> But I don't think above approach is correct because it synchronize for >> each and every retrieval. >> > > Thanks for the code snippet. I guess you meant to sync on the endpoint (e), > not the object. Because the object could actually be null in the above case. > If that is the case then the above solution would work. Subsequent get > operations will find the endpoint from the local store so it's not a > problem. > > >> >> I think this is not the route cause of the problem. We are doing the >> initialization at the wrong place. Ideally initializtion should happen at a >> more deeper level when the endpoint is first created using the factory. This >> is handled nicely with the IndirectEndpoint code. It seems this >> IndirectEndpoint code is not used in some places like proxy services where >> it should be used. Hence we have all this trouble. >> > > I'm actually looking into the possibility of getting this logic coded into > the SynapseConfiguration class (like in your patch to SYNAPSE-577). > Currently SynapseConfiguration makes sure all the static resources are > initialized before returning them. It should ensure the same for the dynamic > resources. I'm working on a fix for this. > > But that takes us to our original problem - Access to dynamic resources are > not synchronized at SynapseConfiguration! Something we should look to fix in > the near future. > > If we do the initialization correctly at the SynapseConfiguration level then we don't have to worry about synchronization. Worse case scenario is same resource can be created multiple times. But as long as they are initialized once we should be ok. Resource creation happens rarely and if it happens two or three times it doesn't give that much of a performance bottleneck comparing to synchronization retrieval of resource at each and every request. Supun.. > Thanks, > Hiranya > > >> Supun.. >> >> >> >>> >>> Thanks, >>> Hiranya >>> >>> >>>> Supun.. >>>> >>>> >>>>> Thanks, >>>>> Hiranya >>>>> >>>>> >>>>>> Thanks, >>>>>> Supun.. >>>>>> >>>>>> On Wed, Sep 9, 2009 at 5:48 AM, Hiranya Jayathilaka < >>>>>> hiranya...@gmail.com> wrote: >>>>>> >>>>>>> Hi Devs, >>>>>>> The Axis2MessageContext class uses SynapseConfiguration#getEndpoint >>>>>>> and SynapseConfiguration#getSequence methods to gain access to dynamic >>>>>>> endpoints and sequences when they are not present in the message >>>>>>> context's >>>>>>> local store. However these method calls are not synchronized and from >>>>>>> the >>>>>>> looks of it I feel that if two message contexts try to access the same >>>>>>> dynamic resource at the same time there is a chance that >>>>>>> SynapseConfiguration would perform two remote registry lookups to fetch >>>>>>> the >>>>>>> same resource. This won't really cause any erroneous behavior but if >>>>>>> the ESB >>>>>>> is under a huge load where each message is trying to load a particular >>>>>>> dynamic resource things may get out of hands. >>>>>>> >>>>>>> So does it make sense to bring in some synchronization into the >>>>>>> picture? Or have I missed something and the above theory is not >>>>>>> applicable? >>>>>>> >>>>>>> Thanks >>>>>>> -- >>>>>>> Hiranya Jayathilaka >>>>>>> Software Engineer; >>>>>>> WSO2 Inc.; http://wso2.org >>>>>>> E-mail: hira...@wso2.com; Mobile: +94 77 633 3491 >>>>>>> Blog: http://techfeast-hiranya.blogspot.com >>>>>>> >>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> Software Engineer, WSO2 Inc >>>>>> http://wso2.org >>>>>> supunk.blogspot.com >>>>>> >>>>>> >>>>>> >>>>> >>>>> >>>>> -- >>>>> Hiranya Jayathilaka >>>>> Software Engineer; >>>>> WSO2 Inc.; http://wso2.org >>>>> E-mail: hira...@wso2.com; Mobile: +94 77 633 3491 >>>>> Blog: http://techfeast-hiranya.blogspot.com >>>>> >>>> >>>> >>>> >>>> -- >>>> Software Engineer, WSO2 Inc >>>> http://wso2.org >>>> supunk.blogspot.com >>>> >>>> >>>> >>> >>> >>> -- >>> Hiranya Jayathilaka >>> Software Engineer; >>> WSO2 Inc.; http://wso2.org >>> E-mail: hira...@wso2.com; Mobile: +94 77 633 3491 >>> Blog: http://techfeast-hiranya.blogspot.com >>> >> >> >> >> -- >> Software Engineer, WSO2 Inc >> http://wso2.org >> supunk.blogspot.com >> >> >> > > > -- > Hiranya Jayathilaka > Software Engineer; > WSO2 Inc.; http://wso2.org > E-mail: hira...@wso2.com; Mobile: +94 77 633 3491 > Blog: http://techfeast-hiranya.blogspot.com > -- Software Engineer, WSO2 Inc http://wso2.org supunk.blogspot.com