Axis2
  1. Axis2
  2. AXIS2-5049

Axis2 Services / ServiceGroups Not Updating Properly When doing Hot-Update with Transport Session

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 1.5.4
    • Fix Version/s: nightly
    • Component/s: kernel
    • Labels:
      None

      Description

      The scenario is explained in the following points,

      • AxisService in Transport Session Scope.
      • Incoming request.
      • Create ServiceContext, ServiceGroupContext from AxisService and AxisServiceGroup and store it in SessionContext.
      • SessionContext contains fixed ServiceContext and ServiceGroupContext throughout Transport Session lifetime.
      • Changes to AxisConfiguratio wont be known by SessionContext, removing of services to service group, removing service group.
      • Removing Service Group, removing Service from service group must be communicated to SessionContext, so it can remove the necessory items from itself, and refresh the services / service groups in the next request.

      Because of the above problem, when doing hotupdate, a service which is in transport session will not be updated to the newly deployed service. Also, same for service groups, if a service group was removed and added again, the service group that is in the ServiceGroupContext will be the old service group.

      Cheers,
      Anjana.

      1. AXIS2-5049-Patch-2.txt
        1 kB
        Anjana Fernando
      2. AXIS2-5049-Patch.txt
        9 kB
        Anjana Fernando

        Activity

        Hide
        Anjana Fernando added a comment - - edited

        Hi Hiranya,

        Thank you for pointing it out. As we discussed offline, that can be fixed by reverting it to the earlier list implementation and simply using another variable for the SessionContext observers to track it's inclusions as AxisObservers. But I also came across another problem, since we are putting the SessionContexts as AxisObservers, these will never be taken out, so these will continue to grow as users make new sessions and they will never be garbage collected, which is a significant problem. So please ignore the above patches, and we would have to figure out a better approach to do this.

        I did see a comment in AbstractContext class's "touch" method, which mentioned about a timer task cleaning up the contexts, but I couldn't find the place it was actually doing that (the timer task).

        Cheers,
        Anjana.

        Show
        Anjana Fernando added a comment - - edited Hi Hiranya, Thank you for pointing it out. As we discussed offline, that can be fixed by reverting it to the earlier list implementation and simply using another variable for the SessionContext observers to track it's inclusions as AxisObservers. But I also came across another problem, since we are putting the SessionContexts as AxisObservers, these will never be taken out, so these will continue to grow as users make new sessions and they will never be garbage collected, which is a significant problem. So please ignore the above patches, and we would have to figure out a better approach to do this. I did see a comment in AbstractContext class's "touch" method, which mentioned about a timer task cleaning up the contexts, but I couldn't find the place it was actually doing that (the timer task). Cheers, Anjana.
        Hide
        Hiranya Jayathilaka added a comment -

        This patch breaks some of the existing stuff. Most significantly the following change is very dangerous:

        public ArrayList<AxisObserver> getObserversList()

        { - return observersList; + return new ArrayList<AxisObserver>(observers); }

        There are many classes that use the getObserversList method to remove an already existing observer. With the above change it cannot be used to unregister observers anymore. There is a definite regression in base transport and there could be others elsewhere.

        Show
        Hiranya Jayathilaka added a comment - This patch breaks some of the existing stuff. Most significantly the following change is very dangerous: public ArrayList<AxisObserver> getObserversList() { - return observersList; + return new ArrayList<AxisObserver>(observers); } There are many classes that use the getObserversList method to remove an already existing observer. With the above change it cannot be used to unregister observers anymore. There is a definite regression in base transport and there could be others elsewhere.
        Hide
        Anjana Fernando added a comment -

        Hi,

        There's a small issue in the previous patch (AXIS2-5049-Patch.txt), where the "sessionContext" can be null when adding it as an AxisObserver. This patch fixes it. Do note that this patch have to be applied after the earlier patch is applied that is, first also apply AXIS2-5049-Patch.txt and then AXIS2-5049-Patch-2.txt.

        Cheers,
        Anjana.

        Show
        Anjana Fernando added a comment - Hi, There's a small issue in the previous patch ( AXIS2-5049 -Patch.txt), where the "sessionContext" can be null when adding it as an AxisObserver. This patch fixes it. Do note that this patch have to be applied after the earlier patch is applied that is, first also apply AXIS2-5049 -Patch.txt and then AXIS2-5049 -Patch-2.txt. Cheers, Anjana.
        Hide
        Anjana Fernando added a comment -

        Hi Azeez,

        Thank you for pointing that out. I guess, that was anyway a problem that was there in Axis2. And this patch just addresses a unique problem that comes with transport session scoped services. I guess we will later look into that problem more carefully and come with a solution to do that. Hope that's ok.

        Cheers,
        Anjana,

        Show
        Anjana Fernando added a comment - Hi Azeez, Thank you for pointing that out. I guess, that was anyway a problem that was there in Axis2. And this patch just addresses a unique problem that comes with transport session scoped services. I guess we will later look into that problem more carefully and come with a solution to do that. Hope that's ok. Cheers, Anjana,
        Hide
        Afkham Azeez added a comment -

        Yes, this is a real production requirement. Our argument that hot update is not necessary, many years ago, is not valid in today's context.

        Anyway, Anjana. I agree with your approach but there is a slight flaw. If there are requests in flight, you must server those requests using the old service, then put the service into maintenance mode (where new requests are temporarily rejected or held back), then switch to the new service & serve the requests.

        Show
        Afkham Azeez added a comment - Yes, this is a real production requirement. Our argument that hot update is not necessary, many years ago, is not valid in today's context. Anyway, Anjana. I agree with your approach but there is a slight flaw. If there are requests in flight, you must server those requests using the old service, then put the service into maintenance mode (where new requests are temporarily rejected or held back), then switch to the new service & serve the requests.
        Hide
        Anjana Fernando added a comment -

        Hi Deepal,

        Thank you for the input. In my situation, I do actually have to use this feature plenty of times, because I'm using my own Axis2 deployer and deploy a service type that needs to be updated frequently. So it require hot-update to be able to do that, which is to see the changes immediately. And also, the services are required to be in transport session for some of the functionalities it has. So for my scenario, this is critical ..

        Cheers,
        Anjana.

        Show
        Anjana Fernando added a comment - Hi Deepal, Thank you for the input. In my situation, I do actually have to use this feature plenty of times, because I'm using my own Axis2 deployer and deploy a service type that needs to be updated frequently. So it require hot-update to be able to do that, which is to see the changes immediately. And also, the services are required to be in transport session for some of the functionalities it has. So for my scenario, this is critical .. Cheers, Anjana.
        Hide
        Deepal Jayasinghe added a comment -

        Well, if I understood it correctly, your approach will also have problems, where this will incur a performance overhead on the actual service invocations, where as, what I suggested just happens at deployment time
        >>> It does not really matter whether it is deployment time or runtime, because hot-update happens at the runtime, so there might be so many other invocations going on in the system when you do the hot-update. There is only one Java process.

        Because, lets say 1000 unique sessions / 1000 users, will be sending requests, for each request from each of the user, the handler will check if the SessionContext has valid service group context's and service contexts inside that group, so there's some logic that needs to be executed to check whether those contexts are valid and up to date against the ServiceGroups and Services.
        >>. I agree, but we have so many these kind of logic processing in Axis2, so I do not see a big issue there.But, it would be much better if we can come up with a good approach.

        So compared to that, what I suggested will only happen when a change to a service group / service has occurred and then notified to the SessionContexts, so 1 time 1000 notifications at once. But in what you suggested, there will always be 1000 sessions (provided they are sending requests often), sending 1000 requests always to check up on a state that most probably have not changed for a while.
        >> Yes I agree, but there is a problem, when you do so many updates once whole system might become the bottleneck, but if you do the handler approach it will not process all of them once, hence, processing time is distributed. In addition, the intended behavior of Observers is to perform light wight tasks such as creating a RSS/Atom feed when a service get updated.

        Anyway this is just my thoughts. If you think this is really needed (I do not see a big reason to implement this feature) and your approach works fine we can commit the code. In the real system hot-updates does not happen so frequently, and in fact Axis2 does not recommend doing hot-update in production settings.

        Deepal

        Show
        Deepal Jayasinghe added a comment - Well, if I understood it correctly, your approach will also have problems, where this will incur a performance overhead on the actual service invocations, where as, what I suggested just happens at deployment time >>> It does not really matter whether it is deployment time or runtime, because hot-update happens at the runtime, so there might be so many other invocations going on in the system when you do the hot-update. There is only one Java process. Because, lets say 1000 unique sessions / 1000 users, will be sending requests, for each request from each of the user, the handler will check if the SessionContext has valid service group context's and service contexts inside that group, so there's some logic that needs to be executed to check whether those contexts are valid and up to date against the ServiceGroups and Services. >>. I agree, but we have so many these kind of logic processing in Axis2, so I do not see a big issue there.But, it would be much better if we can come up with a good approach. So compared to that, what I suggested will only happen when a change to a service group / service has occurred and then notified to the SessionContexts, so 1 time 1000 notifications at once. But in what you suggested, there will always be 1000 sessions (provided they are sending requests often), sending 1000 requests always to check up on a state that most probably have not changed for a while. >> Yes I agree, but there is a problem, when you do so many updates once whole system might become the bottleneck, but if you do the handler approach it will not process all of them once, hence, processing time is distributed. In addition, the intended behavior of Observers is to perform light wight tasks such as creating a RSS/Atom feed when a service get updated. Anyway this is just my thoughts. If you think this is really needed (I do not see a big reason to implement this feature) and your approach works fine we can commit the code. In the real system hot-updates does not happen so frequently, and in fact Axis2 does not recommend doing hot-update in production settings. Deepal
        Hide
        Anjana Fernando added a comment -

        Hi Deepal,

        Well, if I understood it correctly, your approach will also have problems, where this will incur a performance overhead on the actual service invocations, where as, what I suggested just happens at deployment time.

        Because, lets say 1000 unique sessions / 1000 users, will be sending requests, for each request from each of the user, the handler will check if the SessionContext has valid service group context's and service contexts inside that group, so there's some logic that needs to be executed to check whether those contexts are valid and up to date against the ServiceGroups and Services.

        So compared to that, what I suggested will only happen when a change to a service group / service has occurred and then notified to the SessionContexts, so 1 time 1000 notifications at once. But in what you suggested, there will always be 1000 sessions (provided they are sending requests often), sending 1000 requests always to check up on a state that most probably have not changed for a while.

        Cheers,
        Anjana.

        Show
        Anjana Fernando added a comment - Hi Deepal, Well, if I understood it correctly, your approach will also have problems, where this will incur a performance overhead on the actual service invocations, where as, what I suggested just happens at deployment time. Because, lets say 1000 unique sessions / 1000 users, will be sending requests, for each request from each of the user, the handler will check if the SessionContext has valid service group context's and service contexts inside that group, so there's some logic that needs to be executed to check whether those contexts are valid and up to date against the ServiceGroups and Services. So compared to that, what I suggested will only happen when a change to a service group / service has occurred and then notified to the SessionContexts, so 1 time 1000 notifications at once. But in what you suggested, there will always be 1000 sessions (provided they are sending requests often), sending 1000 requests always to check up on a state that most probably have not changed for a while. Cheers, Anjana.
        Hide
        Deepal Jayasinghe added a comment - - edited

        Nope, you approach does not scale well. This will work if you have few sessions, but if the system have many sessions (say 1000), this approach does not work. The reason is Axis2 has to notify each and every SessionContext.

        The better approach would be to invalidate SessionContext if the ServiceGroup has been updated after creating the SessionContext. You can do this using a handler.

        Deepal

        Show
        Deepal Jayasinghe added a comment - - edited Nope, you approach does not scale well. This will work if you have few sessions, but if the system have many sessions (say 1000), this approach does not work. The reason is Axis2 has to notify each and every SessionContext. The better approach would be to invalidate SessionContext if the ServiceGroup has been updated after creating the SessionContext. You can do this using a handler. Deepal
        Hide
        Anjana Fernando added a comment -

        Hi Deepal,

        I just saw your comment, hope my approach is OK to handle the mentioned situation.

        Cheers,
        Anjana.

        Show
        Anjana Fernando added a comment - Hi Deepal, I just saw your comment, hope my approach is OK to handle the mentioned situation. Cheers, Anjana.
        Hide
        Anjana Fernando added a comment - - edited

        Hi,

        I'm attaching a patch to fix the above mentioned issue.

        This is done by making SessionContext an AxisObserver and adding it to AxisConfiguration. So when AxisConfiguration does a service / service group removal, SessionContext will know about it and remove it from the SessionContext - ServiceGroupContext / ServiceContext.

        Also, the observer list has been turned into a set, this is because, the observer (SessionContext) has to be added always to AxisConfiguration when a request comes in, since we don't know if this is a new session or not, we anyway have to check for the existence, so the straightforward way to do is, only allow unique objects as observer by making it a Set, so multiple additions of the same object will not cause it to be added multiple times as observers.

        I've run all Axis2 tests successfully with the patch.

        Cheers,
        Anjana.

        Show
        Anjana Fernando added a comment - - edited Hi, I'm attaching a patch to fix the above mentioned issue. This is done by making SessionContext an AxisObserver and adding it to AxisConfiguration. So when AxisConfiguration does a service / service group removal, SessionContext will know about it and remove it from the SessionContext - ServiceGroupContext / ServiceContext. Also, the observer list has been turned into a set, this is because, the observer (SessionContext) has to be added always to AxisConfiguration when a request comes in, since we don't know if this is a new session or not, we anyway have to check for the existence, so the straightforward way to do is, only allow unique objects as observer by making it a Set, so multiple additions of the same object will not cause it to be added multiple times as observers. I've run all Axis2 tests successfully with the patch. Cheers, Anjana.
        Hide
        Deepal Jayasinghe added a comment -

        In the case of transportsession, the session is managed by the transport and Axis2 does not know anything about it. So, Axis2 does not have a way to invalidate the session in the case of hot-update.

        Deepal

        Show
        Deepal Jayasinghe added a comment - In the case of transportsession, the session is managed by the transport and Axis2 does not know anything about it. So, Axis2 does not have a way to invalidate the session in the case of hot-update. Deepal

          People

          • Assignee:
            Unassigned
            Reporter:
            Anjana Fernando
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:

              Development