Hi zoran,

Reviewed and tested the patch.

As commented earlier the following case is working :

3. If the Implementerset has set timeout to 8. For the second run the
application decides to change it to 10.
     The second run timeout is not reflected because, class already has
class implementer.


On top of AndersBj comments, following is the additional comment

while testing immoitest the following tests are giving core :
[root@Slot-4 ~]# immoitest 4 4

Suite 4: Configuration Objects Implementer
Segmentation fault (core dumped)
[root@Slot-4 ~]# immoitest 4 5

Suite 4: Configuration Objects Implementer
Segmentation fault (core dumped)


/Neel.


On Monday 09 June 2014 08:14 PM, Anders Bjornerstedt wrote:
> I have to NACK this patch due to some issues commented below.
> Mainly performance related.
>
> Zoran Milinkovic wrote:
>>  osaf/libs/common/immsv/immsv_evt.c |  56 +++++++++++++-
>>  osaf/libs/common/immsv/include/immsv_evt.h       |   4 +
>>  osaf/libs/common/immsv/include/immsv_evt_model.h |   1 +
>>  osaf/services/saf/immsv/immd/immd_evt.c          |   9 ++-
>>  osaf/services/saf/immsv/immnd/ImmModel.cc        |  95 
>> ++++++++++++++++++-----
>>  osaf/services/saf/immsv/immnd/ImmModel.hh        |   6 +-
>>  osaf/services/saf/immsv/immnd/immnd_evt.c        |  46 +++++++++-
>>  osaf/services/saf/immsv/immnd/immnd_init.h       |   8 +-
>>  8 files changed, 188 insertions(+), 37 deletions(-)
>>
>>
>> When an implementer is set, the configurable timeout is sent to IMM 
>> service.
>> The configurable timeout is used for calculating timeouts for OI 
>> callbacks and waiting on search replies from RTA update callback.
>>
>> diff --git a/osaf/libs/common/immsv/immsv_evt.c 
>> b/osaf/libs/common/immsv/immsv_evt.c
>> --- a/osaf/libs/common/immsv/immsv_evt.c
>> +++ b/osaf/libs/common/immsv/immsv_evt.c
>> @@ -64,6 +64,7 @@ static const char *immd_evt_names[] = {
>>      "IMMD_EVT_ND2D_FEVS_REQ_2",
>>      "IMMD_EVT_ND2D_LOADING_COMPLETED",
>>      "IMMD_EVT_ND2D_2PBE_PRELOAD",
>> +    "IMMD_EVT_ND2D_IMPLSET_REQ_2",
>>      "undefined (high)"
>>  };
>>
>> @@ -173,6 +174,8 @@ static const char *immnd_evt_names[] = {
>>      "IMMND_EVT_A2ND_CL_TIMEOUT",
>>      "IMMND_EVT_A2ND_ACCESSOR_GET",
>>      "IMMND_EVT_A2ND_CCB_VALIDATE",    /* saImmOmCcbValidate */
>> +    "IMMND_EVT_A2ND_OI_IMPL_SET_2", /* saImmOiImplementerSet */
>> +    "IMMND_EVT_D2ND_IMPLSET_RSP_2",    /* Implementer set reply from 
>> D with impl id */
>>      "undefined (high)"
>>  };
>>
>> @@ -1511,7 +1514,8 @@ static uint32_t immsv_evt_enc_sublevels(
>>              (i_evt->info.immd.type == IMMD_EVT_ND2D_FEVS_REQ_2)) {
>>              IMMSV_OCTET_STRING *os = 
>> &(i_evt->info.immd.info.fevsReq.msg);
>>              immsv_evt_enc_inline_string(o_ub, os);
>> -        } else if (i_evt->info.immd.type == 
>> IMMD_EVT_ND2D_IMPLSET_REQ) {
>> +        } else if ((i_evt->info.immd.type == 
>> IMMD_EVT_ND2D_IMPLSET_REQ) ||
>> +                (i_evt->info.immd.type == 
>> IMMD_EVT_ND2D_IMPLSET_REQ_2)) {
>>              IMMSV_OCTET_STRING *os = 
>> &(i_evt->info.immd.info.impl_set.r.impl_name);
>>              if(!immsv_evt_enc_inline_text(__LINE__, o_ub, os)) {
>>                  return NCSCC_RC_OUT_OF_MEM;
>> @@ -1564,7 +1568,9 @@ static uint32_t immsv_evt_enc_sublevels(
>>              IMMSV_OCTET_STRING *os = 
>> &(i_evt->info.immnd.info.fevsReq.msg);
>>              immsv_evt_enc_inline_string(o_ub, os);
>>          } else if ((i_evt->info.immnd.type == 
>> IMMND_EVT_A2ND_OI_IMPL_SET) ||
>> +               (i_evt->info.immnd.type == 
>> IMMND_EVT_A2ND_OI_IMPL_SET_2) ||
>>                 (i_evt->info.immnd.type == 
>> IMMND_EVT_D2ND_IMPLSET_RSP) ||
>> +               (i_evt->info.immnd.type == 
>> IMMND_EVT_D2ND_IMPLSET_RSP_2) ||
>>                 (i_evt->info.immnd.type == 
>> IMMND_EVT_A2ND_OI_CL_IMPL_SET) ||
>>                 (i_evt->info.immnd.type == 
>> IMMND_EVT_A2ND_OI_CL_IMPL_REL) ||
>>                 (i_evt->info.immnd.type == 
>> IMMND_EVT_A2ND_OI_OBJ_IMPL_SET) ||
>> @@ -2141,7 +2147,8 @@ static uint32_t immsv_evt_dec_sublevels(
>>              (o_evt->info.immd.type == IMMD_EVT_ND2D_FEVS_REQ_2)) {
>>              IMMSV_OCTET_STRING *os = 
>> &(o_evt->info.immd.info.fevsReq.msg);
>>              immsv_evt_dec_inline_string(i_ub, os);
>> -        } else if (o_evt->info.immd.type == 
>> IMMD_EVT_ND2D_IMPLSET_REQ) {
>> +        } else if ((o_evt->info.immd.type == 
>> IMMD_EVT_ND2D_IMPLSET_REQ) ||
>> +                (o_evt->info.immd.type == 
>> IMMD_EVT_ND2D_IMPLSET_REQ_2)) {
>>              IMMSV_OCTET_STRING *os = 
>> &(o_evt->info.immd.info.impl_set.r.impl_name);
>>              immsv_evt_dec_inline_string(i_ub, os);
>>          } else if (o_evt->info.immd.type == 
>> IMMD_EVT_ND2D_OI_OBJ_MODIFY) {
>> @@ -2177,7 +2184,9 @@ static uint32_t immsv_evt_dec_sublevels(
>>              immsv_evt_dec_inline_string(i_ub, os);
>>          } else
>>              if ((o_evt->info.immnd.type == 
>> IMMND_EVT_A2ND_OI_IMPL_SET) ||
>> +            (o_evt->info.immnd.type == IMMND_EVT_A2ND_OI_IMPL_SET_2) ||
>>              (o_evt->info.immnd.type == IMMND_EVT_D2ND_IMPLSET_RSP) ||
>> +            (o_evt->info.immnd.type == IMMND_EVT_D2ND_IMPLSET_RSP_2) ||
>>              (o_evt->info.immnd.type == 
>> IMMND_EVT_A2ND_OI_CL_IMPL_SET) ||
>>              (o_evt->info.immnd.type == 
>> IMMND_EVT_A2ND_OI_CL_IMPL_REL) ||
>>              (o_evt->info.immnd.type == 
>> IMMND_EVT_A2ND_OI_OBJ_IMPL_SET) ||
>> @@ -2949,6 +2958,7 @@ static uint32_t immsv_evt_enc_toplevel(I
>>              break;
>>
>>          case IMMD_EVT_ND2D_IMPLSET_REQ:    /*OiImplementerSet */
>> +        case IMMD_EVT_ND2D_IMPLSET_REQ_2:    /*OiImplementerSet */
>>              IMMSV_RSRV_SPACE_ASSERT(p8, o_ub, 8);
>>              ncs_encode_64bit(&p8, immdevt->info.impl_set.reply_dest);
>>              ncs_enc_claim_space(o_ub, 8);
>> @@ -2966,6 +2976,12 @@ static uint32_t immsv_evt_enc_toplevel(I
>>              ncs_encode_32bit(&p8, immdevt->info.impl_set.r.scope);
>>              ncs_enc_claim_space(o_ub, 4);
>>
>> +            if(immdevt->type == IMMD_EVT_ND2D_IMPLSET_REQ_2) {
>> +                IMMSV_RSRV_SPACE_ASSERT(p8, o_ub, 4);
>> +                ncs_encode_32bit(&p8, 
>> immdevt->info.impl_set.r.oi_timeout);
>> +                ncs_enc_claim_space(o_ub, 4);
>> +            }
>> +
>>              /*intentional fall through - encode impl_id */
>>          case IMMD_EVT_ND2D_DISCARD_IMPL:    /*Internal discard 
>> implementer message */
>>
>> @@ -3274,6 +3290,7 @@ static uint32_t immsv_evt_enc_toplevel(I
>>              break;
>>
>>          case IMMND_EVT_A2ND_OI_IMPL_SET:    /* saImmOiImplementerSet */
>> +        case IMMND_EVT_A2ND_OI_IMPL_SET_2:    /* 
>> saImmOiImplementerSet */
>>              IMMSV_RSRV_SPACE_ASSERT(p8, o_ub, 8);
>>              ncs_encode_64bit(&p8, immndevt->info.implSet.client_hdl);
>>              ncs_enc_claim_space(o_ub, 8);
>> @@ -3284,6 +3301,12 @@ static uint32_t immsv_evt_enc_toplevel(I
>>              /* immndevt->info.implSet.impl_name.buf encoded by 
>> sublevel */
>>
>>              /*skip scope & impl_id */
>> +
>> +            if(immndevt->type == IMMND_EVT_A2ND_OI_IMPL_SET_2) {
>> +                IMMSV_RSRV_SPACE_ASSERT(p8, o_ub, 4);
>> +                ncs_encode_32bit(&p8, 
>> immndevt->info.implSet.oi_timeout);
>> +                ncs_enc_claim_space(o_ub, 4);
>> +            }
>>              break;
>>
>>              /*Fevs call IMMA->IMMD->IMMNDs have to suport non-flat 
>> encoding */
>> @@ -3760,6 +3783,7 @@ static uint32_t immsv_evt_enc_toplevel(I
>>              break;
>>
>>          case IMMND_EVT_D2ND_IMPLSET_RSP:    /* Impl set reply from D 
>> with impl id */
>> +        case IMMND_EVT_D2ND_IMPLSET_RSP_2:
>>              IMMSV_RSRV_SPACE_ASSERT(p8, o_ub, 8);
>>              ncs_encode_64bit(&p8, immndevt->info.implSet.client_hdl);
>>              ncs_enc_claim_space(o_ub, 8);
>> @@ -3776,6 +3800,12 @@ static uint32_t immsv_evt_enc_toplevel(I
>>              IMMSV_RSRV_SPACE_ASSERT(p8, o_ub, 4);
>>              ncs_encode_32bit(&p8, immndevt->info.implSet.scope);
>>              ncs_enc_claim_space(o_ub, 4);
>> +
>> +            if(immndevt->type == IMMND_EVT_D2ND_IMPLSET_RSP_2) {
>> +                IMMSV_RSRV_SPACE_ASSERT(p8, o_ub, 4);
>> +                ncs_encode_32bit(&p8, 
>> immndevt->info.implSet.oi_timeout);
>> +                ncs_enc_claim_space(o_ub, 4);
>> +            }
>>              break;
>>
>>          case IMMND_EVT_D2ND_DISCARD_IMPL:    /* Discard implementer 
>> broadcast to NDs */
>> @@ -4263,6 +4293,7 @@ static uint32_t immsv_evt_dec_toplevel(N
>>              break;
>>
>>          case IMMD_EVT_ND2D_IMPLSET_REQ:    /*OiImplementerSet */
>> +        case IMMD_EVT_ND2D_IMPLSET_REQ_2:    /*OiImplementerSet */
>>              IMMSV_FLTN_SPACE_ASSERT(p8, local_data, i_ub, 8);
>>              immdevt->info.impl_set.reply_dest = ncs_decode_64bit(&p8);
>>              ncs_dec_skip_space(i_ub, 8);
>> @@ -4280,6 +4311,14 @@ static uint32_t immsv_evt_dec_toplevel(N
>>              immdevt->info.impl_set.r.scope = ncs_decode_32bit(&p8);
>>              ncs_dec_skip_space(i_ub, 4);
>>
>> +            if(immdevt->type == IMMD_EVT_ND2D_IMPLSET_REQ_2) {
>> +                IMMSV_FLTN_SPACE_ASSERT(p8, local_data, i_ub, 4);
>> +                immdevt->info.impl_set.r.oi_timeout = 
>> ncs_decode_32bit(&p8);
>> +                ncs_dec_skip_space(i_ub, 4);
>> +            } else {
>> +                immdevt->info.impl_set.r.oi_timeout = 0;
>> +            }
> We have an else branch above for the decode of 
> IMMD_EVT_ND2D_IMPLSET_REQ_2
> but not for ......
>> +
>>              /*intentional fall through - decode impl_id */
>>          case IMMD_EVT_ND2D_DISCARD_IMPL:    /*Internal discard 
>> implementer message */
>>
>> @@ -4603,6 +4642,7 @@ static uint32_t immsv_evt_dec_toplevel(N
>>              break;
>>
>>          case IMMND_EVT_A2ND_OI_IMPL_SET:    /* saImmOiImplementerSet */
>> +        case IMMND_EVT_A2ND_OI_IMPL_SET_2:    /* 
>> saImmOiImplementerSet */
>>              IMMSV_FLTN_SPACE_ASSERT(p8, local_data, i_ub, 8);
>>              immndevt->info.implSet.client_hdl = ncs_decode_64bit(&p8);
>>              ncs_dec_skip_space(i_ub, 8);
>> @@ -4613,6 +4653,11 @@ static uint32_t immsv_evt_dec_toplevel(N
>>              /* immndevt->info.implSet.impl_name.buf decoded by 
>> sublevel */
>>
>>              /*skip scope & impl_id */
>> +            if(immndevt->type == IMMND_EVT_A2ND_OI_IMPL_SET_2) {
>> +                IMMSV_FLTN_SPACE_ASSERT(p8, local_data, i_ub, 4);
>> +                immndevt->info.implSet.oi_timeout = 
>> ncs_decode_32bit(&p8);
>> +                ncs_dec_skip_space(i_ub, 4);
>> +            }
> The  IMMND_EVT_A2ND_OI_IMPL_SET_2
> and ...
>>              break;
>>
>>              /*Fevs call IMMA->IMMD->IMMNDs have to suport non-flat 
>> encoding */
>> @@ -5132,6 +5177,7 @@ static uint32_t immsv_evt_dec_toplevel(N
>>              break;
>>
>>          case IMMND_EVT_D2ND_IMPLSET_RSP:    /* Implter set reply 
>> from D with impl id */
>> +        case IMMND_EVT_D2ND_IMPLSET_RSP_2:
>>              IMMSV_FLTN_SPACE_ASSERT(p8, local_data, i_ub, 8);
>>              immndevt->info.implSet.client_hdl = ncs_decode_64bit(&p8);
>>              ncs_dec_skip_space(i_ub, 8);
>> @@ -5148,6 +5194,12 @@ static uint32_t immsv_evt_dec_toplevel(N
>>              IMMSV_FLTN_SPACE_ASSERT(p8, local_data, i_ub, 4);
>>              immndevt->info.implSet.scope = ncs_decode_32bit(&p8);
>>              ncs_dec_skip_space(i_ub, 4);
>> +
>> +            if(immndevt->type == IMMND_EVT_D2ND_IMPLSET_RSP_2) {
>> +                IMMSV_FLTN_SPACE_ASSERT(p8, local_data, i_ub, 4);
>> +                immndevt->info.implSet.oi_timeout = 
>> ncs_decode_32bit(&p8);
>> +                ncs_dec_skip_space(i_ub, 4);
>> +            }
> The IMMND_EVT_D2ND_IMPLSET_RSP_2.
> Not sure if it is a real problem but please chack.
>
> Additional cooments
>>              break;
>>
>>          case IMMND_EVT_D2ND_DISCARD_IMPL:    /* Discard implementer 
>> broadcast to NDs */
>> diff --git a/osaf/libs/common/immsv/include/immsv_evt.h 
>> b/osaf/libs/common/immsv/include/immsv_evt.h
>> --- a/osaf/libs/common/immsv/include/immsv_evt.h
>> +++ b/osaf/libs/common/immsv/include/immsv_evt.h
>> @@ -205,6 +205,8 @@ typedef enum immnd_evt_type {
>>
>>      IMMND_EVT_A2ND_ACCESSOR_GET = 94,    /* saImmOmAccessorGet_2 */
>>      IMMND_EVT_A2ND_CCB_VALIDATE = 95,    /* saImmOmCcbValidate */
>> +    IMMND_EVT_A2ND_OI_IMPL_SET_2 = 96,    /* saImmOiImplementerSet */
>> +    IMMND_EVT_D2ND_IMPLSET_RSP_2 = 97,    /* Implementer set reply 
>> from D with impl id */
>>
>>      IMMND_EVT_MAX
>>  } IMMND_EVT_TYPE;
>> @@ -251,6 +253,8 @@ typedef enum immd_evt_type {
>>
>>      IMMD_EVT_ND2D_2PBE_PRELOAD = 26, /* Redundant PBE preload stats 
>> to IMMD. */
>>
>> +    IMMD_EVT_ND2D_IMPLSET_REQ_2 = 27, /* OiImplementerSet */
>> +
>>      IMMD_EVT_MAX
>>  } IMMD_EVT_TYPE;
>>  /* Make sure the string array in immsv_evt.c matches the 
>> IMMD_EVT_TYPE enum. */
>> diff --git a/osaf/libs/common/immsv/include/immsv_evt_model.h 
>> b/osaf/libs/common/immsv/include/immsv_evt_model.h
>> --- a/osaf/libs/common/immsv/include/immsv_evt_model.h
>> +++ b/osaf/libs/common/immsv/include/immsv_evt_model.h
>> @@ -274,6 +274,7 @@ extern "C" {
>>          IMMSV_OCTET_STRING impl_name;    /*and className and objName */
>>          SaUint32T impl_id;
>>          SaUint32T scope;    /*Only for obj impl set/rel */
>> +        SaUint32T oi_timeout;    /* Timeout for OI callbacks */
>>      } IMMSV_OI_IMPLSET_REQ;
>>
>>      typedef struct immsv_oi_ccb_upcall_rsp {
>> diff --git a/osaf/services/saf/immsv/immd/immd_evt.c 
>> b/osaf/services/saf/immsv/immd/immd_evt.c
>> --- a/osaf/services/saf/immsv/immd/immd_evt.c
>> +++ b/osaf/services/saf/immsv/immd/immd_evt.c
>> @@ -133,6 +133,7 @@ void immd_process_evt(void)
>>          rc = immd_evt_proc_adminit_req(cb, &evt->info.immd, 
>> &evt->sinfo);
>>          break;
>>      case IMMD_EVT_ND2D_IMPLSET_REQ:
>> +    case IMMD_EVT_ND2D_IMPLSET_REQ_2:
>>          rc = immd_evt_proc_impl_set_req(cb, &evt->info.immd, 
>> &evt->sinfo);
>>          break;
>>      case IMMD_EVT_ND2D_DISCARD_IMPL:
>> @@ -1766,12 +1767,18 @@ static uint32_t immd_evt_proc_impl_set_r
>>
>>      memset(&fevs_evt, 0, sizeof(IMMSV_EVT));
>>      fevs_evt.type = IMMSV_EVT_TYPE_IMMND;
>> -    fevs_evt.info.immnd.type = IMMND_EVT_D2ND_IMPLSET_RSP;
>>      fevs_evt.info.immnd.info.implSet.impl_id = globalId;
>>      fevs_evt.info.immnd.info.implSet.impl_name.size = 
>> impl_req->impl_name.size;
>>      fevs_evt.info.immnd.info.implSet.impl_name.buf = 
>> impl_req->impl_name.buf;    /*Warning, borrowing pointer, dont 
>> deallocate */
>>      fevs_evt.info.immnd.info.implSet.client_hdl = 
>> impl_req->client_hdl;    /*redundant */
>>
>> +    if(evt->type == IMMD_EVT_ND2D_IMPLSET_REQ_2) {
>> +        fevs_evt.info.immnd.info.implSet.oi_timeout = 
>> impl_req->oi_timeout;
>> +        fevs_evt.info.immnd.type = IMMND_EVT_D2ND_IMPLSET_RSP_2;
>> +    } else {
>> +        fevs_evt.info.immnd.type = IMMND_EVT_D2ND_IMPLSET_RSP;
>> +    }
>> +
>>      proc_rc = ncs_enc_init_space(&uba);
>>      if (proc_rc != NCSCC_RC_SUCCESS) {
>>          LOG_WA("Failed init ubaid");
>> diff --git a/osaf/services/saf/immsv/immnd/ImmModel.cc 
>> b/osaf/services/saf/immsv/immnd/ImmModel.cc
>> --- a/osaf/services/saf/immsv/immnd/ImmModel.cc
>> +++ b/osaf/services/saf/immsv/immnd/ImmModel.cc
>> @@ -79,7 +79,8 @@ struct AttrInfo
>>  struct ImplementerInfo
>>  {
>>      ImplementerInfo():mId(0), mConn(0), mNodeId(0), mMds_dest(0LL),
>> -                      mAdminOpBusy(0), mDying(false), mApplier(false){}
>> +                      mAdminOpBusy(0), mDying(false), mApplier(false),
>> +                      mTimeout(DEFAULT_TIMEOUT_SEC) {}
>>      SaUint32T       mId;
>>      SaUint32T       mConn; //Current implementer, only valid on one 
>> node.
>>      //NULL otherwise.
>> @@ -89,6 +90,7 @@ struct ImplementerInfo
>>      unsigned int    mAdminOpBusy;
>>      bool            mDying;
>>      bool            mApplier; //This is an applier OI
>> +    SaUint32T        mTimeout; //OI callback timeout
>>  };
>>
>>  typedef std::vector<ImplementerInfo*> ImplementerVector;
>> @@ -1321,7 +1323,7 @@ immModel_nextResult(IMMND_CB *cb, void*      
>> IMMSV_OM_RSP_SEARCH_NEXT** rsp,
>>      SaUint32T* implConn, SaUint32T* implNodeId,
>>      struct ImmsvAttrNameList** rtAttrsToFetch,
>> -    MDS_DEST* implDest, SaBoolT retardSync)
>> +    MDS_DEST* implDest, SaBoolT retardSync, SaUint32T *oiTimeout)
>>  {
>>      AttributeList* rtAttrs = NULL;
>>      SaAisErrorT err = SA_AIS_OK;
>> @@ -1342,6 +1344,10 @@ immModel_nextResult(IMMND_CB *cb, void* 
>>              return SA_AIS_ERR_TRY_AGAIN;
>>          }
>>          err = ImmModel::instance(&cb->immModel)->nextSyncResult(rsp, 
>> *op);
>> +
>> +        if(oiTimeout && err == SA_AIS_OK) {
>> +            *oiTimeout = DEFAULT_TIMEOUT_SEC;
>> +        }
>>      } else {
>>          /* Reset search time */
>>          op->updateSearchTime();
>> @@ -1349,6 +1355,16 @@ immModel_nextResult(IMMND_CB *cb, void* 
>>          err = op->nextResult(rsp, implConn, implNodeId,
>>              (rtAttrsToFetch)?(&rtAttrs):NULL,
>>              (SaUint64T*) implDest);
>> +
>> +        if(oiTimeout && err == SA_AIS_OK) {
>> +            std::string objectName((*rsp)->objectName.buf, 
>> (*rsp)->objectName.size - 1);
>> +            ObjectMap::iterator omi = sObjectMap.find(objectName);
>> +            if(omi != sObjectMap.end()) {
>> +                *oiTimeout = (omi->second->mImplementer) ? 
>> omi->second->mImplementer->mTimeout : DEFAULT_TIMEOUT_SEC;
>> +            } else {
>> +                *oiTimeout = DEFAULT_TIMEOUT_SEC;
>> +            }
>> +        }
> There is an extra object lookup here for every object in the search 
> scope, even if it has not pure RTAs.
> I would prefer that the object lookup to find the implementer to get 
> the oi-timeout is only done for the
> case B and C at the immnd_evt.c leve lwhere there is a pure RTA and 
> there is an OI connected.
> The object name would be passed down from immnd_evt to ImmModel then 
> only when it was needed.
>
> One more ...
>>      }
>>
>>      if(err != SA_AIS_OK) { return err; }
>> @@ -1425,9 +1441,9 @@ immModel_setAdmReqContinuation(IMMND_CB
>>  void
>>  immModel_setSearchReqContinuation(IMMND_CB *cb, SaInvocationT invoc, 
>> -    SaUint32T reqConn)
>> -{
>> - ImmModel::instance(&cb->immModel)->setSearchReqContinuation(invoc, 
>> reqConn);
>> +    SaUint32T reqConn, SaUint32T oiTimeout)
>> +{
>> + ImmModel::instance(&cb->immModel)->setSearchReqContinuation(invoc, 
>> reqConn, oiTimeout);
>>  }
>>
>>  void
>> @@ -1442,14 +1458,16 @@ immModel_setSearchImplContinuation(IMMND
>>  SaAisErrorT
>>  immModel_implementerSet(IMMND_CB *cb, const IMMSV_OCTET_STRING* 
>> implName,
>>      SaUint32T implConn, SaUint32T implNodeId,
>> -    SaUint32T implId, MDS_DEST mds_dest)
>> +    SaUint32T implId, MDS_DEST mds_dest,
>> +    SaUint32T oiTimeout)
>>  {
>>      return 
>> ImmModel::instance(&cb->immModel)->implementerSet(implName, 
>>              implConn,
>>              implNodeId,
>>              implId,
>> -            (SaUint64T) mds_dest);
>> +            (SaUint64T) mds_dest,
>> +            oiTimeout);
>>  }
>>
>>  SaAisErrorT @@ -10293,11 +10311,11 @@ 
>> ImmModel::setAdmReqContinuation(SaInvoca
>>  }
>>
>>  void
>> -ImmModel::setSearchReqContinuation(SaInvocationT& saInv, SaUint32T 
>> reqConn)
>> -{
>> -    TRACE_ENTER();
>> -    TRACE_5("setSearchReqContinuation <%llu, %u>", saInv, reqConn);
>> -    sSearchReqContinuationMap[saInv] = ContinuationInfo2(reqConn, 
>> DEFAULT_TIMEOUT_SEC);
>> +ImmModel::setSearchReqContinuation(SaInvocationT& saInv, SaUint32T 
>> reqConn, SaUint32T oiTimeout)
>> +{
>> +    TRACE_ENTER();
>> +    TRACE_5("setSearchReqContinuation <%llu, %u, %usec>", saInv, 
>> reqConn, oiTimeout);
>> +    sSearchReqContinuationMap[saInv] = ContinuationInfo2(reqConn, 
>> oiTimeout);
>>      TRACE_LEAVE();
>>  }
>>
>> @@ -11052,12 +11070,23 @@ ImmModel::getOldCriticalCcbs(IdVector& c
> This function ImmModel::getOldCritical only deals with CCBs in state 
> critical.
> That state means that all regular OIs have replied and only the PBE 
> remains to
> reply or not. Since we are not waiting on an OIs we should not be 
> dealing with
> OI timeouts here. So this function should revert to the state it had 
> before this patch.
> It is rather in cleanTheBasement where regular OI processing is 
> monitored.
>
> That was what I could find.
>
> /AndersBj
>>      *pbeConnPtr = 0;
>>      *pbeIdPtr = 0;
>>      CcbVector::iterator i;
>> +    CcbImplementerMap::iterator cim;
>> +    uint32_t max_oi_timeout;
>>      time_t now = time(NULL);
>>      osafassert(now > ((time_t) 0));
>>      for(i=sCcbVector.begin(); i!=sCcbVector.end(); ++i) {
>> +        max_oi_timeout = 0;
>> +        for(cim = (*i)->mImplementers.begin(); cim != 
>> (*i)->mImplementers.end(); ++cim) {
>> +            if(cim->second->mImplementer->mTimeout > max_oi_timeout) {
>> +                max_oi_timeout = cim->second->mImplementer->mTimeout;
>> +            }
>> +        }
>> +        if(!max_oi_timeout)
>> +            max_oi_timeout = DEFAULT_TIMEOUT_SEC;
>> +
>>          if((*i)->mState == IMM_CCB_CRITICAL &&             
>> (((*i)->mWaitStartTime && - now - (*i)->mWaitStartTime >= 
>> DEFAULT_TIMEOUT_SEC)||/* Should be saImmOiTimeout*/
>> +             now - (*i)->mWaitStartTime >= max_oi_timeout)||/* 
>> Should be saImmOiTimeout*/
>>               (*i)->mPbeRestartId))
>>          {
>>              /* We have a critical ccb that has: timed out OR lived 
>> through a PBE restart.
>> @@ -11114,9 +11143,9 @@ ImmModel::getOldCriticalCcbs(IdVector& c
>>                  continue;
>>              }
>> -            if((ccb->mPbeRestartId == 0) && now - 
>> ccb->mWaitStartTime < (DEFAULT_TIMEOUT_SEC + addSecs)) {
>> +            if((ccb->mPbeRestartId == 0) && now - 
>> ccb->mWaitStartTime < (max_oi_timeout + addSecs)) {
>>                  LOG_NO("Ccb %u is old, but also large (%u) will wait 
>> secs:%ld", ccb->mId, mutations, - (DEFAULT_TIMEOUT_SEC + addSecs) - 
>> (now - ccb->mWaitStartTime));
>> +                    (max_oi_timeout + addSecs) - (now - 
>> ccb->mWaitStartTime));
>>                  continue;
>>              }
>>
>> @@ -11307,7 +11336,7 @@ ImmModel::cleanTheBasement(InvocVector& 
>> ci2!=sSearchReqContinuationMap.end();
>>          ++ci2) {
>>          //TODO the timeout should not be hardwired, but for now it is.
>> -        if(now - ci2->second.mCreateTime >= DEFAULT_TIMEOUT_SEC) {
>> +        if(now - ci2->second.mCreateTime >= ci2->second.mTimeout) {
>>              TRACE_5("Timeout on Search continuation %llu", ci2->first);
>>              searchReqs.push_back(ci2->first);
>>          } @@ -11338,14 +11367,29 @@ 
>> ImmModel::cleanTheBasement(InvocVector& (*i3)->mId);
>>              TRACE("state:%u waitsart:%u 
>> PberestartId:%u",(*i3)->mState,                  (unsigned int) 
>> (*i3)->mWaitStartTime, (*i3)->mPbeRestartId);
>> +
>> +            CcbImplementerMap::iterator cim;
>> +            uint32_t max_oi_timeout = 0;
>> +            for(cim = (*i3)->mImplementers.begin(); cim != 
>> (*i3)->mImplementers.end(); ++cim) {
>> +                if(cim->second->mImplementer->mTimeout > 
>> max_oi_timeout) {
>> +                    max_oi_timeout = 
>> cim->second->mImplementer->mTimeout;
>> +                }
>> +            }
>> +            if(!max_oi_timeout) {
>> +                max_oi_timeout = DEFAULT_TIMEOUT_SEC;
>> +            }
>> +
>> +            uint32_t oi_timeout = ((*i3)->mState == 
>> IMM_CCB_CRITICAL) ? DEFAULT_TIMEOUT_SEC : max_oi_timeout;
>>              if(((*i3)->mWaitStartTime &&
>> -                (now - (*i3)->mWaitStartTime >= 
>> DEFAULT_TIMEOUT_SEC)) ||
>> +                (now - (*i3)->mWaitStartTime >= oi_timeout)) ||
>>                  ((*i3)->mPbeRestartId)) {
>>                  //TODO Timeout value should be fetched from IMM 
>> service object.
>> -                if((*i3)->mPbeRestartId) { + if((*i3)->mPbeRestartId) {
>> +                    oi_timeout = 0;
>>                      TRACE_5("PBE restarted id:%u with ccb:%u in 
>> critical",
>>                          (*i3)->mPbeRestartId, (*i3)->mId);
>> -                } else {
>> +                } else if(now - (*i3)->mWaitStartTime >= 
>> max_oi_timeout) {
>> +                    oi_timeout = 0;
>>                      TRACE_5("CCB %u timeout while waiting on 
>> implementer reply",
>>                          (*i3)->mId);
>>                  }
>> @@ -11366,7 +11410,8 @@ ImmModel::cleanTheBasement(InvocVector& 
>>                              (DEFAULT_TIMEOUT_SEC + addSecs), 
>> (*i3)->mId);
>>                          ccbsStuck=1;
>>                      }
>> -                } else {
>> +                } else if(!oi_timeout) {
>> +                    // oi_timeout set to 0 means that the ccb should 
>> be added to the vector
>>                      ccbs.push_back((*i3)->mId); /*Non critical ccb 
>> to abort.*/
>>                  }
>>              }
>> @@ -11476,7 +11521,8 @@ ImmModel::implementerSet(const IMMSV_OCT
>>      SaUint32T conn,
>>      SaUint32T nodeId,
>>      SaUint32T implementerId,
>> -    SaUint64T mds_dest)
>> +    SaUint64T mds_dest,
>> +    SaUint32T oiTimeout)
>>  {
>>      SaAisErrorT err = SA_AIS_OK;
>>      CcbVector::iterator i;
>> @@ -11486,7 +11532,11 @@ ImmModel::implementerSet(const IMMSV_OCT
>>          TRACE_LEAVE();
>>          return SA_AIS_ERR_TRY_AGAIN;
>>      }    -    +
>> +    if(!oiTimeout) {
>> +        oiTimeout = DEFAULT_TIMEOUT_SEC;
>> +    }
>> +
>>      //Check first if implementer name already exists.
>>      //If so check if occupied, if not re-use.
>>      size_t sz = strnlen((char *) implementerName->buf,
>> @@ -11587,6 +11637,7 @@ ImmModel::implementerSet(const IMMSV_OCT
>>      info->mAdminOpBusy = 0;
>>      info->mMds_dest = mds_dest;
>>      info->mDying = false;
>> +    info->mTimeout = oiTimeout;
>>           if(isApplier) {
>>          info->mApplier = true;
>> diff --git a/osaf/services/saf/immsv/immnd/ImmModel.hh 
>> b/osaf/services/saf/immsv/immnd/ImmModel.hh
>> --- a/osaf/services/saf/immsv/immnd/ImmModel.hh
>> +++ b/osaf/services/saf/immsv/immnd/ImmModel.hh
>> @@ -379,7 +379,8 @@ public:
>>                                         SaUint32T con,
>>                                         SaUint32T nodeId,
>>                                         SaUint32T ownerId,
>> -                                       SaUint64T mds_dest);
>> +                                       SaUint64T mds_dest,
>> +                                       SaUint32T oiTimeout);
>>           SaAisErrorT         classImplementerSet(
>>                                              const struct 
>> ImmsvOiImplSetReq* req,
>> @@ -544,7 +545,8 @@ public:
>>                                                   SaUint32T* 
>> reqConn); //in-out?
>>      void              setSearchReqContinuation(
>> SaInvocationT& inv,
>> -                                               SaUint32T conn);
>> +                                               SaUint32T conn,
>> +                                               SaUint32T oiTimeout);
>>
>>      void              setAdmReqContinuation(
>> SaInvocationT& inv,
>> diff --git a/osaf/services/saf/immsv/immnd/immnd_evt.c 
>> b/osaf/services/saf/immsv/immnd/immnd_evt.c
>> --- a/osaf/services/saf/immsv/immnd/immnd_evt.c
>> +++ b/osaf/services/saf/immsv/immnd/immnd_evt.c
>> @@ -447,7 +447,9 @@ uint32_t immnd_evt_destroy(IMMSV_EVT *ev
>>          evt->info.immnd.info.classDescr.attrDefinitions = NULL;
>>
>>      } else if ((evt->info.immnd.type == IMMND_EVT_A2ND_OI_IMPL_SET) ||
>> +           (evt->info.immnd.type == IMMND_EVT_A2ND_OI_IMPL_SET_2) ||
>>             (evt->info.immnd.type == IMMND_EVT_D2ND_IMPLSET_RSP) ||
>> +           (evt->info.immnd.type == IMMND_EVT_D2ND_IMPLSET_RSP_2) ||
>>             (evt->info.immnd.type == IMMND_EVT_A2ND_OI_CL_IMPL_SET) ||
>>             (evt->info.immnd.type == IMMND_EVT_A2ND_OI_CL_IMPL_REL) ||
>>             (evt->info.immnd.type == IMMND_EVT_A2ND_OI_OBJ_IMPL_SET) ||
>> @@ -563,6 +565,7 @@ void immnd_process_evt(void)
>>          break;
>>
>>      case IMMND_EVT_A2ND_OI_IMPL_SET:
>> +    case IMMND_EVT_A2ND_OI_IMPL_SET_2:
>>          rc = immnd_evt_proc_impl_set(cb, &evt->info.immnd, 
>> &evt->sinfo);
>>          break;
>>
>> @@ -1178,7 +1181,7 @@ static uint32_t immnd_evt_proc_oi_att_pu
>>          if (err == SA_AIS_OK) {
>>              TRACE_2("oi_att_pull_rpl searchInit returned OK, calling 
>> searchNext");
>>              IMMSV_OM_RSP_SEARCH_NEXT *rsp = 0;
>> -            err = immModel_nextResult(cb, searchOp, &rsp, NULL, 
>> NULL, NULL, NULL, SA_FALSE);
>> +            err = immModel_nextResult(cb, searchOp, &rsp, NULL, 
>> NULL, NULL, NULL, SA_FALSE, NULL);
>>              if (err == SA_AIS_OK) {
>>                  rspo->runtimeAttrs.attrValuesList = 
>> rsp->attrValuesList;
>>                  /*STEALING*/ rsp->attrValuesList = NULL;
>> @@ -1390,6 +1393,7 @@ static uint32_t immnd_evt_proc_search_ne
>>      IMMSV_OM_RSP_SEARCH_BUNDLE_NEXT bundleSearch = {0, NULL};
>>      int ix;
>>      SaBoolT isAccessor = SA_FALSE;
>> +    SaUint32T oiTimeout;
>>
>>      TRACE_ENTER();
>>
>> @@ -1434,7 +1438,7 @@ static uint32_t immnd_evt_proc_search_ne
>>      }
>>
>>      error = immModel_nextResult(cb, sn->searchOp, &rsp, &implConn, 
>> &implNodeId, &rtAttrsToFetch,
>> -        &implDest, retardSync);
>> +        &implDest, retardSync, &oiTimeout);
>>      if (error != SA_AIS_OK) {
>>          goto agent_rsp;
>>      }
>> @@ -1520,7 +1524,7 @@ static uint32_t immnd_evt_proc_search_ne
>>
>>          TRACE_2("SETTING SEARCH REQ CONTINUATION FOR %u|%x->%u", 
>> sn->searchId, implNodeId, clientId);
>>
>> -        immModel_setSearchReqContinuation(cb, invoc, clientId);
>> +        immModel_setSearchReqContinuation(cb, invoc, clientId, 
>> oiTimeout);
>>
>>          cl_node->tmpSinfo = *sinfo;    //TODO should be part of 
>> continuation?
>>
>> @@ -1547,7 +1551,7 @@ static uint32_t immnd_evt_proc_search_ne
>>                  break;
>>
>>              err = immModel_nextResult(cb, sn->searchOp, &rsp1, 
>> &implConn, &implNodeId, &rtAttrs,
>> -                    &implDest, retardSync);
>> +                    &implDest, retardSync, NULL);
>>              if(err != SA_AIS_OK) {
>>                  osafassert(err == SA_AIS_ERR_NOT_EXIST);
>>                  break;
>> @@ -2316,6 +2320,14 @@ static uint32_t immnd_evt_proc_impl_set(
>>          goto agent_rsp;
>>      }
>>
>> +    if(evt->type == IMMND_EVT_A2ND_OI_IMPL_SET_2 && 
>> !immModel_protocol45Allowed(cb)) {
>> +        LOG_WA("Failed to set OI implementer (%u) with OI callback 
>> timeout "
>> +                "(OpenSAF 4.5 features are disabled)",
>> +                evt->info.implSet.impl_id);
>> +        send_evt.info.imma.info.implSetRsp.error = 
>> SA_AIS_ERR_NO_RESOURCES;
>> +        goto agent_rsp;
>> +    }
>> +
>>      if (cb->fevs_replies_pending >= IMMSV_DEFAULT_FEVS_MAX_PENDING) {
>>          TRACE_2("ERR_TRY_AGAIN: Too many pending incoming fevs 
>> messages (> %u) rejecting impl_set request",
>>              IMMSV_DEFAULT_FEVS_MAX_PENDING);
>> @@ -2374,12 +2386,18 @@ static uint32_t immnd_evt_proc_impl_set(
>>      }
>>
>>      send_evt.type = IMMSV_EVT_TYPE_IMMD;
>> -    send_evt.info.immd.type = IMMD_EVT_ND2D_IMPLSET_REQ;
>>      send_evt.info.immd.info.impl_set.r.client_hdl = client_hdl;
>>      send_evt.info.immd.info.impl_set.r.impl_name.size = 
>> evt->info.implSet.impl_name.size;
>>      send_evt.info.immd.info.impl_set.r.impl_name.buf = 
>> evt->info.implSet.impl_name.buf;    /*Warning re-using buffer, no 
>> copy. */
>>      send_evt.info.immd.info.impl_set.reply_dest = cb->immnd_mdest_id;
>>
>> +    if(evt->type == IMMND_EVT_A2ND_OI_IMPL_SET_2) {
>> +        send_evt.info.immd.info.impl_set.r.oi_timeout = 
>> evt->info.implSet.oi_timeout;
>> +        send_evt.info.immd.type = IMMD_EVT_ND2D_IMPLSET_REQ_2;
>> +    } else {
>> +        send_evt.info.immd.type = IMMD_EVT_ND2D_IMPLSET_REQ;
>> +    }
>> +
>>      /* send the request to the IMMD, reply comes back over fevs. */
>>
>>      rc = immnd_mds_msg_send(cb, NCSMDS_SVC_ID_IMMD, 
>> cb->immd_mdest_id, &send_evt);
>> @@ -3124,6 +3142,11 @@ static SaAisErrorT immnd_fevs_local_chec
>>          error = SA_AIS_ERR_LIBRARY;
>>          break;
>>
>> +    case IMMND_EVT_D2ND_IMPLSET_RSP_2:
>> +        LOG_WA("ERR_LIBRARY: IMMND_EVT_D2ND_IMPLSET_RSP_2 can not 
>> arrive from client lib");
>> +        error = SA_AIS_ERR_LIBRARY;
>> +        break;
>> +
>>      case IMMND_EVT_D2ND_CCBINIT:
>>          LOG_WA("ERR_LIBRARY: IMMND_EVT_D2ND_CCBINIT can not arrive 
>> from client lib");
>>          error = SA_AIS_ERR_LIBRARY;
>> @@ -7437,7 +7460,8 @@ static uint32_t immnd_restricted_ok(IMMN
>>              id == IMMND_EVT_A2ND_OI_IMPL_CLR ||
>>              id == IMMND_EVT_D2ND_SYNC_FEVS_BASE ||
>>              id == IMMND_EVT_A2ND_OI_OBJ_MODIFY ||
>> -            id == IMMND_EVT_D2ND_IMPLSET_RSP) {
>> +            id == IMMND_EVT_D2ND_IMPLSET_RSP ||
>> +            id == IMMND_EVT_D2ND_IMPLSET_RSP_2) {
>>              return 1;
>>          }
>>      }
>> @@ -7593,6 +7617,7 @@ immnd_evt_proc_fevs_dispatch(IMMND_CB *c
>>          break;
>>
>>      case IMMND_EVT_D2ND_IMPLSET_RSP:
>> +    case IMMND_EVT_D2ND_IMPLSET_RSP_2:
>>          immnd_evt_proc_impl_set_rsp(cb, &frwrd_evt.info.immnd, 
>> originatedAtThisNd, clnt_hdl, reply_dest);
>>          break;
>>
>> @@ -8997,8 +9022,15 @@ static void immnd_evt_proc_impl_set_rsp(
>>      nodeId = m_IMMSV_UNPACK_HANDLE_LOW(clnt_hdl);
>>      TRACE_2("originated here?:%u nodeId:%x conn: %u", 
>> originatedAtThisNd, nodeId, conn);
>>
>> +    if(evt->type == IMMND_EVT_D2ND_IMPLSET_RSP) {
>> +        /* In immModel_implementerSet, 0 will be replaced with 
>> DEFAULT_TIMEOUT_SEC
>> +         * as the default value */
>> +        evt->info.implSet.oi_timeout = 0;
>> +    }
>> +
>>      err = immModel_implementerSet(cb, &(evt->info.implSet.impl_name),
>> -                      (originatedAtThisNd) ? conn : 0, nodeId, 
>> evt->info.implSet.impl_id, reply_dest);
>> +            (originatedAtThisNd) ? conn : 0, nodeId, 
>> evt->info.implSet.impl_id,
>> +            reply_dest, evt->info.implSet.oi_timeout);
>>
>>      if (originatedAtThisNd) {    /*Send reply to client from this 
>> ND. */
>>          immnd_client_node_get(cb, clnt_hdl, &cl_node);
>> diff --git a/osaf/services/saf/immsv/immnd/immnd_init.h 
>> b/osaf/services/saf/immsv/immnd/immnd_init.h
>> --- a/osaf/services/saf/immsv/immnd/immnd_init.h
>> +++ b/osaf/services/saf/immsv/immnd/immnd_init.h
>> @@ -199,7 +199,8 @@ extern "C" {
>>          immModel_nextResult(IMMND_CB *cb, void *searchOp,
>>                  IMMSV_OM_RSP_SEARCH_NEXT **rsp,
>>                  SaUint32T *implConn, SaUint32T *implNodeId,
>> -                struct ImmsvAttrNameList **rtAttrsToFetch, MDS_DEST 
>> *implDest, SaBoolT retardSync);
>> +                struct ImmsvAttrNameList **rtAttrsToFetch,
>> +                MDS_DEST *implDest, SaBoolT retardSync, SaUint32T 
>> *oiTimeout);
>>
>>      void immModel_deleteSearchOp(void *searchOp);
>>
>> @@ -211,14 +212,15 @@ extern "C" {
>>
>>      void immModel_setAdmReqContinuation(IMMND_CB *cb, SaInvocationT 
>> invoc, SaUint32T reqCon);
>>
>> -    void immModel_setSearchReqContinuation(IMMND_CB *cb, 
>> SaInvocationT invoc, SaUint32T reqCon);
>> +    void immModel_setSearchReqContinuation(IMMND_CB *cb, 
>> SaInvocationT invoc, SaUint32T reqCon, SaUint32T oiTimeout);
>>
>>      void immModel_setSearchImplContinuation(IMMND_CB *cb, SaUint32T 
>> searchId,
>>                          SaUint32T requestnodeId, MDS_DEST reply_dest);
>>
>>      SaAisErrorT
>>          immModel_implementerSet(IMMND_CB *cb, const 
>> IMMSV_OCTET_STRING *implName,
>> -                    SaUint32T implConn, SaUint32T implNodeId, 
>> SaUint32T implId, MDS_DEST mds_dest);
>> +                    SaUint32T implConn, SaUint32T implNodeId, 
>> SaUint32T implId,
>> +                    MDS_DEST mds_dest, SaUint32T oiTimeout);
>>
>>      SaAisErrorT
>>          immModel_implementerClear(IMMND_CB *cb, const struct 
>> ImmsvOiImplSetReq *req,
>>
>> ------------------------------------------------------------------------------
>>  
>>
>> Learn Graph Databases - Download FREE O'Reilly Book
>> "Graph Databases" is the definitive new guide to graph databases and 
>> their applications. Written by three acclaimed leaders in the field, 
>> this first edition is now available. Download your free book today!
>> http://p.sf.net/sfu/NeoTech
>> _______________________________________________
>> Opensaf-devel mailing list
>> Opensaf-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/opensaf-devel
>


------------------------------------------------------------------------------
HPCC Systems Open Source Big Data Platform from LexisNexis Risk Solutions
Find What Matters Most in Your Big Data with HPCC Systems
Open Source. Fast. Scalable. Simple. Ideal for Dirty Data.
Leverages Graph Analysis for Fast Processing & Easy Data Exploration
http://p.sf.net/sfu/hpccsystems
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to