Note also that if the client crashes, or the client co-located IMMND crashes before managing to send the "undo" then the applier will be cleaned up by the client-detach or node-detach logic. So this approach should definitely be safe and also quite simple.
The patch should also remove the unnecessary allocation of non-local class/obj applier data. That data wastes memory. The only point of having it at other nodes would be for om-client-read-information. The admin-op that Neel implemented for displaying applier and implementer info will actually only display extended applier info for local appliers. Currently this display is incomplete anyway since the extended applier info is not synced. /AndersBj -----Original Message----- From: Anders Björnerstedt Sent: den 8 oktober 2015 15:44 To: Hung Nguyen D; Neelakanta Reddy; Zoran Milinkovic Cc: opensaf-devel@lists.sourceforge.net Subject: RE: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes [#1504] Yes I think I agree with Hung although this is getting messay and hard to follow in terminology. I don't like the patch slogan "synchronize applier set on all nodes". The erm "synchronize" is not the one we want here. Syncrony in OpenSAF IMM for anything that needs to be cluster synchronous is achieved by fevs synchrony, period. What needs to be corrected and what this ticket is about is that agreement on the *outcome* of an applierSet. Did it succeed or not. The main strategy of the fix should be that the extra checking for implicit class/obj being bussy should only be done at the local Node were the applier is being attached. The other nodes should not do the check for appliers. This means that the implementerSet Will in some rare cases succeed at all nodes except the client node. So for the case where the client node discovers that it should Reject the implementerSet on implisit class/obj grounds, it also must send a fevs message to "undo" the successful implementeSet at the other noes. /AndersBj -----Original Message----- From: Hung Nguyen [mailto:hung.d.ngu...@dektech.com.au] Sent: den 8 oktober 2015 15:30 To: Neelakanta Reddy; Zoran Milinkovic Cc: opensaf-devel@lists.sourceforge.net Subject: Re: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes [#1504] Hi Neel, I think the solution for this should be > 2) Since the remote nodes doesn't have the applier mapping, they should not > do the ccb interference check. > In the ccb check of ImmModel::implementerSet(), 'if(isApplier)' should be > changed to 'if(isApplier && conn)'. When the same applier name is set on a newly joined node, we don't do the ccb check on the 'old' nodes. Of course, implicit class/object-implementer set won't work when the applier is moved to a different node. And I think 2) can be considered a defect rather than an enhancement. In the original design/protocol, class/obj-applier mapping is synced to remote nodes, so the remote nodes shouldn't do any check related to the mapping. Best Regards, Hung Nguyen - DEK Technologies -------------------------------------------------------------------------------- From: Neelakanta Reddy reddy.neelaka...@oracle.com Sent: Thursday, October 08, 2015 8:17PM To: Zoran Milinkovic zoran.milinko...@ericsson.com Cc: Opensaf-devel opensaf-devel@lists.sourceforge.net Subject: Re: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes [#1504] Hi zoran, The below patch does not solve the problem indicated in the ticket. . if (originatedAtThisNd) { /*Send reply to client from this ND. */ immnd_client_node_get(cb, clnt_hdl, &cl_node); - if (cl_node == NULL) { + if ((cl_node == NULL) || (discardImplementer == SA_TRUE)) { The above lines will only send IMMD_EVT_ND2D_DISCARD_IMPL when the implementer is connected from the originating node. In case , if the same applier is connected from the newly joined node DISCARD_IMPL is not sent. The solution is DISCARD_IMPL in immnd_evt_proc_cl_impl_set and immnd_evt_proc_obj_impl_set. Allow creation of applier in immnd_evt_proc_impl_set_rsp(i.e Remove the applier checks, so that the applier get connected with saImmOiImplementerSet(). If there is any active CCBs then the applier will be discarded either in class/object implementer set. /Neel. On Wednesday 07 October 2015 08:16 PM, Zoran Milinkovic wrote: > osaf/services/saf/immsv/immnd/ImmModel.cc | 12 +++++++++--- > osaf/services/saf/immsv/immnd/ImmModel.hh | 3 ++- > osaf/services/saf/immsv/immnd/immnd_evt.c | 15 ++++++++++----- > osaf/services/saf/immsv/immnd/immnd_init.h | 2 +- > 4 files changed, 22 insertions(+), 10 deletions(-) > > > If applier fails on the local node, the global discard implementer message > will be broadcasted to all nodes. > > 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 > @@ -1589,7 +1589,7 @@ SaAisErrorT > immModel_implementerSet(IMMND_CB *cb, const IMMSV_OCTET_STRING* implName, > SaUint32T implConn, SaUint32T implNodeId, > SaUint32T implId, MDS_DEST mds_dest, > - SaUint32T implTimeout) > + SaUint32T implTimeout, SaBoolT *discardImplementer) > { > return > ImmModel::instance(&cb->immModel)->implementerSet(implName, > @@ -1597,7 +1597,8 @@ immModel_implementerSet(IMMND_CB *cb, co > implNodeId, > implId, > (SaUint64T) mds_dest, > - implTimeout); > + implTimeout, > + discardImplementer); > } > > SaAisErrorT > @@ -13030,12 +13031,15 @@ ImmModel::implementerSet(const IMMSV_OCT > SaUint32T nodeId, > SaUint32T implementerId, > SaUint64T mds_dest, > - SaUint32T implTimeout) > + SaUint32T implTimeout, > + SaBoolT *discardImplementer) > { > SaAisErrorT err = SA_AIS_OK; > CcbVector::iterator i; > TRACE_ENTER(); > > + *discardImplementer = SA_FALSE; > + > if(immNotWritable() && !protocol43Allowed()) { > TRACE_LEAVE(); > return SA_AIS_ERR_TRY_AGAIN; @@ -13108,6 +13112,7 @@ > ImmModel::implementerSet(const IMMSV_OCT > TRACE("TRY_AGAIN: ccb %u is active on > object '%s' " > "bound to class applier '%s'. Can > not re-attach applier", > ccb->mId, > omit->first.c_str(), implName.c_str()); > + *discardImplementer = SA_TRUE; > err = SA_AIS_ERR_TRY_AGAIN; > goto done; > } > @@ -13121,6 +13126,7 @@ ImmModel::implementerSet(const IMMSV_OCT > TRACE("TRY_AGAIN: ccb %u is active on > object '%s' " > "bound to object applier '%s'. Can > not re-attach applier", > ccb->mId, > omit->first.c_str(), implName.c_str()); > + *discardImplementer = SA_TRUE; > err = SA_AIS_ERR_TRY_AGAIN; > goto done; > } > 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 > @@ -397,7 +397,8 @@ public: > SaUint32T nodeId, > SaUint32T ownerId, > SaUint64T mds_dest, > - SaUint32T implTimeout); > + SaUint32T implTimeout, > + SaBoolT *discardImplementer); > > SaAisErrorT classImplementerSet( > const struct > ImmsvOiImplSetReq* req, 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 > @@ -9523,6 +9523,7 @@ static void immnd_evt_proc_impl_set_rsp( > NCS_NODE_ID nodeId; > SaUint32T conn; > SaUint32T implId = 0; > + SaBoolT discardImplementer = SA_FALSE; > > osafassert(evt); > osafassert(!originatedAtThisNd || reply_dest == > cb->immnd_mdest_id); @@ -9539,13 +9540,13 @@ static void > immnd_evt_proc_impl_set_rsp( > > err = immModel_implementerSet(cb, &(evt->info.implSet.impl_name), > (originatedAtThisNd) ? conn : 0, nodeId, implId, > - reply_dest, evt->info.implSet.oi_timeout); > + reply_dest, evt->info.implSet.oi_timeout, > &discardImplementer); > > if (originatedAtThisNd) { /*Send reply to client from this ND. */ > immnd_client_node_get(cb, clnt_hdl, &cl_node); > - if (cl_node == NULL) { > + if ((cl_node == NULL) || (discardImplementer == SA_TRUE)) { > /* Client was down */ > - TRACE_5("Failed to get client node, discarding > implementer id:%u for connection: %u", implId, conn); > + TRACE_5("Discarding implementer id:%u for connection: > %u", implId, > +conn); > memset(&send_evt, '\0', sizeof(IMMSV_EVT)); > send_evt.type = IMMSV_EVT_TYPE_IMMD; > send_evt.info.immd.type = IMMD_EVT_ND2D_DISCARD_IMPL; > @@ -9556,8 > +9557,12 @@ static void immnd_evt_proc_impl_set_rsp( > /* Mark the implementer as dying to make sure no upcall > is sent to the client. > The implementer will be really discarded when > global-discard message comes */ > immModel_discardImplementer(cb, implId, SA_FALSE, NULL, > NULL); > - return; > - } else if (cl_node->mIsStale) { > + > + if(cl_node == NULL) { > + return; > + } > + } > + if (cl_node->mIsStale) { > LOG_WA("IMMND - Client went down so no response"); > return; > } > 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 > @@ -225,7 +225,7 @@ extern "C" { > SaAisErrorT > immModel_implementerSet(IMMND_CB *cb, const IMMSV_OCTET_STRING > *implName, > SaUint32T implConn, SaUint32T implNodeId, > SaUint32T implId, > - MDS_DEST mds_dest, SaUint32T implTimeout); > + MDS_DEST mds_dest, SaUint32T implTimeout, > SaBoolT > +*discardImplementer); > > SaAisErrorT > immModel_implementerClear(IMMND_CB *cb, const struct > ImmsvOiImplSetReq *req, ------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel ------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel ------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
