Hi Nagu,
the question was related to:
while iterating over list_of_comp a component is removed/erase from
list_of_comp in remove_comp() function , is the iterator valid after the
erase?
The same applies to the iterating over list_of_su.
If iterating over a copy of list_of_comp, list_of_su that iterator
should be valid. Where are the iterators updated?
and erase while iterating in:
for (std::map<std::string, AVD_COMPCS_TYPE*>::const_iterator it =
compcstype_db->begin();
it != compcstype_db->end(); it++) {
AVD_COMPCS_TYPE *compcstype = it->second;
if (compcstype->comp == comp) {
TRACE("Standby Amfd, compcstype '%s' not
deleted",
compcstype->name.value);
compcstype_db->erase(Amf::to_string(&compcstype->name));
delete compcstype;
}
}
I prposed an alternative solution in the previous mail.
/Thanks HansN
On 03/03/2016 02:06 PM, Nagendra Kumar wrote:
> Hi Hans N,
> I had found the code working without updating, I wonder how it was printing 3
> SU names.
>>> to iterate over a copy of these lists and then the deletes should not
>>> affect respective iterators.
> Can you please elaborate it. Did you mean the below:
> for (const auto& su : node->list_of_su) {
>
> Delete SU
> }
>
> Can you please give some example ?
>
> Thanks
> -Nagu
>> -----Original Message-----
>> From: Hans Nordebäck [mailto:[email protected]]
>> Sent: 03 March 2016 12:48
>> To: Nagendra Kumar; Praveen Malviya; Minh Chau H; Gary Lee
>> Cc: [email protected]
>> Subject: RE: [devel] [PATCH 1 of 1] amfd: delete su and its child objects at
>> stdby amfd [#1683]
>>
>> Hi Nagu,
>>
>> good , my questions below was related to the three iterators and deleting
>> while iterating. /Thanks HansN
>>
>> -----Original Message-----
>> From: Nagendra Kumar [mailto:[email protected]]
>> Sent: den 3 mars 2016 07:01
>> To: Hans Nordebäck; Praveen Malviya; Minh Chau H; Gary Lee
>> Cc: [email protected]
>> Subject: RE: [devel] [PATCH 1 of 1] amfd: delete su and its child objects at
>> stdby amfd [#1683]
>>
>> Hi Hans,
>> I will send the modified patch.
>>
>> Thanks
>> -Nagu
>>
>>> -----Original Message-----
>>> From: Nagendra Kumar
>>> Sent: 03 March 2016 10:50
>>> To: Hans Nordebäck; Praveen Malviya; Minh Chau H; Gary Lee
>>> Cc: [email protected]
>>> Subject: Re: [devel] [PATCH 1 of 1] amfd: delete su and its child
>>> objects at stdby amfd [#1683]
>>>
>>> Hi Hans N,
>>> Thanks for your review. If I incorporate the comment, Is that
>> Ack ?
>>> Thanks
>>> -Nagu
>>>> -----Original Message-----
>>>> From: Hans Nordebäck [mailto:[email protected]]
>>>> Sent: 29 February 2016 19:30
>>>> To: Nagendra Kumar; Praveen Malviya; Minh Chau H; Gary Lee
>>>> Cc: [email protected]; Hans Nordebäck
>>>> Subject: RE: [PATCH 1 of 1] amfd: delete su and its child objects at
>>>> stdby amfd [#1683]
>>>>
>>>> Hi Nagu, minor correction below, (not tested or compiled though):
>>>>
>>>> std::map<std::string, AVD_COMPCS_TYPE*>::iterator it =
>>>> compcstype_db-
>>>>> begin();
>>>> while (it != compcstype_db->end() {
>>>> AVD_COMPCS_TYPE *compcstype = it->second; if (compcstype->comp
>> ==
>>>> comp) {
>>>> TRACE("Standby Amfd, compcstype '%s' not deleted",
>>>> compcstype->name.value);
>>>> it = compcstype_db->erase(it));
>>>> delete compcstype;
>>>> else
>>>> it++
>>>> }
>>>> }
>>>>
>>>> /Thanks HansN
>>>> -----Original Message-----
>>>> From: Hans Nordebäck
>>>> Sent: den 29 februari 2016 14:29
>>>> To: [email protected]; [email protected]; Minh Chau
>> H;
>>>> Gary Lee
>>>> Cc: [email protected]; Hans Nordebäck
>>>> Subject: Re: [PATCH 1 of 1] amfd: delete su and its child objects at
>>>> stdby amfd [#1683]
>>>>
>>>> Hi Nagu,
>>>>
>>>> the deepest for loop can be changed to:
>>>>
>>>>
>>>> and db_template.h is changed to:
>>>>
>>>> // c++11, returns iterator
>>>> iterator erase(const iterator &it) {return db.erase(it);}
>>>>
>>>> iterator begin() {return db.begin();}
>>>> iterator end() {return db.end();}
>>>>
>>>> const_iterator cbegin() const {return db.cbegin();}
>>>> const_iterator cend() const {return db.cend();}
>>>>
>>>> as c++11 returns an iterator in erase. It should be backwards
>>>> compatible and all present calls to begin can be changed later as needed.
>>>>
>>>> The other two loops over su and comp, why is the iterator not updated?
>>>> Elements in node list_of_su and
>>>> su list_of_comp is deleted while iterating, an alternative could be
>>>> to iterate over a copy of these lists and then the deletes should
>>>> not affect respective iterators.
>>>>
>>>> /Thanks HansN
>>>>
>>>>
>>>>
>>>>
>>>> On 02/26/2016 07:52 AM, [email protected] wrote:
>>>>> osaf/services/saf/amf/amfd/comp.cc | 2 +-
>>>>> osaf/services/saf/amf/amfd/include/comp.h | 1 +
>>>>> osaf/services/saf/amf/amfd/include/su.h | 2 +-
>>>>> osaf/services/saf/amf/amfd/node.cc | 54
>>>> +++++++++++++++++++++++++++++++
>>>>> osaf/services/saf/amf/amfd/su.cc | 2 +-
>>>>> osaf/services/saf/amf/amfd/util.cc | 16 +++++++++
>>>>> 6 files changed, 74 insertions(+), 3 deletions(-)
>>>>>
>>>>>
>>>>> If SUs are deleted at Act Amfd when Stdby Amfd has finished
>>>>> reading objects from Imm database, Stdby Amfd still has those SUs.
>>>>> Now, if node hosting those Sus are deleted, Act Amfd says Ok, but
>>>>> Stdby Amfd says it still has some SUs left on that node to be deleted.
>>>>> Hence in apply callback, Stdby Amfd crashes.
>>>>> So, this patch makes sure that in this particular scenarios, if
>>>>> node is being deleted and if there are some Sus and its child
>>>>> obkects are still there, Stdby Amfd deletes them.
>>>>>
>>>>> diff --git a/osaf/services/saf/amf/amfd/comp.cc
>>>>> b/osaf/services/saf/amf/amfd/comp.cc
>>>>> --- a/osaf/services/saf/amf/amfd/comp.cc
>>>>> +++ b/osaf/services/saf/amf/amfd/comp.cc
>>>>> @@ -1577,7 +1577,7 @@ static void comp_ccb_apply_modify_hdlr(s
>>>>> TRACE_LEAVE();
>>>>> }
>>>>>
>>>>> -static void comp_ccb_apply_delete_hdlr(struct
>>>>> CcbUtilOperationData
>>>>> *opdata)
>>>>> +void comp_ccb_apply_delete_hdlr(struct CcbUtilOperationData
>>>>> +*opdata)
>>>>> {
>>>>> TRACE_ENTER();
>>>>>
>>>>> diff --git a/osaf/services/saf/amf/amfd/include/comp.h
>>>>> b/osaf/services/saf/amf/amfd/include/comp.h
>>>>> --- a/osaf/services/saf/amf/amfd/include/comp.h
>>>>> +++ b/osaf/services/saf/amf/amfd/include/comp.h
>>>>> @@ -275,4 +275,5 @@ bool comp_is_preinstantiable(const AVD_C
>>>>> extern bool is_comp_assigned_any_csi(AVD_COMP *comp);
>>>>> extern SaAisErrorT check_comp_stability(const AVD_COMP*);
>>>>> extern AVD_CTCS_TYPE *get_ctcstype(const SaNameT
>>> *comptype_name,
>>>>> const SaNameT *cstype_name);
>>>>> +extern void comp_ccb_apply_delete_hdlr(struct
>>>>> +CcbUtilOperationData *opdata);
>>>>> #endif
>>>>> diff --git a/osaf/services/saf/amf/amfd/include/su.h
>>>>> b/osaf/services/saf/amf/amfd/include/su.h
>>>>> --- a/osaf/services/saf/amf/amfd/include/su.h
>>>>> +++ b/osaf/services/saf/amf/amfd/include/su.h
>>>>> @@ -171,5 +171,5 @@ extern SaAisErrorT avd_su_config_get(con
>>>>> * Class constructor, must be called before any other function
>>>>> */
>>>>> extern void avd_su_constructor(void);
>>>>> -
>>>>> +extern void su_ccb_apply_delete_hdlr(struct CcbUtilOperationData
>>>>> +*opdata);
>>>>> #endif
>>>>> diff --git a/osaf/services/saf/amf/amfd/node.cc
>>>>> b/osaf/services/saf/amf/amfd/node.cc
>>>>> --- a/osaf/services/saf/amf/amfd/node.cc
>>>>> +++ b/osaf/services/saf/amf/amfd/node.cc
>>>>> @@ -165,9 +165,56 @@ AVD_AVND *avd_node_new(const SaNameT
>>> *dn
>>>>> void avd_node_delete(AVD_AVND *node)
>>>>> {
>>>>> + TRACE_ENTER();
>>>>> osafassert(node->pg_csi_list.n_nodes == 0);
>>>>> if (node->node_info.nodeId)
>>>>> avd_node_delete_nodeid(node);
>>>>> + /* Check if the SUs and related objects are still left. This can
>>>>> + happen on Standby Amfd when it has just read the configuration
>>>>> + and before it becomes applier, Act Amfd deletes SUs. Those SUs
>>>>> + will be left out at Standby Amfd. Though this could be rare.*/
>>>>> + if (avd_cb->avail_state_avd != SA_AMF_HA_ACTIVE) {
>>>>> + if (node->list_of_su.empty() != true) {
>>>>> + std::vector<AVD_SU*> *su_list = nullptr;
>>>>> + su_list = &node->list_of_su;
>>>>> + std::vector<AVD_SU*>::const_iterator su_iter;
>>>>> + for (su_iter = su_list->begin(); su_iter != su_list-
>>>>> end(); ) {
>>>>> + AVD_SU *su = *su_iter;
>>>>> + TRACE("Standby Amfd, su '%s' not deleted",
>>>> su->name.value);
>>>>> + std::vector<AVD_COMP*> *comp_list =
>>>> nullptr;
>>>>> + comp_list = &su->list_of_comp;
>>>>> + std::vector<AVD_COMP*>::const_iterator
>>>> comp_iter;
>>>>> + for (comp_iter = comp_list->begin();
>>>> comp_iter != comp_list->end(); ) {
>>>>> + AVD_COMP *comp = *comp_iter;
>>>>> + TRACE("Standby Amfd, comp '%s' not
>>>> deleted", comp->comp_info.name.value);
>>>>> + /* Delete the CompCsType. */
>>>>> + for (std::map<std::string,
>>>> AVD_COMPCS_TYPE*>::const_iterator it =
>>>>> + compcstype_db-
>>>>> begin();
>>>>> + it != compcstype_db-
>>>>> end(); it++) {
>>>>> + AVD_COMPCS_TYPE
>>>> *compcstype = it->second;
>>>>> + if (compcstype->comp ==
>>>> comp) {
>>>>> + TRACE("Standby
>>>> Amfd, compcstype '%s' not deleted",
>>>>> +
>>>> compcstype->name.value);
>>>>> + compcstype_db-
>>>>> erase(Amf::to_string(&compcstype->name));
>>>>> + delete compcstype;
>>>>> + }
>>>>> + }
>>>>> +
>>>>> + /* Delete the Comp. */
>>>>> + struct CcbUtilOperationData opdata;
>>>>> + memset(&opdata.objectName, 0,
>>>> sizeof(SaNameT));
>>>>> + memcpy(opdata.objectName.value,
>>>> comp->comp_info.name.value,
>>>>> + comp-
>>>>> comp_info.name.length);
>>>>> + opdata.objectName.length = comp-
>>>>> comp_info.name.length;
>>>>> +
>>>> comp_ccb_apply_delete_hdlr(&opdata);
>>>>> + }
>>>>> + /* Delete the SU. */
>>>>> + struct CcbUtilOperationData opdata;
>>>>> + opdata.userData = su;
>>>>> + su_ccb_apply_delete_hdlr(&opdata);
>>>>> + }
>>>>> + }
>>>>> + }
>>>>> m_AVSV_SEND_CKPT_UPDT_ASYNC_RMV(avd_cb, node,
>>>> AVSV_CKPT_AVD_NODE_CONFIG);
>>>>> node_name_db->erase(Amf::to_string(&node->name));
>>>>> delete [] node->node_name;
>>>>> @@ -493,6 +540,10 @@ static SaAisErrorT node_ccb_completed_de
>>>>> return SA_AIS_ERR_BAD_OPERATION;
>>>>> }
>>>>>
>>>>> + if (avd_cb->avail_state_avd != SA_AMF_HA_ACTIVE) {
>>>>> + goto done_ok;
>>>>> + }
>>>>> +
>>>>> /* Check to see that no SUs exists on this node */
>>>>> if (node->list_of_su.empty() != true) {
>>>>> /* check whether there exists a delete operation for @@
>>>>> -
>>>> 569,6
>>>>> +620,9 @@ static SaAisErrorT node_ccb_completed_de
>>>>> }
>>>>> }
>>>>> }
>>>>> +
>>>>> +done_ok:
>>>>> + rc = SA_AIS_OK;
>>>>> opdata->userData = node;
>>>>> done:
>>>>> TRACE_LEAVE();
>>>>> diff --git a/osaf/services/saf/amf/amfd/su.cc
>>>>> b/osaf/services/saf/amf/amfd/su.cc
>>>>> --- a/osaf/services/saf/amf/amfd/su.cc
>>>>> +++ b/osaf/services/saf/amf/amfd/su.cc
>>>>> @@ -1783,7 +1783,7 @@ static void su_ccb_apply_modify_hdlr(str
>>>>> *
>>>>> * @param su
>>>>> */
>>>>> -static void su_ccb_apply_delete_hdlr(struct CcbUtilOperationData
>>>>> *opdata)
>>>>> +void su_ccb_apply_delete_hdlr(struct CcbUtilOperationData
>>>>> +*opdata)
>>>>> {
>>>>> AVD_SU *su = static_cast<AVD_SU*>(opdata->userData);
>>>>> AVD_AVND *su_node_ptr;
>>>>> diff --git a/osaf/services/saf/amf/amfd/util.cc
>>>>> b/osaf/services/saf/amf/amfd/util.cc
>>>>> --- a/osaf/services/saf/amf/amfd/util.cc
>>>>> +++ b/osaf/services/saf/amf/amfd/util.cc
>>>>> @@ -1551,6 +1551,22 @@ int amfd_file_dump(const char *filename)
>>>>> fprintf(f, " saAmfCompCurrProxyName: %s\n",
>>>> comp->saAmfCompCurrProxyName.value);
>>>>> }
>>>>>
>>>>> + fprintf(f, "COMPCS_TYPE:\n");
>>>>> + for (std::map<std::string,
>>>>> + AVD_COMPCS_TYPE*>::const_iterator it =
>>>> compcstype_db->begin();
>>>>> + it != compcstype_db->end(); it++) {
>>>>> + const AVD_COMPCS_TYPE *compcs_type = it->second;
>>>>> + fprintf(f, " dn: %s\n", compcs_type->name.value);
>>>>> + fprintf(f, " saAmfCompNumMaxActiveCSIs: %u\n",
>>>>> + compcs_type->saAmfCompNumMaxActiveCSIs);
>>>>> + fprintf(f, " saAmfCompNumMaxStandbyCSIs: %u\n",
>>>>> + compcs_type->saAmfCompNumMaxStandbyCSIs);
>>>>> + fprintf(f, " saAmfCompNumCurrActiveCSIs: %u\n",
>>>>> + compcs_type->saAmfCompNumCurrActiveCSIs);
>>>>> + fprintf(f, " saAmfCompNumCurrStandbyCSIs: %u\n",
>>>>> +
>>>>> compcs_type->saAmfCompNumCurrStandbyCSIs);
>>>>> + }
>>>>> +
>>>>> +
>>>>> fprintf(f, "Node Groups:\n");
>>>>> for (std::map<std::string, AVD_AMF_NG*>::const_iterator it =
>>>> nodegroup_db->begin();
>>>>> it != nodegroup_db->end(); it++) {
>>> ----------------------------------------------------------------------
>>> --------
>>> Site24x7 APM Insight: Get Deep Visibility into Application Performance
>>> APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
>>> Monitor end-to-end web transactions and take corrective actions now
>>> Troubleshoot faster and improve end-user experience. Signup Now!
>>> http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
>>> _______________________________________________
>>> Opensaf-devel mailing list
>>> [email protected]
>>> https://lists.sourceforge.net/lists/listinfo/opensaf-devel
------------------------------------------------------------------------------
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
