OK. No more comment.
-----Original Message-----
From: Minh Hon Chau <minh.c...@dektech.com.au>
Sent: Tuesday, May 18, 2021 9:47 AM
To: Thang Duc Nguyen <thang.d.ngu...@dektech.com.au>; Thien Minh Huynh
<thien.m.hu...@dektech.com.au>; Surbhi Tripathi <surbhi.tripa...@dektech.com.au>
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/2] immnd: Make IMMSV_FEVS_MAX_PENDING environment
variable [#3260]
Hi Thang,
Please see reply.
Thanks
Minh
On 18/5/21 12:42 pm, Thang Duc Nguyen wrote:
> Hi Minh,
> One more.
>
> B.R/Thang
>
> -----Original Message-----
> From: Minh Hon Chau <minh.c...@dektech.com.au>
> Sent: Tuesday, May 18, 2021 9:36 AM
> To: Thang Duc Nguyen <thang.d.ngu...@dektech.com.au>; Thien Minh Huynh
> <thien.m.hu...@dektech.com.au>; Surbhi Tripathi
> <surbhi.tripa...@dektech.com.au>
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [PATCH 1/2] immnd: Make IMMSV_FEVS_MAX_PENDING
> environment variable [#3260]
>
> Hi Thang,
>
> Please see inline comments
>
> Thanks
>
> Minh
>
> On 18/5/21 12:28 pm, Thang Duc Nguyen wrote:
>> Hi Minh,
>> 2 comments inline.
>>
>> B.R/Thang
>>
>> -----Original Message-----
>> From: Minh Hon Chau <minh.c...@dektech.com.au>
>> Sent: Tuesday, May 18, 2021 6:51 AM
>> To: Thien Minh Huynh <thien.m.hu...@dektech.com.au>; Surbhi Tripathi
>> <surbhi.tripa...@dektech.com.au>; Thang Duc Nguyen
>> <thang.d.ngu...@dektech.com.au>
>> Cc: opensaf-devel@lists.sourceforge.net; Minh Hon Chau
>> <minh.c...@dektech.com.au>
>> Subject: [PATCH 1/2] immnd: Make IMMSV_FEVS_MAX_PENDING environment
>> variable [#3260]
>>
>> Immnd allows IMMSV_FEVS_MAX_PENDING sourced from enviroment variable,
>> or uses default value (16) otherwise
>> [Thang]: I think it's better if we can define MAX VALUE for this env var.
> [M: It's already checked with UINT8_MAX.
> [Thang]: IMM can work/any degradation if it is to high value, e.g,
> (UINT8_MAX-1).
[M] Not sure what is comment/suggestion here? The old max value of fevs is 255
according to legacy code. UINT8_MAX is 255
>
>> For Example 64 or 128 ....
>> ---
>> src/imm/common/immsv_api.h | 4 ----
>> src/imm/immloadd/imm_loader.cc | 2 +-
>> src/imm/immnd/ImmModel.cc | 2 +-
>> src/imm/immnd/immnd.conf | 4 ++++
>> src/imm/immnd/immnd_cb.h | 1 +
>> src/imm/immnd/immnd_evt.c | 41 ++++++++++++++++------------------
>> src/imm/immnd/immnd_main.c | 15 +++++++++++++
>> 7 files changed, 41 insertions(+), 28 deletions(-)
>>
>> diff --git a/src/imm/common/immsv_api.h b/src/imm/common/immsv_api.h
>> index c37a624ed..f809c8168 100644
>> --- a/src/imm/common/immsv_api.h
>> +++ b/src/imm/common/immsv_api.h
>> @@ -77,10 +77,6 @@ typedef enum {
>> ACCESS_CONTROL_ENFORCING = 2
>> } OsafImmAccessControlModeT;
>>
>> -/*Max # of outstanding fevs messages towards director.*/ -/*Note
>> max-max is 255. cb->fevs_replies_pending is an uint8_t*/ -#define
>> IMMSV_DEFAULT_FEVS_MAX_PENDING 16
>> -
>> #define IMMSV_MAX_OBJECTS 10000
>> #define IMMSV_MAX_ATTRIBUTES 128
>> #define IMMSV_MAX_ADMO_NAME_LENGTH 256 diff --git
>> a/src/imm/immloadd/imm_loader.cc b/src/imm/immloadd/imm_loader.cc
>> index 516fb24ec..fadb0da51 100644
>> --- a/src/imm/immloadd/imm_loader.cc
>> +++ b/src/imm/immloadd/imm_loader.cc
>> @@ -2318,7 +2318,7 @@ int syncObjectsOfClass(std::string className,
>> SaImmHandleT &immHandle,
>> do {
>> if (retries) {
>> /* TRY_AGAIN while sync is in progress means *this* IMMND most
>> likely
>> - has reached IMMSV_DEFAULT_FEVS_MAX_PENDING. This means that
>> *this*
>> + has reached max pending fevs messages. This means that
>> + *this*
>> IMMND has sent its quota of fevs messages to IMMD without
>> having
>> received them back via broadcast from IMMD.
>>
>> diff --git a/src/imm/immnd/ImmModel.cc b/src/imm/immnd/ImmModel.cc
>> index 2d750040e..8631dc21f 100644
>> --- a/src/imm/immnd/ImmModel.cc
>> +++ b/src/imm/immnd/ImmModel.cc
>> @@ -1614,7 +1614,7 @@ SaAisErrorT immModel_nextResult(IMMND_CB* cb, void*
>> searchOp,
>> TRACE_2(
>> "ERR_TRY_AGAIN: Too many pending incoming fevs "
>> "messages (> %u) rejecting sync iteration next request",
>> - IMMSV_DEFAULT_FEVS_MAX_PENDING);
>> + cb->mFevsMaxPending);
>> return SA_AIS_ERR_TRY_AGAIN;
>> }
>> err = ImmModel::instance(&cb->immModel)->nextSyncResult(rsp,
>> *op); diff --git a/src/imm/immnd/immnd.conf
>> b/src/imm/immnd/immnd.conf index e62367654..f9a809e16 100644
>> --- a/src/imm/immnd/immnd.conf
>> +++ b/src/imm/immnd/immnd.conf
>> @@ -95,3 +95,7 @@ export IMMSV_ENV_HEALTHCHECK_KEY="Default"
>> # objects, objects_blob_multi,
>> # objects_int_multi,
>> objects_real_multi,
>> # objects_text_multi, pbe_rep_version";
>> +
>> +# Max outstanding fevs messages towards director without having
>> +received # them back via director's broadcast message. Default value is 16.
>> +# export IMMSV_FEVS_MAX_PENDING=64
>> diff --git a/src/imm/immnd/immnd_cb.h b/src/imm/immnd/immnd_cb.h
>> index
>> bb3bb8493..bc2d75b05 100644
>> --- a/src/imm/immnd/immnd_cb.h
>> +++ b/src/imm/immnd/immnd_cb.h
>> @@ -209,6 +209,7 @@ typedef struct immnd_cb_tag {
>> NCS_PATRICIA_TREE immnd_clm_list; /* IMMND_IMM_CLIENT_NODE - node */
>> tmr_t splitbrain_tmr;
>> bool splitbrain_tmr_run;
>> + uint8_t mFevsMaxPending; /* Max pending fevs messages towards
>> + director */
>> } IMMND_CB;
>>
>> /* CB prototypes */
>> diff --git a/src/imm/immnd/immnd_evt.c b/src/imm/immnd/immnd_evt.c
>> index 0e801a942..64c5223c4 100644
>> --- a/src/imm/immnd/immnd_evt.c
>> +++ b/src/imm/immnd/immnd_evt.c
>> @@ -1787,7 +1787,7 @@ static uint32_t immnd_evt_proc_search_next(IMMND_CB
>> *cb, IMMND_EVT *evt,
>> IMMSV_OM_RSP_SEARCH_NEXT **rspList = NULL;
>> MDS_DEST implDest = 0LL;
>> bool retardSync =
>> - ((cb->fevs_replies_pending >= IMMSV_DEFAULT_FEVS_MAX_PENDING) &&
>> + ((cb->fevs_replies_pending >= cb->mFevsMaxPending) &&
>> cb->mIsCoord && (cb->syncPid > 0));
>> SaUint32T resultSize = 0;
>> IMMSV_OM_RSP_SEARCH_BUNDLE_NEXT bundleSearch = {0, NULL}; @@ -2767,10
>> +2767,10 @@ static uint32_t immnd_evt_proc_admowner_init(IMMND_CB *cb,
>> IMMND_EVT *evt,
>> goto agent_rsp;
>> }
>>
>> - if (cb->fevs_replies_pending >= IMMSV_DEFAULT_FEVS_MAX_PENDING) {
>> + if (cb->fevs_replies_pending >= cb->mFevsMaxPending) {
>> TRACE_2(
>> "ERR_TRY_AGAIN: Too many pending incoming fevs messages (>
>> %u) rejecting admo_init request",
>> - IMMSV_DEFAULT_FEVS_MAX_PENDING);
>> + cb->mFevsMaxPending);
>> send_evt.info.imma.info.admInitRsp.error = SA_AIS_ERR_TRY_AGAIN;
>> goto agent_rsp;
>> }
>> @@ -2895,10 +2895,10 @@ static uint32_t immnd_evt_proc_impl_set(IMMND_CB
>> *cb, IMMND_EVT *evt,
>> goto agent_rsp;
>> }
>>
>> - if (cb->fevs_replies_pending >= IMMSV_DEFAULT_FEVS_MAX_PENDING) {
>> + if (cb->fevs_replies_pending >= cb->mFevsMaxPending) {
>> TRACE_2(
>> "ERR_TRY_AGAIN: Too many pending incoming fevs messages (>
>> %u) rejecting impl_set request",
>> - IMMSV_DEFAULT_FEVS_MAX_PENDING);
>> + cb->mFevsMaxPending);
>> send_evt.info.imma.info.implSetRsp.error = SA_AIS_ERR_TRY_AGAIN;
>> goto agent_rsp;
>> }
>> @@ -3061,10 +3061,10 @@ static uint32_t immnd_evt_proc_ccb_init(IMMND_CB
>> *cb, IMMND_EVT *evt,
>> goto agent_rsp;
>> }
>>
>> - if (cb->fevs_replies_pending >= IMMSV_DEFAULT_FEVS_MAX_PENDING) {
>> + if (cb->fevs_replies_pending >= cb->mFevsMaxPending) {
>> TRACE_2(
>> "ERR_TRY_AGAIN: Too many pending incoming fevs messages (>
>> %u) rejecting ccb_init request",
>> - IMMSV_DEFAULT_FEVS_MAX_PENDING);
>> + cb->mFevsMaxPending);
>> send_evt.info.imma.info.ccbInitRsp.error = SA_AIS_ERR_TRY_AGAIN;
>> goto agent_rsp;
>> }
>> @@ -3220,11 +3220,10 @@ static uint32_t immnd_evt_proc_rt_update(IMMND_CB
>> *cb, IMMND_EVT *evt,
>> writbale.
>> */
>>
>> - if (cb->fevs_replies_pending >=
>> - IMMSV_DEFAULT_FEVS_MAX_PENDING) {
>> + if (cb->fevs_replies_pending >= cb->mFevsMaxPending) {
>> TRACE_2(
>> "ERR_TRY_AGAIN: Too many pending incoming fevs
>> messages (> %u) rejecting rt_update request",
>> - IMMSV_DEFAULT_FEVS_MAX_PENDING);
>> + cb->mFevsMaxPending);
>> err = SA_AIS_ERR_TRY_AGAIN;
>> goto agent_rsp;
>> }
>> @@ -3497,7 +3496,7 @@ static uint32_t immnd_evt_proc_fevs_forward(IMMND_CB
>> *cb, IMMND_EVT *evt,
>> /* If overflow .....OR IMMD is down.....OR sync is on-going AND
>> out-queue is not empty => go via out-queue. The sync should be
>> throttled by immnd_evt_proc_search_next. */
>> - if ((cb->fevs_replies_pending >= IMMSV_DEFAULT_FEVS_MAX_PENDING) ||
>> + if ((cb->fevs_replies_pending >= cb->mFevsMaxPending) ||
>> !immnd_is_immd_up(cb) ||
>> ((cb->mState == IMM_SERVER_SYNC_SERVER) && cb->fevs_out_count &&
>> asyncReq && newMsg)) {
>> @@ -3518,12 +3517,12 @@ static uint32_t immnd_evt_proc_fevs_forward(IMMND_CB
>> *cb, IMMND_EVT *evt,
>> TRACE_2(
>> "Too many pending incoming FEVS messages (>
>> %u) "
>> "enqueueing async message. Backlog:%u",
>> - IMMSV_DEFAULT_FEVS_MAX_PENDING, backlog);
>> + cb->mFevsMaxPending, backlog);
>> } else {
>> LOG_IN(
>> "Too many pending incoming FEVS messages (>
>> %u) "
>> "enqueueing async message. Backlog:%u",
>> - IMMSV_DEFAULT_FEVS_MAX_PENDING, backlog);
>> + cb->mFevsMaxPending, backlog);
>> }
>>
>> return NCSCC_RC_SUCCESS;
>> @@ -3533,7 +3532,7 @@ static uint32_t immnd_evt_proc_fevs_forward(IMMND_CB
>> *cb, IMMND_EVT *evt,
>> case. */
>> TRACE_2(
>> "ERR_TRY_AGAIN: Too many pending FEVS message
>> replies (> %u) rejecting request",
>> - IMMSV_DEFAULT_FEVS_MAX_PENDING);
>> + cb->mFevsMaxPending);
>> error = SA_AIS_ERR_TRY_AGAIN;
>> goto agent_rsp;
>> }
>> @@ -3824,8 +3823,7 @@ static SaAisErrorT immnd_fevs_local_checks(IMMND_CB
>> *cb, IMMSV_FEVS *fevsReq,
>> break;
>> }
>> if (immModel_pbeNotWritable(cb) ||
>> - (cb->fevs_replies_pending >=
>> - IMMSV_DEFAULT_FEVS_MAX_PENDING) ||
>> + (cb->fevs_replies_pending >= cb->mFevsMaxPending) ||
>> !immnd_is_immd_up(cb)) {
>> /* NO_RESOURCES is here imm internal proxy for
>> TRY_AGAIN. The library code for saImmOmCcbApply will
>> @@ -3836,12 +3834,11 @@ static SaAisErrorT immnd_fevs_local_checks(IMMND_CB
>> *cb, IMMSV_FEVS *fevsReq,
>> TRY_AGAIN towards that particular library code.
>> */
>> error = SA_AIS_ERR_NO_RESOURCES;
>> - if (cb->fevs_replies_pending >=
>> - IMMSV_DEFAULT_FEVS_MAX_PENDING) {
>> + if (cb->fevs_replies_pending >= cb->mFevsMaxPending) {
>> TRACE_2(
>> "ERR_TRY_AGAIN: Too many pending FEVS
>> message replies (> %u) rejecting request"
>> "for CcbApply",
>> - IMMSV_DEFAULT_FEVS_MAX_PENDING);
>> + cb->mFevsMaxPending);
>> }
>> }
>> break;
>> @@ -10731,15 +10728,15 @@ void dequeue_outgoing(IMMND_CB *cb)
>> IMMND_EVT dummy_evt;
>>
>> unsigned int space =
>> - (cb->fevs_replies_pending < IMMSV_DEFAULT_FEVS_MAX_PENDING)
>> - ? (IMMSV_DEFAULT_FEVS_MAX_PENDING - cb->fevs_replies_pending)
>> + (cb->fevs_replies_pending < cb->mFevsMaxPending)
>> + ? (cb->mFevsMaxPending - cb->fevs_replies_pending)
>> : 0;
>>
>> TRACE("Pending replies:%u space:%u out list?:%p",
>> cb->fevs_replies_pending, space, cb->fevs_out_list);
>>
>> while (cb->fevs_out_list && space &&
>> - (cb->fevs_replies_pending < IMMSV_DEFAULT_FEVS_MAX_PENDING) &&
>> + (cb->fevs_replies_pending < cb->mFevsMaxPending) &&
>> immnd_is_immd_up(cb)) {
>> memset(&dummy_evt, '\0', sizeof(IMMND_EVT));
>> unsigned int backlog = immnd_dequeue_outgoing_fevs_msg( diff
>> --git a/src/imm/immnd/immnd_main.c b/src/imm/immnd/immnd_main.c index
>> 410134c97..a26005528 100644
>> --- a/src/imm/immnd/immnd_main.c
>> +++ b/src/imm/immnd/immnd_main.c
>> @@ -337,6 +337,21 @@ static uint32_t immnd_initialize(char *progname)
>> "Persistent Back-End capability configured, Pbe file:%s
>> (suffix may get added)",
>> immnd_cb->mPbeFile);
>> }
>> + immnd_cb->mFevsMaxPending = 16;
>> + if ((envVar = getenv("IMMSV_FEVS_MAX_PENDING"))) {
>> + int maxFevsPending = atoi(envVar);
>> + if (maxFevsPending > UINT8_MAX) {
>> + LOG_WA("IMMSV_FEVS_MAX_PENDING set too large(%u)",
>> + maxFevsPending);
>> + maxFevsPending = UINT8_MAX;
>> + } else if (maxFevsPending <= 0) {
>> + LOG_WA("Invalid IMMSV_FEVS_MAX_PENDING environment"
>> + " variable");
>> [Thang]: It's better to inform default value(16) will be used.
> [M]: The below LOG_NO has informed the value sourced from the env
> variable, whether it is valid or invalid, the value being used is
> printed out in LOG_NO
>> + maxFevsPending = 16;
>> + }
>> + immnd_cb->mFevsMaxPending = maxFevsPending;
>> + LOG_NO("Use IMMSV_FEVS_MAX_PENDING (%u)", maxFevsPending);
>> + }
>>
>> FILE *fp;
>> char node_type[20];
>> --
>> 2.20.1
>>
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
