On 27-Aug-15 5:57 PM, Hans Nordebäck wrote:
> Hi Praveen,
>
> I forgot one question, is it correct that the standby amfd also updates imm?
No standby AMFD will not update to IMM.
Standby is only keeping some 200 elements (only for four classes) in the
job queue. If controller failover happens, then standby will update
at-least these 200 updates to IMM after becoming active. It is kept 200
because in this defect AMFD misseed around 152 updates to IMM because
controller failover happened before active controller could update to
IMM and standby does not maintain the queue.
As I mentioned in "TODO" in commit log, I am developing more refined
version of this patch in which standby controller will maintain job
queue for all the classes with no size limit. The mechanism will be:
When there are no IMM jobs left on the active controller it will send a
checkpointing message to standby AMFD to flush its job queue.
In the current patch standby, as applier, clearing its job queue in this
if condition, whenever it reaches above 200:
> + if (!avd_cb->is_implementer) {
> + check_and_flush_job_queue_standby_amfd();
> return JOB_EINVH;
> + }
Thanks
Praveen
/Thanks HansN
>
> -----Original Message-----
> From: Hans Nordebäck [mailto:[email protected]]
> Sent: den 10 augusti 2015 09:01
> To: [email protected]; [email protected]
> Cc: [email protected]
> Subject: Re: [devel] [PATCH 1 of 1] amfd: maintain runtime updates for su,
> comp, si and csi at standby [#1141]
>
> Ack, code review only, with one comment below/Thanks HansN
>
>
>
> -----Original Message-----
> From: [email protected] [mailto:[email protected]]
> Sent: den 31 juli 2015 14:35
> To: Hans Nordebäck; [email protected]
> Cc: [email protected]
> 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
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/opensaf-devel
>
------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
