Hi Zoran, comments inline.
On 2016/08/09 07:55 PM, Zoran Milinkovic wrote: > Hi Neelakanta, > > When IMM is running in 2PBE mode, the slave PBE is running on the standby > node (where the standby IMMD is). The #1925 is about pre-load. When 2PBE is enabled and both controllers are started at same time, the IMMD will determine from which node the loading should start based on the pre-load information sent by controller IMMNDs. If both the pre-load information is same, then standby IMMND is chosen as coordinator. Otherwise base on the pre-load information that IMMND is chosen as coordinator. Only IMMND at controllers can send pre-load information. From the #79 patch, the standby role is given only when the amfd comes up. Because, of this delay the IMMND or active IMMD does not register as standby, pre-load information is not sent to active IMMD. If the latest information is present at standby controller, then the cluster will not load with latest imm.db. > In the document update you wrote "There is a chance that chosen standby immnd > can be from different node than the actual node which got standby role.". In case of spares, there are more than two controllers, and there is a chance, the all spares can sent pre-load information. The latest chosen IMMND for loading may not be given standby role. > A node receives "canBeCoord" with value 2 only if the node is a controller. > So, the part with reading "node_type" file can be skipped to determine if the > node is a controller or not, and rely on the value from "canBeCoord". > In the document update, you wrote that it is not recommended to run 2PBE with > spare nodes. It means that it can run with spare nodes. In this case every > node will be a controller. "controller" string will be written in node_type > file on every node. when spares are configured, 2PBE is not recommended. > > If IMMND on a controller was introduced earlier (in NCSMDS_RED_UP event) or > in node introduce message, node_info->isOnController is set to true. Then in > NCSMDS_RED_DOWN event node_info->isOnController should be set to false. > Otherwise node_info->isOnController of earlier controllers will remain true > value. > > One more detail that is out of this patch, but can be included in the patch. > Shouldn't "cb->immd_remote_id" be set to 0 in NCSMDS_RED_DOWN event when > "cb->immd_remote_up" is set to FALSE ? > > Other minor comments inline. > > > -----Original Message----- > From: [email protected] [mailto:[email protected]] > Sent: den 26 juli 2016 13:21 > To: Zoran Milinkovic; Hung Duc Nguyen > Cc: [email protected] > Subject: [PATCH 1 of 1] imm:send 2PBE preload information for controller > IMMND [#1925] > > osaf/services/saf/immsv/README.2PBE | 7 +++++ > osaf/services/saf/immsv/immd/immd_evt.c | 38 > +++++++++++++++++++++++++++-- > osaf/services/saf/immsv/immnd/immnd_cb.h | 1 + > osaf/services/saf/immsv/immnd/immnd_evt.c | 2 +- > osaf/services/saf/immsv/immnd/immnd_main.c | 22 +++++++++++++++++ > 5 files changed, 66 insertions(+), 4 deletions(-) > > > with the #79 patch, the mds_register of standby is delayed until amfd gives > role. > Because of this the standby IMMND is not able to send pr-load information. > > The patch sends preload information by checking node_type. If the node_type is > controller the preload information is sent. > > diff --git a/osaf/services/saf/immsv/README.2PBE > b/osaf/services/saf/immsv/README.2PBE > --- a/osaf/services/saf/immsv/README.2PBE > +++ b/osaf/services/saf/immsv/README.2PBE > @@ -175,3 +175,10 @@ toggle this flag on. > For "normal", but unplanned, processor restarts, we recommend that this > flag not be > toggled on. This means that for such processor restarts, persistent writes > will not be > allowed untill both SCs are available again. > + > +2PBE with spares(#79 & #1925) > +---------------------------- > +From OpenSAG 5.0 the mds_register for IMMD is delayed until amfd comes up > and gives role. > > [Zoran] Typo. OpenSAF instead of OpenSAG update, while pushing. > +Because of this the standby role is delayed. There is a chance that chosen > standby immnd > +can be from different node than the actual node which got standby role. It > is not > +recommended to configure 2PBE with spares. > 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 > @@ -732,7 +732,11 @@ static void immd_accept_node(IMMD_CB *cb > cb->immnd_coord = node_info->immnd_key; > cb->payload_coord_dest = node_info->immnd_dest; > node_info->isCoord = true; > - } > + } else if(cb->immnd_coord == 0 && cb->mIs2Pbe){ > + LOG_NO("IMMND found at %x Cluster is loading. 2PBE configured > => Wait.", node_info->immnd_key); > + accept_evt.info.immnd.info.ctrl.canBeCoord = 2; /* 2PBE => > order preload. */ > + } > > [Zoran] The block above is not needed. "canBeCoord" has value 2 only if the > node is a controller. This is after #79 patch, as stadby mds_install is delayed. > + > > if (node_info->isCoord) { > accept_evt.info.immnd.info.ctrl.isCoord = true; > @@ -1979,8 +1983,18 @@ static uint32_t immd_evt_proc_2pbe_prelo > evt->info.pbe2.epoch, evt->info.pbe2.maxCcbId, > evt->info.pbe2.maxCommitTime); > cb->remPbe = evt->info.pbe2; > + } else if (!cb->is_rem_immnd_up && !cb->immd_remote_up) { > + LOG_NO("2PBE preload info from first remote SC can be standby > Epoch: %u MaxCcb:%u MaxTime%u", > + evt->info.pbe2.epoch, evt->info.pbe2.maxCcbId, > + evt->info.pbe2.maxCommitTime); > + cb->is_rem_immnd_up = true; > + cb->rem_immnd_dest = sinfo->dest; > + cb->remPbe = evt->info.pbe2; > + } else if (cb->is_rem_immnd_up){ > + LOG_NO("2PBE preload info from remote SC may be from spare will > be discarded"); > } > > + > if(cb->m2PbeCanLoad) { > LOG_NO("m2PbeCanLoad already set (timeout ?)"); > } else { > @@ -2632,6 +2646,7 @@ static uint32_t immd_evt_proc_mds_evt(IM > TRACE_5("Process MDS EVT NCSMDS_RED_UP, my PID:%u", getpid()); > if (cb->node_id != mds_info->node_id) { > MDS_DEST tmpDest = 0LL; > + uint32_t immnd_remote_id = 0; > TRACE_5("Remote IMMD is UP."); > > cb->immd_remote_id = > immd_get_slot_and_subslot_id_from_node_id(mds_info->node_id); > @@ -2652,16 +2667,33 @@ static uint32_t immd_evt_proc_mds_evt(IM > > node_info->isOnController = > true; > TRACE_5("Located STDBY IMMND = > %x node_id:%x", > - > immd_get_slot_and_subslot_id_from_node_id(mds_info->node_id), > + > immd_get_slot_and_subslot_id_from_node_id(mds_info->node_id), > > [Zoran] Indentation. No need for removing a tab space. The code will not be > aligned. > > mds_info->node_id); > - immd_accept_node(cb, node_info, > true, false); /* <==== Can not be sc-absence veteran if on sc. */ > + if(!cb->is_rem_immnd_up){ > + immd_accept_node(cb, > node_info, true, false); /* <==== Can not be sc-absence veteran if on sc. */ > + } if (cb->is_rem_immnd_up && > cb->mIs2Pbe){ > > [Zoran] Move IF statement to the next line. else needs to be added here. > > + immnd_remote_id = > + > immd_get_slot_and_subslot_id_from_mds_dest(node_info->immnd_dest); > + } > + if(cb->is_rem_immnd_up && > cb->mIs2Pbe){ /* The immnd and immd must be from same node*/ > > [Zoran] Remove this IF statement. The same IF statement "if > (cb->is_rem_immnd_up && cb->mIs2Pbe)" as IF statement above. > > + TRACE("The > immd_remote_id =%d immnd_remote_id=%d", > + > cb->immd_remote_id, immnd_remote_id); > + > osafassert(cb->immd_remote_id == immnd_remote_id); > + } > } > + > /* Break out of while-1. We found */ > break; > } > tmpDest = node_info->immnd_dest; > immd_immnd_info_node_getnext(&cb->immnd_tree, > &tmpDest, &node_info); > } > + /* From OpenSAF 5.1 with #79 the mds_register for immd > will happen after amfd started. > + Ihe IMMND should have started.synced or loaded > before the IMMD registers #1925 > + */ > + if ( cb->mIs2Pbe && cb->mds_role == V_DEST_RL_STANDBY){ > + cb->m2PbeCanLoad = true; > + } > } > break; > > diff --git a/osaf/services/saf/immsv/immnd/immnd_cb.h > b/osaf/services/saf/immsv/immnd/immnd_cb.h > --- a/osaf/services/saf/immsv/immnd/immnd_cb.h > +++ b/osaf/services/saf/immsv/immnd/immnd_cb.h > @@ -177,6 +177,7 @@ typedef struct immnd_cb_tag { > NCS_SEL_OBJ usr1_sel_obj; /* Selection object for USR1 signal > events */ > SaSelectionObjectT amf_sel_obj; /* Selection Object for AMF events */ > int nid_started; /* true if started by NID */ > + bool isNodeTypeController; // true node type is controller > } IMMND_CB; > > /* CB prototypes */ > 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 > @@ -8936,7 +8936,7 @@ static uint32_t immnd_evt_proc_intro_rsp > } > > cb->mCanBeCoord = evt->info.ctrl.canBeCoord; > - if((cb->mCanBeCoord == 2) && (cb->m2Pbe < 2)) { > + if((cb->mCanBeCoord == 2) && (cb->m2Pbe < 2) && > immnd_cb->isNodeTypeController) { > LOG_NO("2PBE startup arbitration initiated from IMMD"); > cb->m2Pbe = 2; > /* mCanBeCoord > 0 => This immnd resides on an SC. */ > diff --git a/osaf/services/saf/immsv/immnd/immnd_main.c > b/osaf/services/saf/immsv/immnd/immnd_main.c > --- a/osaf/services/saf/immsv/immnd/immnd_main.c > +++ b/osaf/services/saf/immsv/immnd/immnd_main.c > @@ -168,6 +168,28 @@ static uint32_t immnd_initialize(char *p > LOG_NO("Persistent Back-End capability configured, Pbe file:%s > (suffix may get added)", > immnd_cb->mPbeFile); > } > + > + FILE *fp; > + char node_type[20]; > + fp = fopen(PKGSYSCONFDIR "/node_type", "r"); > + if (fp == NULL) { > + LOG_ER("Could not open file %s - %s", PKGSYSCONFDIR > "/node_type", strerror(errno)); > + goto done; > + } > + if(EOF == fscanf(fp, "%s", node_type)) { > > [Zoran] Limit input string like "%15s" instead of "%s". If node_type value is > longer than 20 characters (it should not be), it will be buffer overflow. > Also use TAB for indentation. update while pushing. Thanks, Neel. > Thanks, > Zoran > > + LOG_ER("Could not read node type - %s", strerror(errno)); > + fclose(fp); > + goto done; > + } > + if (strcmp(node_type,"controller")==0){ > + immnd_cb->isNodeTypeController = true; > + } else if (strcmp(node_type,"payload")==0){ > + immnd_cb->isNodeTypeController = false; > + } else { > + LOG_ER("Wrong node type is specified for the node"); > + goto done; > + } > + fclose(fp); > > immnd_cb->mRim = SA_IMM_INIT_FROM_FILE; > immnd_cb->mPbeVeteran = SA_FALSE; ------------------------------------------------------------------------------ What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic patterns at an interface-level. Reveals which users, apps, and protocols are consuming the most bandwidth. Provides multi-vendor support for NetFlow, J-Flow, sFlow and other flows. Make informed decisions using capacity planning reports. http://sdm.link/zohodev2dev _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
