Re: [devel] [PATCH 1/1] amfd: Update the assignment counters after restore absent assignment from imm [#2977]

2018-12-11 Thread Hans Nordebäck
Hi Minh,

ack, code review only/Thanks HansN

On 12/3/18 04:29, Minh Chau wrote:
> AMF performs headless recovery by syncing the assignments from AMFND(s) and
> re-create them in AMFD's db and IMM. Next step, AMFD compares the assignment
> objects from IMM and from AMFND(s) to figure out the on-going assignments
> that have been left over before headless and failover them, the assignments
> states/counters are also restored in this step. If all payloads come from
> headless without occurence of network split (legacy headless), IMM db in all
> payloads should be consistent, thus AMFD creates the IMM assignments normally
> without any problem. But if the payloads come from headless and there was a
> network split before, IMM appears often busy at the time AMFD creates the
> synced assignments in IMM. The assignment object creation is pending in the
> queue and executed later, but AMFD has missed to restore the assignment states
> and counters of the synced assignments at the time comparision between IMM
> and AMFND(s).
> Also in legacy headless, when both SCs go down, the assignment objects are
> still in IMM. Even IMM is busy, AMFD has not missed the counter updates.
>
> The patch moves the counter update after restoring absent assignment from IMM.
> ---
>   src/amf/amfd/siass.cc | 67 
> +--
>   1 file changed, 38 insertions(+), 29 deletions(-)
>
> diff --git a/src/amf/amfd/siass.cc b/src/amf/amfd/siass.cc
> index ffde7b1..8a2d217 100644
> --- a/src/amf/amfd/siass.cc
> +++ b/src/amf/amfd/siass.cc
> @@ -264,14 +264,48 @@ void avd_susi_read_headless_cached_rta(AVD_CL_CB *cb) {
> }
>   
>   #endif
> +} else {  // For ABSENT SUSI
> +  TRACE("Check absent SUSI, ha_state:'%u', fsm_state:'%u'", imm_ha_state,
> +imm_susi_fsm);
> +  if (avd_susi_validate_absent_assignment(su, si,
> +  imm_ha_state, imm_susi_fsm) == false) {
> +avd_saImmOiRtObjectDelete(Amf::to_string(&dn));
> +continue;
> +  }
> +  absent_susi = avd_susi_create(avd_cb, si, su, imm_ha_state, false,
> +  AVSV_SUSI_ACT_BASE);
> +  // Restore the fsm of this absent SUSI, which is used to determine
> +  // whether a SU should be added in SG's SUOperationList
> +  // Memorize it in temporary var @absent
> +  // The fsm of this SUSI will be changed to AVD_SU_SI_STATE_ABSENT
> +  // after restoring SUOperationList
> +  absent_susi->fsm = imm_susi_fsm;
> +  absent_susi->absent = true;
> +  if (absent_susi->si->saAmfSIAdminState == SA_AMF_ADMIN_LOCKED ||
> +  absent_susi->si->saAmfSIAdminState == SA_AMF_ADMIN_SHUTTING_DOWN) {
> +if (absent_susi->fsm == AVD_SU_SI_STATE_MODIFY &&
> +(absent_susi->state == SA_AMF_HA_QUIESCED ||
> +absent_susi->state == SA_AMF_HA_QUIESCING)) {
> +  m_AVD_SET_SG_ADMIN_SI(cb, si);
> +}
> +  }
> +}
> +  }
> +  (void)immutil_saImmOmSearchFinalize(searchHandle);
> +
> +  // Update all PRESENT SUSI, in case that a SUSI is missed to update because
> +  // it is not present in IMM
> +  for (const auto &value : *su_db) {
> +AVD_SU *su = value.second;
> +susi = su->list_of_susi;
> +while (susi != nullptr && susi->absent == false) {
> +  AVD_SI *si = susi->si;
> // validate SUSI assignments that are over assigned
> if (avd_susi_validate_excessive_assignment(susi) == true) {
>   susi->fsm = AVD_SU_SI_STATE_EXCESSIVE;
> }
> -
> // Checkpoint to add this SUSI
> m_AVSV_SEND_CKPT_UPDT_ASYNC_ADD(avd_cb, susi, AVSV_CKPT_AVD_SI_ASS);
> -
> // restore assignment counter
> if (susi->fsm == AVD_SU_SI_STATE_ASGN ||
> susi->fsm == AVD_SU_SI_STATE_ASGND ||
> @@ -296,36 +330,11 @@ void avd_susi_read_headless_cached_rta(AVD_CL_CB *cb) {
> // only restore if not done
> if (susi->su->su_on_node->admin_ng == nullptr)
>   avd_ng_restore_headless_states(cb, susi);
> -} else {  // For ABSENT SUSI
> -  TRACE("Check absent SUSI, ha_state:'%u', fsm_state:'%u'", imm_ha_state,
> -imm_susi_fsm);
> -  if (avd_susi_validate_absent_assignment(su, si,
> -  imm_ha_state, imm_susi_fsm) == false) {
> -avd_saImmOiRtObjectDelete(Amf::to_string(&dn));
> -continue;
> -  }
> -  absent_susi = avd_susi_create(avd_cb, si, su, imm_ha_state, false,
> -  AVSV_SUSI_ACT_BASE);
> -  // Restore the fsm of this absent SUSI, which is used to determine
> -  // whether a SU should be added in SG's SUOperationList
> -  // Memorize it in temporary var @absent
> -  // The fsm of this SUSI will be changed to AVD_SU_SI_STATE_ABSENT
> -  // after restoring SUOperationList
> -  absent_susi->fsm = imm_susi_fsm;
> -  absent_susi->absent = true;
> -  if (absent_susi->si->saAmfSIAdminState == SA_AMF_ADMIN_LOCKED ||
> -  absent_susi->si->saAmfSIAdminState == SA_AMF_ADMIN_SH

[devel] [PATCH 1/1] amfd: Update the assignment counters after restore absent assignment from imm [#2977]

2018-12-02 Thread Minh Chau
AMF performs headless recovery by syncing the assignments from AMFND(s) and
re-create them in AMFD's db and IMM. Next step, AMFD compares the assignment
objects from IMM and from AMFND(s) to figure out the on-going assignments
that have been left over before headless and failover them, the assignments
states/counters are also restored in this step. If all payloads come from
headless without occurence of network split (legacy headless), IMM db in all
payloads should be consistent, thus AMFD creates the IMM assignments normally
without any problem. But if the payloads come from headless and there was a
network split before, IMM appears often busy at the time AMFD creates the
synced assignments in IMM. The assignment object creation is pending in the
queue and executed later, but AMFD has missed to restore the assignment states
and counters of the synced assignments at the time comparision between IMM
and AMFND(s).
Also in legacy headless, when both SCs go down, the assignment objects are
still in IMM. Even IMM is busy, AMFD has not missed the counter updates.

The patch moves the counter update after restoring absent assignment from IMM.
---
 src/amf/amfd/siass.cc | 67 +--
 1 file changed, 38 insertions(+), 29 deletions(-)

diff --git a/src/amf/amfd/siass.cc b/src/amf/amfd/siass.cc
index ffde7b1..8a2d217 100644
--- a/src/amf/amfd/siass.cc
+++ b/src/amf/amfd/siass.cc
@@ -264,14 +264,48 @@ void avd_susi_read_headless_cached_rta(AVD_CL_CB *cb) {
   }
 
 #endif
+} else {  // For ABSENT SUSI
+  TRACE("Check absent SUSI, ha_state:'%u', fsm_state:'%u'", imm_ha_state,
+imm_susi_fsm);
+  if (avd_susi_validate_absent_assignment(su, si,
+  imm_ha_state, imm_susi_fsm) == false) {
+avd_saImmOiRtObjectDelete(Amf::to_string(&dn));
+continue;
+  }
+  absent_susi = avd_susi_create(avd_cb, si, su, imm_ha_state, false,
+  AVSV_SUSI_ACT_BASE);
+  // Restore the fsm of this absent SUSI, which is used to determine
+  // whether a SU should be added in SG's SUOperationList
+  // Memorize it in temporary var @absent
+  // The fsm of this SUSI will be changed to AVD_SU_SI_STATE_ABSENT
+  // after restoring SUOperationList
+  absent_susi->fsm = imm_susi_fsm;
+  absent_susi->absent = true;
+  if (absent_susi->si->saAmfSIAdminState == SA_AMF_ADMIN_LOCKED ||
+  absent_susi->si->saAmfSIAdminState == SA_AMF_ADMIN_SHUTTING_DOWN) {
+if (absent_susi->fsm == AVD_SU_SI_STATE_MODIFY &&
+(absent_susi->state == SA_AMF_HA_QUIESCED ||
+absent_susi->state == SA_AMF_HA_QUIESCING)) {
+  m_AVD_SET_SG_ADMIN_SI(cb, si);
+}
+  }
+}
+  }
+  (void)immutil_saImmOmSearchFinalize(searchHandle);
+
+  // Update all PRESENT SUSI, in case that a SUSI is missed to update because
+  // it is not present in IMM
+  for (const auto &value : *su_db) {
+AVD_SU *su = value.second;
+susi = su->list_of_susi;
+while (susi != nullptr && susi->absent == false) {
+  AVD_SI *si = susi->si;
   // validate SUSI assignments that are over assigned
   if (avd_susi_validate_excessive_assignment(susi) == true) {
 susi->fsm = AVD_SU_SI_STATE_EXCESSIVE;
   }
-
   // Checkpoint to add this SUSI
   m_AVSV_SEND_CKPT_UPDT_ASYNC_ADD(avd_cb, susi, AVSV_CKPT_AVD_SI_ASS);
-
   // restore assignment counter
   if (susi->fsm == AVD_SU_SI_STATE_ASGN ||
   susi->fsm == AVD_SU_SI_STATE_ASGND ||
@@ -296,36 +330,11 @@ void avd_susi_read_headless_cached_rta(AVD_CL_CB *cb) {
   // only restore if not done
   if (susi->su->su_on_node->admin_ng == nullptr)
 avd_ng_restore_headless_states(cb, susi);
-} else {  // For ABSENT SUSI
-  TRACE("Check absent SUSI, ha_state:'%u', fsm_state:'%u'", imm_ha_state,
-imm_susi_fsm);
-  if (avd_susi_validate_absent_assignment(su, si,
-  imm_ha_state, imm_susi_fsm) == false) {
-avd_saImmOiRtObjectDelete(Amf::to_string(&dn));
-continue;
-  }
-  absent_susi = avd_susi_create(avd_cb, si, su, imm_ha_state, false,
-  AVSV_SUSI_ACT_BASE);
-  // Restore the fsm of this absent SUSI, which is used to determine
-  // whether a SU should be added in SG's SUOperationList
-  // Memorize it in temporary var @absent
-  // The fsm of this SUSI will be changed to AVD_SU_SI_STATE_ABSENT
-  // after restoring SUOperationList
-  absent_susi->fsm = imm_susi_fsm;
-  absent_susi->absent = true;
-  if (absent_susi->si->saAmfSIAdminState == SA_AMF_ADMIN_LOCKED ||
-  absent_susi->si->saAmfSIAdminState == SA_AMF_ADMIN_SHUTTING_DOWN) {
-if (absent_susi->fsm == AVD_SU_SI_STATE_MODIFY &&
-(absent_susi->state == SA_AMF_HA_QUIESCED ||
-absent_susi->state == SA_AMF_HA_QUIESCING)) {
-  m_AVD_SET_SG_ADMIN_SI(cb, si);
-}
-  }
+
+  susi = susi->su_next;
 }