Hi Zoran,

Please find the inline comments below.
This patch fixes the problem but there are a few more things need to be done.

1) The only thing that the remote nodes should know about an appliers is its 
existence.
The class<->applier mapping and object<->applier mapping should not be created 
on remote nodes at all.
In these function:
   immnd_evt_proc_cl_impl_set()
   immnd_evt_proc_cl_impl_rel()
   immnd_evt_proc_obj_impl_set()
   immnd_evt_proc_obj_impl_rel()
if it's an applier and not originated at this node, we should do nothing and 
return.


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


I also have some inline comments for the patch, please find them below.

Best Regards,

Hung Nguyen - DEK Technologies


--------------------------------------------------------------------------------
From: Zoran Milinkovic zoran.milinko...@ericsson.com
Sent: Wednesday, October 07, 2015 10:10AM
To: Neelakanta Reddy
     reddy.neelaka...@oracle.com
Cc: Opensaf-devel
     opensaf-devel@lists.sourceforge.net
Subject: [devel] [PATCH 1 of 1] imm: synchronize applier set on all nodes       
[#1504]


  osaf/services/saf/immsv/immnd/ImmModel.cc |  44 
+++++++++++++++++++++++++++++++
  osaf/services/saf/immsv/immnd/immnd_evt.c |   7 +++-
  2 files changed, 49 insertions(+), 2 deletions(-)


In the first request of OiImplementerSet, from library to IMMND, a pre-check is 
added for checking if the applier can be set on a class or an object.
The pre-check will improve response and skip sending the message over fevs, if 
the applier cannot be set to the class or the object.

If the pre-check is successful, then it might be a short time frame that 
applier set can still fail until the message sent over fevs reach all nodes.
If applier set fails at this point, discard implementer message will be sent 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
@@ -1617,6 +1617,50 @@ immModel_implIsFree(IMMND_CB *cb, const
      if(impl->mId == 0) {return SA_AIS_OK;}
      if(impl->mDying) {return SA_AIS_ERR_TRY_AGAIN;}
  
+    /* Check if applier can be attached to a class or an object */
+    if(impl->mApplier) {
+        CcbVector::iterator i;
+        CcbInfo *ccb;
+        ObjectMutationMap::iterator omit;
+        ObjectMap::iterator oi;
+        ObjectInfo *obj;
+        ImplementerSetMap::iterator ismIter;
+
+        for(i=sCcbVector.begin(); i!=sCcbVector.end(); ++i) {
+            ccb = (*i);
+            if(ccb->isActive()) {
+                for(omit=ccb->mMutations.begin(); omit!=ccb->mMutations.end(); 
++omit) {
+                    oi = sObjectMap.find(omit->first);
+                    osafassert(oi != sObjectMap.end());
+                    obj = oi->second;
+
+                    if( ! obj->mClassInfo->mAppliers.empty()) {
+                        ImplementerSet::iterator ii = 
obj->mClassInfo->mAppliers.begin();
+                        for(; ii != obj->mClassInfo->mAppliers.end(); ++ii) {
+                            if((*ii) == impl) {
+                                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());
+                                return SA_AIS_ERR_EXIST;
[Hung] Why it's SA_AIS_ERR_EXIST and not SA_AIS_ERR_TRY_AGAIN?

+                            }
+                        }
+                    }
+
+                    ismIter = sObjAppliersMap.find(omit->first);
+                    if(ismIter != sObjAppliersMap.end()) {
+                        ImplementerSet *implSet = ismIter->second;
+                        if(implSet->find(impl) != implSet->end()) {
+                            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());
+                            return SA_AIS_ERR_EXIST;
+                        }
+                    }
+                }
+            }
+        }
+    }
+
      return SA_AIS_ERR_EXIST;
  }
  
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
@@ -9543,7 +9543,7 @@ static void immnd_evt_proc_impl_set_rsp(
  
        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 || (err == SA_AIS_ERR_TRY_AGAIN && 
!cl_node->mIsStale)) {
[Hung] This is for appliers only, not for regular OI. The if statement 
should include 'isApplier' (or something like that). Why we have to 
check for 'cl_node->mIsStale' here? Stale or not we still have to 
discard the applier on remote nodes.

                        /* Client was down */
                        TRACE_5("Failed to get client node, discarding 
implementer id:%u for connection: %u", implId, conn);
[Hung] The trace is misleading in case of discarding applier.

                        memset(&send_evt, '\0', sizeof(IMMSV_EVT));
@@ -9556,7 +9556,10 @@ 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;
+
+                       if(cl_node == NULL) {
+                               return;
+                       }
                } else if (cl_node->mIsStale) {
[Hung] Should be a new if statement instead of 'else if'. Just to make 
sure IMMND don't send response message when ERR_TRY_AGAIN and mIsStale 
coincide.

                        LOG_WA("IMMND - Client went down so no response");
                        return;

------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel


------------------------------------------------------------------------------
Full-scale, agent-less Infrastructure Monitoring from a single dashboard
Integrate with 40+ ManageEngine ITSM Solutions for complete visibility
Physical-Virtual-Cloud Infrastructure monitoring from one console
Real user monitoring with APM Insights and performance trend reports 
Learn More http://pubads.g.doubleclick.net/gampad/clk?id=247754911&iu=/4140
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to