Hi,

I guess you can change all_csis_in_restarting_stable(su, ignored_csi = 
nullptr). For the fix of this ticket, we can specified the csi which 
should be skipped.
Adding specific code for particular type of component should be 
restricted in comp clc state change, current avnd_comp_clc_st_chng_prc() 
there are some parts of state change handling very similar of npi/pi 
that makes it up a long function. The avnd_comp_clc_st_chng_prc() should 
only have abstract skeleton of state handling and calling external 
methods of PI/NPI SU class. That's just my suggestion and you can decide 
it since I'm not sure if we have plan to refactor it.

Thanks,
Minh

On 26/04/16 20:51, praveen malviya wrote:
> Hi Minh,
>
> I thought of that but su_evaluate_restarting_state() is for PI SU. For 
> NPI SU all_csis_in_restarting_state() is there. But this function 
> cannot be used in comp FSM as the current COMPCSI will be marked 
> RESTARTING in comp_restart_init() very lately. At most for comp FSM , 
> the for loop in discussion can be moved in a function like 
> all_csis_in_restarting_execpt_given(su, csi).
>
> Thanks,
> Praveen
>
> On 26-Apr-16 3:24 PM, minh chau wrote:
>> Tested the patch, ack with minor comment, please see inline
>> Thanks,
>> Minh
>>
>> On 22/04/16 19:36, praveen.malv...@oracle.com wrote:
>>> osaf/services/saf/amf/amfnd/clc.cc            |  85
>>> ++++++++++++++++++++++++++-
>>>   osaf/services/saf/amf/amfnd/include/avnd_su.h |   1 +
>>>   osaf/services/saf/amf/amfnd/susm.cc           |   2 +-
>>>   3 files changed, 86 insertions(+), 2 deletions(-)
>>>
>>>
>>> In reported problem, AMFD does not send state change notification for
>>> 1)SU presence state change from INSTANTIATED to RESTARTING and
>>> 2)SU presence state change from RESTARTING to INSTANTIATED
>>> when component restarts due to RESTART admin op on it or faults with
>>> comp-restart recovery.
>>>
>>> As per AMF spec, presence state of SU will be RESTARTING when all of
>>> its comp are in RESTARTING
>>> presence state. In this case when comp restarts due to fault or
>>> RESTART admin op on it, AMFND
>>> is not marking SU's presence state RESTARTING and SU remains in
>>> INSTANTIATED state. Since there
>>> is no change in presence state of SU, no state change notification for
>>> SU is sent.
>>>
>>> Patch fixes the problem by marking SU's presence state to RESTARTING
>>> when:
>>> 1)SU consists of a single restartable component and this comp restarts
>>> due to fault or RESTART
>>> admin op on it.
>>> 2)SU consists of all restartable components and all these components
>>> faults simultaneously.
>>>
>>> diff --git a/osaf/services/saf/amf/amfnd/clc.cc
>>> b/osaf/services/saf/amf/amfnd/clc.cc
>>> --- a/osaf/services/saf/amf/amfnd/clc.cc
>>> +++ b/osaf/services/saf/amf/amfnd/clc.cc
>>> @@ -993,7 +993,36 @@ uint32_t avnd_comp_clc_st_chng_prc(AVND_
>>>           if ((SA_AMF_PRESENCE_TERMINATING == prv_st) &&
>>> (SA_AMF_PRESENCE_TERMINATION_FAILED == final_st)) {
>>>               /* termination failed.. log it */
>>>           }
>>> -
>>> +        //Instantiated -> Restarting.
>>> +        if ((prv_st == SA_AMF_PRESENCE_INSTANTIATED) && (final_st ==
>>> SA_AMF_PRESENCE_RESTARTING)) {
>>> +            /*
>>> +               This presence state transition involving RESTARTING
>>> state may originate
>>> +               with or without any SU FSM event :
>>> +                   a)Without SU FSM event: when component is 
>>> restartable
>>> +                   and event is fault or RESTART admin op on 
>>> component.
>>> +                    b)With SU FSM event: when all comps are
>>> restartable and RESTART
>>> +                  admin op on SU.
>>> +               In the case b) SU FSM takes care of moving presence
>>> state of SU to
>>> +               RESTARTING when all of its components (all
>>> restartable) are
>>> +               in RESTARTING state at any given point of time.
>>> +
>>> +               In case a), SU FSM never gets triggered because
>>> restart of component
>>> +               is totally restricted to comp FSM. So in this case
>>> comp FSM itself
>>> +               will have to mark SU's presence state RESTARTING
>>> whenever all the components
>>> +               are in Restarting state. This can occur in:
>>> +                -confs with single restartable component because of
>>> fault with comp-restart
>>> +                 recovery or RESTART admin op on comp. OR
>>> +                -confs with all comp restartable when all comps
>>> faults with comp-restart recovery.
>>> +               So if I am here because of case a) check if I can mark
>>> SU RESTARTING.
>>> +             */
>>> +            if ((isRestartSet(comp->su) == false) &&
>>> +                ((m_AVND_COMP_IS_FAILED(comp)) || (comp->admin_oper
>>> == true)) &&
>>> +                (su_evaluate_restarting_state(comp->su) == true)) {
>>> +                TRACE_1("Comp RESTARTING due to comp-restart recovery
>>> or RESTART admin op");
>>> +                avnd_su_pres_state_set(cb, comp->su,
>>> SA_AMF_PRESENCE_RESTARTING);
>>> +            }
>>> +
>>> +        }
>>>           /* restarting -> instantiated */
>>>           if ((SA_AMF_PRESENCE_RESTARTING == prv_st) &&
>>> (SA_AMF_PRESENCE_INSTANTIATED == final_st)) {
>>>               /* reset the comp failed flag & set the oper state to
>>> enabled */
>>> @@ -1021,6 +1050,16 @@ uint32_t avnd_comp_clc_st_chng_prc(AVND_
>>>                   rc = avnd_comp_csi_reassign(cb, comp);
>>>                   if (NCSCC_RC_SUCCESS != rc)
>>>                       goto done;
>>> +                /*
>>> +                   Mark SU Instantiated when atleast one component
>>> moves to instantiated state.
>>> +                   Single comp restarting case or fault of all
>>> restartable comps with comp-restart
>>> +                   recovery.  For more details read in transition
>>> from INSTANTIATED to RESTARTING.
>>> +                 */
>>> +                if ((comp->su->pres == SA_AMF_PRESENCE_RESTARTING) &&
>>> +                        (isRestartSet(comp->su) == false)) {
>>> +                    TRACE_1("Comp INSTANTIATED due to comp-restart
>>> recovery or RESTART admin op");
>>> +                    avnd_su_pres_state_set(cb, comp->su,
>>> SA_AMF_PRESENCE_INSTANTIATED);
>>> +                }
>>>               }
>>>                 /* wild case */
>>> @@ -1193,6 +1232,22 @@ uint32_t avnd_comp_clc_st_chng_prc(AVND_
>>>               if (NCSCC_RC_SUCCESS != rc)
>>>                   goto done;
>>>               clear_error_report_alarm(comp);
>>> +            /*
>>> +               Mark SU Instantiated when atleast one component moves
>>> to instantiated state.
>>> +               Single comp restarting case or fault of all
>>> restartable comps with comp-restart
>>> +               recovery. Please read detailed explanation in state
>>> transition for PI
>>> +               comp in PI SU from INSTANTIATED to RESTARTING in this
>>> function.
>>> +             */
>>> +            if ((comp->su->pres == SA_AMF_PRESENCE_RESTARTING)
>>> +                    && (isRestartSet(comp->su) == false)) {
>>> +                TRACE_1("Comp INSTANTIATED due to comp-restart
>>> recovery or RESTART admin op");
>>> +                avnd_su_pres_state_set(cb, comp->su,
>>> SA_AMF_PRESENCE_INSTANTIATED);
>>> +            }
>>> +            csi =
>>> m_AVND_CSI_REC_FROM_COMP_DLL_NODE_GET(m_NCS_DBLIST_FIND_FIRST(&comp->csi_list));
>>>  
>>>
>>>
>>> +            //Mark CSI ASSIGNED in case of comp-restart recovery and
>>> RESTART admin op on comp.
>>> +            if ((isRestartSet(comp->su) == false) &&
>>> +
>>> (m_AVND_COMP_CSI_CURR_ASSIGN_STATE_IS_RESTARTING(csi)))
>>> +                m_AVND_COMP_CSI_CURR_ASSIGN_STATE_SET(csi,
>>> AVND_COMP_CSI_ASSIGN_STATE_ASSIGNED);
>>>           }
>>>             /* terminating -> uninstantiated */
>>> @@ -1223,6 +1278,34 @@ uint32_t avnd_comp_clc_st_chng_prc(AVND_
>>>               if (NCSCC_RC_SUCCESS != rc)
>>>                   goto done;
>>>           }
>>> +        //Instantiated -> Restarting.
>>> +                if ((prv_st == SA_AMF_PRESENCE_INSTANTIATED) &&
>>> (final_st == SA_AMF_PRESENCE_RESTARTING)) {
>>> +            //Please read detailed explanation in same state
>>> transition for PI comp in PI SU.
>>> +
>>> +            bool su_restarting = true;
>>> +            csi =
>>> m_AVND_CSI_REC_FROM_COMP_DLL_NODE_GET(m_NCS_DBLIST_FIND_FIRST(&comp->csi_list));
>>>  
>>>
>>>
>>> +            AVND_SU_SI_REC *si = csi->si; //NPI SU can have only one
>>> SI assigned to it.
>>> +                        if ((isRestartSet(comp->su) == false) &&
>>> + ((m_AVND_COMP_IS_FAILED(comp)) ||
>>> (comp->admin_oper == true))) {
>>> +                //Check if all COMPCSIs are in RESTARTING state
>>> execpt this compcsi.
>>> +                for (csi = (AVND_COMP_CSI_REC
>>> *)m_NCS_DBLIST_FIND_FIRST(&si->csi_list);
>>> +                    csi; csi = (AVND_COMP_CSI_REC
>>> *)m_NCS_DBLIST_FIND_NEXT(&csi->si_dll_node)) {
>>> +                    if (csi->comp == comp)
>>> +                        continue; /*skip this compcsi as CSI is
>>> lately moved to
>>> +                                RESTARTING state in
>>> comp_restart_initiate().*/
>>> +                    if
>>> (!m_AVND_COMP_CSI_CURR_ASSIGN_STATE_IS_RESTARTING(csi)) {
>>> +                        su_restarting = false;
>>> +                        break;
>>> +                    }
>>> +                }
>> [Minh]
>> Can we think of migrating the code of evaluate su restarting state for
>> NPI into su_evaluate_restarting_state()?
>>> +                if (su_restarting == true) {
>>> +                    TRACE_1("Comp RESTARTING due to comp-restart
>>> recovery or RESTART admin op");
>>> +                    avnd_su_pres_state_set(cb, comp->su,
>>> SA_AMF_PRESENCE_RESTARTING);
>>> +                }
>>> +            }
>>> +
>>> +                }
>>> +
>>>       }
>>>         /* when a comp moves from inst->orph, we need to delete the
>>> diff --git a/osaf/services/saf/amf/amfnd/include/avnd_su.h
>>> b/osaf/services/saf/amf/amfnd/include/avnd_su.h
>>> --- a/osaf/services/saf/amf/amfnd/include/avnd_su.h
>>> +++ b/osaf/services/saf/amf/amfnd/include/avnd_su.h
>>> @@ -422,4 +422,5 @@ bool all_csis_in_assigned_state(const AV
>>>   bool isAdminRestarted(const AVND_SU *su);
>>>   bool isFailed(const AVND_SU *su);
>>>   bool isRestartSet(const AVND_SU *su);
>>> +bool su_evaluate_restarting_state(AVND_SU *su);
>>>   #endif
>>> diff --git a/osaf/services/saf/amf/amfnd/susm.cc
>>> b/osaf/services/saf/amf/amfnd/susm.cc
>>> --- a/osaf/services/saf/amf/amfnd/susm.cc
>>> +++ b/osaf/services/saf/amf/amfnd/susm.cc
>>> @@ -2363,7 +2363,7 @@ uint32_t avnd_su_pres_inst_suterm_hdler(
>>>    * @param  ptr to su.
>>>    * @return  true/false.
>>>    */
>>> -static bool su_evaluate_restarting_state(AVND_SU *su)
>>> +bool su_evaluate_restarting_state(AVND_SU *su)
>>>   {
>>>       for (AVND_COMP *comp =
>>> m_AVND_COMP_FROM_SU_DLL_NODE_GET(m_NCS_DBLIST_FIND_FIRST(&su->comp_list)); 
>>>
>>>
>>>           comp;
>>>
>>
>


------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to