Hi Gary,
Thanks for your comment.
I put the update in V2. Please help to take alook.


B.R
/Thang

-----Original Message-----
From: Gary Lee <gary....@dektech.com.au> 
Sent: Monday, October 8, 2018 11:15 AM
To: Minh Hon Chau <minh.c...@dektech.com.au>; thang.nguyen 
<thang.d.ngu...@dektech.com.au>; hans.nordeb...@ericsson.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/1] amf: retry when implementer set returned ERR_EXIST 
[#2921]

Hi

One comment inline.

/Gary

On 3/10/18, 7:00 am, "Minh Hon Chau" <minh.c...@dektech.com.au> wrote:

    Hi Thang,
    
    It looks no harm to me if we do some retries to set implementer.
    
    You may need Gary/Hans to have a look, since it relates to consensus case.
    
    Thanks
    
    Minh
    
    
    On 31/08/18 20:36, thang.nguyen wrote:
    > In some condition, due to the AMF implementer released
    > was not synced to the remain SC. So the ERR_EXIST returned
    > when trying invoking implementer set on the remain SC.
    >
    > Add some retries on the ERR_EXIST.
    > ---
    >  src/amf/amfd/role.cc     | 27 ++++++++++++++++++++++++---
    >  src/clm/clmd/clms_imm.cc | 25 ++++++++++++++++---------
    >  2 files changed, 40 insertions(+), 12 deletions(-)
    >
    > diff --git a/src/amf/amfd/role.cc b/src/amf/amfd/role.cc
    > index 790983e..dc71665 100644
    > --- a/src/amf/amfd/role.cc
    > +++ b/src/amf/amfd/role.cc
    > @@ -49,12 +49,14 @@
    >  #include "base/osaf_utility.h"
    >  #include "role.h"
    >  #include "nid/agent/nid_api.h"
    > +#include "base/osaf_time.h"
    >  
    >  extern pthread_mutex_t imm_reinit_mutex;
    >  
    >  static uint32_t avd_role_failover(AVD_CL_CB *cb, SaAmfHAStateT role);
    >  static uint32_t avd_role_failover_qsd_actv(AVD_CL_CB *cb, SaAmfHAStateT 
role);
    >  static uint32_t avd_rde_set_role(SaAmfHAStateT role);
    > +extern struct ImmutilWrapperProfile immutilWrapperProfile;
    >  
    >  
/****************************************************************************\
    >   * Function: avd_role_change
    > @@ -259,12 +261,31 @@ done:
    >  
\**************************************************************************/
    >  uint32_t avd_active_role_initialization(AVD_CL_CB *cb, SaAmfHAStateT 
role) {
    >    uint32_t status = NCSCC_RC_FAILURE;
    > +  SaAisErrorT rc = SA_AIS_OK;
    >  
    >    TRACE_ENTER();
    >  
    > -  if (avd_imm_impl_set() != SA_AIS_OK) {
    > -    LOG_ER("avd_imm_impl_set FAILED");
    > -    goto done;
    > +  struct timespec time = {0, 0};
    > +  uint32_t no_of_retries = 0;
    > +  const uint32_t MAX_NO_RETRIES = immutilWrapperProfile.nTries;
    > +  osaf_millis_to_timespec(immutilWrapperProfile.retryInterval, &time);
    > +  /*
    > +   * Some retries in case takeover with consensus
    > +   * when OI not released/synced yet
    > +  */
    > +  while (++no_of_retries < MAX_NO_RETRIES) {
    > +    rc = avd_imm_impl_set();
    > +    if (rc != SA_AIS_OK) {
    > +      if (rc == SA_AIS_ERR_EXIST) {
    > +        osaf_nanosleep(&time);
    > +        continue;
    > +      } else {
    > +        LOG_ER("avd_imm_impl_set FAILED");
    > +        goto done;
    > +      }
    > +    } else {
    > +      break;
    > +    }
    >    }
    >  
    >    if (avd_imm_config_get() != NCSCC_RC_SUCCESS) {
    > diff --git a/src/clm/clmd/clms_imm.cc b/src/clm/clmd/clms_imm.cc
    > index 076890e..bfb9441 100644
    > --- a/src/clm/clmd/clms_imm.cc
    > +++ b/src/clm/clmd/clms_imm.cc
    > @@ -516,21 +516,28 @@ SaAisErrorT clms_imm_activate(CLMS_CB *cb) {
    >          goto done;
    >        }
    >  
    > +      /*
    > +      * Some retries in case takeover with consensus
    > +      * when OI not released/synced yet
    > +      */
    >        rc = immutil_saImmOiImplementerSet(
    >            cb->immOiHandle,
    >            const_cast<SaImmOiImplementerNameT>(IMPLEMENTER_NAME));

[GL] Can the code below be simplified? ie. create a separate if branch for 
SA_AIS_ERR_EXIST.

    > -      if (rc == SA_AIS_ERR_TIMEOUT || rc == SA_AIS_ERR_BAD_HANDLE) {
    > +      if (rc == SA_AIS_ERR_TIMEOUT || rc == SA_AIS_ERR_BAD_HANDLE
    > +          || rc == SA_AIS_ERR_EXIST) {
    >          LOG_WA("saImmOiImplementerSet returned %u", (unsigned)rc);
    >          usleep(sleep_delay_ms * 1000);
    >          msecs_waited += sleep_delay_ms;
    > -        saImmOiFinalize(cb->immOiHandle);
    > -        cb->immOiHandle = 0;
    > -        cb->imm_sel_obj = -1;
    > -        cb->is_impl_set = false;
    > -        if (clms_imm_init(clms_cb) != NCSCC_RC_SUCCESS) {
    > -          rc = SA_AIS_ERR_LIBRARY;
    > -          LOG_ER("clms_imm_init FAILED");
    > -          goto done;
    > +        if (rc != SA_AIS_ERR_EXIST) {
    > +          saImmOiFinalize(cb->immOiHandle);
    > +          cb->immOiHandle = 0;
    > +          cb->imm_sel_obj = -1;
    > +          cb->is_impl_set = false;
    > +          if (clms_imm_init(clms_cb) != NCSCC_RC_SUCCESS) {
    > +            rc = SA_AIS_ERR_LIBRARY;
    > +            LOG_ER("clms_imm_init FAILED");
    > +            goto done;
    > +          }
    >          }
    >          continue;
    >        }
    
    





_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to