ack, code review only. One comment though,
it would be good in to reduce the cyclomatic complexity in amf as mentioned in the refactoring tickets. This is particularly true for e.g clc.cc. Last time I run pmccabe the numbers for clc.cc were around 115 which is extraordinary high, now the number is 181, (not introduced by this patch, but the patch adds a small amount to the numbers). It would be good to re-factor the complex if statements to reusable functions instead. Numbers around 30-40 means the function wil be harder to understand and more difficult to get acceptable code coverage. pmccabe clc.cc | sort -nr | head -10 181 181 255 927 566 clc.cc(927): avnd_comp_clc_st_chng_prc 37 39 68 758 154 clc.cc(758): avnd_comp_clc_fsm_run 23 23 34 2024 85 clc.cc(2024): avnd_comp_clc_inst_clean_hdler /Regards HansN On 09/29/2016 12:08 PM, [email protected] wrote: > osaf/services/saf/amf/amfnd/clc.cc | 8 ++++++++ > 1 files changed, 8 insertions(+), 0 deletions(-) > > > SU stuck in TERMINATING state when comp fails to instantiate as part of > repair. > Repair was performed after comp-failover recovery. > > Comp faults with comp-failover recovery. After removal of assignments, AMFNFD > tries to repair component by trying to instantiate it. Since instantiate > scripit > is not present, comp instantiation fails and AMFND cleans up it succesfully. > After > successful cleanup AMFND niether tries to instantiate comp again not it marks > comp > and SU to INST_FAILED state. AMFND should mark the comp and SU to INST_FAILED > state > after finishing instantiation of comp MAX_TRY times. If the value is not > configured > for the comp then AMFND should honour the default value of > saAmfNumMaxInstantiateWithoutDelay=2. > > Patch honours saAmfNumMaxInstantiateWithoutDelay and if comp instantiation > fails after > these many retries then both comp and SU moves to INST_FAILED state. > > 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 > @@ -1785,6 +1785,14 @@ static bool is_failed_comp_eligible_for_ > /*A failed component is eligible for instantiation > in the context of comp-restart recovery.*/ > return true; > + } else {//SU is failed. > + /*As a part of repair comp instantiation failed > + and it was cleaned up again.*/ > + if (m_AVND_COMP_IS_FAILED(comp) && !comp->csi_list.n_nodes && > + (!m_AVND_SU_IS_ADMN_TERM(comp->su)) && > + (avnd_cb->oper_state == SA_AMF_OPERATIONAL_ENABLED) && > + (!m_AVND_IS_SHUTTING_DOWN(avnd_cb)) && > !sufailover_in_progress(comp->su)) > + return true; > } > } > ------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
