Ack, code review only, with one comment below/Thanks HansN
-----Original Message----- From: praveen.malv...@oracle.com [mailto:praveen.malv...@oracle.com] Sent: den 31 juli 2015 14:35 To: Hans Nordebäck; nagendr...@oracle.com Cc: opensaf-devel@lists.sourceforge.net Subject: [PATCH 1 of 1] amfd: maintain runtime updates for su, comp, si and csi at standby [#1141] osaf/services/saf/amf/amfd/ckpt_dec.cc | 10 ++++++ osaf/services/saf/amf/amfd/csi.cc | 3 - osaf/services/saf/amf/amfd/imm.cc | 53 +++++++++++++++++++++++++++++-- osaf/services/saf/amf/amfd/include/imm.h | 2 + osaf/services/saf/amf/amfd/siass.cc | 9 ----- 5 files changed, 61 insertions(+), 16 deletions(-) Amfd was killed when lock operation on su was going on. After successful failover, susi of locked su is still shown. In the reported issue, lock operation was successful and active AMFD was able to delete SUSIs and COMPCSIs in its database before got killed. AMFD was killed when it was performing run time updates for su, comp, si and csi to imm. SUSI and COMPCSI of locked Su is still shown because active AMFD was killed without performing all the updates and since these updates are not maintained at standby AMFD, even after failover imm was not updated for the same. Patch proposes a solution, in which standby AMFD will maintain run time updates for su, comp, si and csi. However the size of the job queue will not execeed more that 200. Job queue will be emptied iat standby if it crosses this max size limit. The value of 200 is taken from the reproted issue in which AMFD missed around 150 updates to imm for comp, su, csi and csi during failover. TODO: This solution has following limitations: 1)Fix size(200) of job queue at standby. 2)Standby maintains updates for only su,comp, csi and si. 3)New active may still miss some updates if updates are more than 200. Improved solution will be something like this: 1)Standby AMFD will maintain updates for all the classes and with no size limitation. 2)Whenever active controller is finished updating to imm and there are no more jobs to update, it will ask standby AMFD to flush its job queue. Implementaion of this will require new messaging (AMFD version update) and thus will be backward incompatible. diff --git a/osaf/services/saf/amf/amfd/ckpt_dec.cc b/osaf/services/saf/amf/amfd/ckpt_dec.cc --- a/osaf/services/saf/amf/amfd/ckpt_dec.cc +++ b/osaf/services/saf/amf/amfd/ckpt_dec.cc @@ -1372,6 +1372,8 @@ static uint32_t dec_su_admin_state(AVD_C cb->async_updt_cnt.su_updt++; + avd_saImmOiRtObjectUpdate(&su->name, "saAmfSUAdminState", + SA_IMM_ATTR_SAUINT32T, &su->saAmfSUAdminState); TRACE_LEAVE2("'%s', saAmfSUAdminState=%u, su_updt:%d", name.value, su->saAmfSUAdminState, cb->async_updt_cnt.su_updt); return NCSCC_RC_SUCCESS; @@ -1403,6 +1405,8 @@ static uint32_t dec_su_readiness_state(A cb->async_updt_cnt.su_updt++; + avd_saImmOiRtObjectUpdate(&su->name, "saAmfSUReadinessState", + SA_IMM_ATTR_SAUINT32T, &su->saAmfSuReadinessState); TRACE_LEAVE2("'%s', saAmfSuReadinessState=%u, su_updt:%d", name.value, su->saAmfSuReadinessState, cb->async_updt_cnt.su_updt); return NCSCC_RC_SUCCESS; @@ -1434,6 +1438,8 @@ static uint32_t dec_su_pres_state(AVD_CL cb->async_updt_cnt.su_updt++; + avd_saImmOiRtObjectUpdate(&su->name, "saAmfSUPresenceState", + SA_IMM_ATTR_SAUINT32T, &su->saAmfSUPresenceState); TRACE_LEAVE2("'%s', saAmfSUPresenceState=%u, su_updt:%d", name.value, su->saAmfSUPresenceState, cb->async_updt_cnt.su_updt); return NCSCC_RC_SUCCESS; @@ -2082,6 +2088,8 @@ static uint32_t dec_comp_readiness_state cb->async_updt_cnt.comp_updt++; + avd_saImmOiRtObjectUpdate(&comp_struct->comp_info.name, "saAmfCompReadinessState", + SA_IMM_ATTR_SAUINT32T, &comp_struct->saAmfCompReadinessState); TRACE_LEAVE2("status '%u'", status); return status; } @@ -2132,6 +2140,8 @@ static uint32_t dec_comp_pres_state(AVD_ comp_struct->saAmfCompPresenceState = comp_ptr->saAmfCompPresenceState; cb->async_updt_cnt.comp_updt++; + avd_saImmOiRtObjectUpdate(&comp_struct->comp_info.name, "saAmfCompPresenceState", + SA_IMM_ATTR_SAUINT32T, &comp_struct->saAmfCompPresenceState); TRACE_LEAVE2("status '%u'", status); return status; diff --git a/osaf/services/saf/amf/amfd/csi.cc b/osaf/services/saf/amf/amfd/csi.cc --- a/osaf/services/saf/amf/amfd/csi.cc +++ b/osaf/services/saf/amf/amfd/csi.cc @@ -1175,9 +1175,6 @@ static void avd_delete_csiassignment_fro { SaNameT dn; - if (avd_cb->avail_state_avd != SA_AMF_HA_ACTIVE) - return; - avsv_create_association_class_dn(comp_dn, csi_dn, "safCSIComp", &dn); TRACE("Deleting %s", dn.value); diff --git a/osaf/services/saf/amf/amfd/imm.cc b/osaf/services/saf/amf/amfd/imm.cc --- a/osaf/services/saf/amf/amfd/imm.cc +++ b/osaf/services/saf/amf/amfd/imm.cc @@ -73,6 +73,7 @@ static char *StrDup(const char *s) std::strcpy(c,s); return c; } +uint32_t const MAX_JOB_SIZE_AT_STANDBY = 200; // Job::~Job() @@ -294,6 +295,20 @@ Job* Fifo::dequeue() return tmp; } +/** + * @brief As of now standby AMFD will maintain immjobs for object of few classes. + * Flush all the jobs without updating to imm if MAX_JOB_SIZE_AT_STANDBY is + * reached. + * + */ +void check_and_flush_job_queue_standby_amfd(void) +{ + if (Fifo::size() >= MAX_JOB_SIZE_AT_STANDBY) { + TRACE("Emptying imm job queue of size:%u",Fifo::size()); + Fifo::empty(); + } +} + // AvdJobDequeueResultT Fifo::execute(SaImmOiHandleT immOiHandle) { @@ -304,8 +319,10 @@ AvdJobDequeueResultT Fifo::execute(SaImm if (!avd_cb->active_services_exist) return JOB_ETRYAGAIN; - if (!avd_cb->is_implementer) + if (!avd_cb->is_implementer) { + check_and_flush_job_queue_standby_amfd(); return JOB_EINVH; + } if ((ajob = peek()) == NULL) return JOB_ENOTEXIST; @@ -332,6 +349,12 @@ void Fifo::empty() TRACE_LEAVE(); } + +uint32_t Fifo::size() +{ + return imm_job_.size(); +} + // std::queue<Job*> Fifo::imm_job_; // @@ -1531,6 +1554,25 @@ SaAisErrorT avd_saImmOiRtObjectUpdate_sy } /** + * @brief As of now standby AMFD will maintain immjobs for object of few classes. + * This function checks if immjobs for this object can be maintained at standby. + * + * @param dn (ptr to SaNameT) + * @return true/false + */ +bool check_to_create_immjob_at_standby_amfd(const SaNameT *dn) { + + AVSV_AMF_CLASS_ID class_type = AVSV_SA_AMF_CLASS_INVALID; + class_type = object_name_to_class_type(dn); + if ((class_type = AVSV_SA_AMF_SU) || (class_type = AVSV_SA_AMF_COMP) || [HansN] I guess it should be a compare above, not assignment + (class_type == AVSV_SA_AMF_SI_ASSIGNMENT) || + (class_type == AVSV_SA_AMF_CSI_ASSIGNMENT)) + return true; + return false; +} + +/** * Queue an IM object update to be executed later, NON BLOCKING * @param dn * @param attributeName @@ -1544,7 +1586,8 @@ void avd_saImmOiRtObjectUpdate(const SaN size_t sz; - if (avd_cb->avail_state_avd != SA_AMF_HA_ACTIVE) + if ((avd_cb->avail_state_avd != SA_AMF_HA_ACTIVE) && + (check_to_create_immjob_at_standby_amfd(dn) == false)) return; ImmObjUpdate *ajob = new ImmObjUpdate; @@ -1573,7 +1616,8 @@ void avd_saImmOiRtObjectCreate(const cha { TRACE_ENTER2("%s %s", className, parentName->value); - if (avd_cb->avail_state_avd != SA_AMF_HA_ACTIVE) + if ((avd_cb->avail_state_avd != SA_AMF_HA_ACTIVE) && + (check_to_create_immjob_at_standby_amfd(parentName) == false)) return; ImmObjCreate* ajob = new ImmObjCreate; @@ -1595,7 +1639,8 @@ void avd_saImmOiRtObjectDelete(const SaN { TRACE_ENTER2("%s", dn->value); - if (avd_cb->avail_state_avd != SA_AMF_HA_ACTIVE) + if ((avd_cb->avail_state_avd != SA_AMF_HA_ACTIVE) && + (check_to_create_immjob_at_standby_amfd(dn) == false)) return; ImmObjDelete *ajob = new ImmObjDelete; diff --git a/osaf/services/saf/amf/amfd/include/imm.h b/osaf/services/saf/amf/amfd/include/imm.h --- a/osaf/services/saf/amf/amfd/include/imm.h +++ b/osaf/services/saf/amf/amfd/include/imm.h @@ -119,6 +119,7 @@ public: static void empty(); + static uint32_t size(); private: static std::queue<Job*> imm_job_; }; @@ -165,5 +166,6 @@ void report_ccb_validation_error(const C void report_admin_op_error(SaImmOiHandleT immOiHandle, SaInvocationT invocation, SaAisErrorT result, struct admin_oper_cbk *pend_cbk, const char *format, ...) __attribute__ ((format(printf, 5, 6))); +extern void check_and_flush_job_queue_standby_amfd(void); #endif diff --git a/osaf/services/saf/amf/amfd/siass.cc b/osaf/services/saf/amf/amfd/siass.cc --- a/osaf/services/saf/amf/amfd/siass.cc +++ b/osaf/services/saf/amf/amfd/siass.cc @@ -73,9 +73,6 @@ static void avd_create_susi_in_imm(SaAmf NULL }; - if (avd_cb->avail_state_avd != SA_AMF_HA_ACTIVE) - return; - avsv_create_association_class_dn(su_dn, NULL, "safSISU", &dn); avd_saImmOiRtObjectCreate("SaAmfSIAssignment", si_dn, attrValues); } @@ -89,9 +86,6 @@ static void avd_delete_siassignment_from { SaNameT dn; - if (avd_cb->avail_state_avd != SA_AMF_HA_ACTIVE) - return; - avsv_create_association_class_dn(su_dn, si_dn, "safSISU", &dn); avd_saImmOiRtObjectDelete(&dn); } @@ -107,9 +101,6 @@ void avd_susi_update(AVD_SU_SI_REL *susi SaNameT dn; AVD_COMP_CSI_REL *compcsi; - if (avd_cb->avail_state_avd != SA_AMF_HA_ACTIVE) - return; - avsv_create_association_class_dn(&susi->su->name, &susi->si->name, "safSISU", &dn); TRACE("HA State %s of %s for %s", avd_ha_state[ha_state], ------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
