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).
> 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
