Hi Hans, I am going to remove the assertion since the original one is now always true with #2794 fix.
Do you have any concern with that? Regards, Vu > -----Original Message----- > From: Hans Nordebäck [mailto:[email protected]] > Sent: Thursday, March 15, 2018 4:56 PM > To: Vu Minh Nguyen <[email protected]> > Cc: [email protected]; [email protected]; > [email protected]; [email protected]; > [email protected] > Subject: Re: [PATCH 1/1] imm: coredump during scale-in on large > configuration [#2794] > > Hi Vu, > > I guess the assert should be kept but remove the > > reply_dest == cb->immnd_mdest_id > > condtition, or? > > /Regards HansN > > > On 03/15/2018 10:46 AM, Vu Minh Nguyen wrote: > > Hi Hans, > > > > You are right. I will remove that assert. Thanks! 😊 > > > > Regards, Vu > > > >> -----Original Message----- > >> From: Hans Nordebäck [mailto:[email protected]] > >> Sent: Thursday, March 15, 2018 4:34 PM > >> To: Vu Minh Nguyen <[email protected]>; > >> [email protected]; [email protected]; > >> [email protected]; [email protected] > >> Cc: [email protected] > >> Subject: Re: [PATCH 1/1] imm: coredump during scale-in on large > >> configuration [#2794] > >> > >> Hi Vu, > >> > >> this osafassert will never assert, no matter what values reply_dest or > >> isObjSync have, > >> > >> if reply_dest is 0 then osafassert(!reply_dest will be true. > >> > >> > >> /Regards HansN > >> > >> > >> On 03/15/2018 10:20 AM, Vu Minh Nguyen wrote: > >>> Hi Hans, > >>> > >>> I don't think so. In some cases, reply_dest could be 0 and not equal to > >>> cb- > >>> immd_mdest_id. > >>> > >>> Regards, Vu > >>> > >>>> -----Original Message----- > >>>> From: Hans Nordebäck [mailto:[email protected]] > >>>> Sent: Thursday, March 15, 2018 4:10 PM > >>>> To: Vu Minh Nguyen <[email protected]>; > >>>> [email protected]; [email protected]; > >>>> [email protected]; [email protected] > >>>> Cc: [email protected] > >>>> Subject: Re: [PATCH 1/1] imm: coredump during scale-in on large > >>>> configuration [#2794] > >>>> > >>>> Hi Vu, > >>>> > >>>> not a big deal, but you are wrong, as I mentioned, the osafassert is now > >>>> a nop as > >>>> > >>>> "reply_dest == cb->immnd_mdest_id" are always true at the osafassert > >> stmt. > >>>> /Regards HansN > >>>> > >>>> > >>>> > >>>> > >>>> On 03/15/2018 07:52 AM, Vu Minh Nguyen wrote: > >>>>> Hi Hans, > >>>>> > >>>>> See my response inline. > >>>>> > >>>>> Regards, Vu > >>>>> > >>>>>> -----Original Message----- > >>>>>> From: Hans Nordebäck [mailto:[email protected]] > >>>>>> Sent: Tuesday, March 13, 2018 9:10 PM > >>>>>> To: Vu Minh Nguyen <[email protected]>; > >>>>>> [email protected]; [email protected]; > >>>>>> [email protected]; [email protected] > >>>>>> Cc: [email protected] > >>>>>> Subject: Re: [PATCH 1/1] imm: coredump during scale-in on large > >>>>>> configuration [#2794] > >>>>>> > >>>>>> ack, review only. One question below. > >>>>>> > >>>>>> /Thanks HansN > >>>>>> > >>>>>> > >>>>>> On 03/12/2018 08:12 AM, Vu Minh Nguyen wrote: > >>>>>>> When IMMND restarts (e.g: OUT OF ORDER detection), it may get > >>>> message > >>>>>>> from active IMMD which is originated from just-dead IMMND > process. > >>>>>>> In such case, we are in confused situation - messages come from > >>>>>>> local IMMND, but not me (reply_dest != cb->immnd_mdest_id)! > >>>>>>> > >>>>>>> This patch discards such messages, notify the case to syslog > >>>>>>> instead of aborting the IMMND progress. > >>>>>>> --- > >>>>>>> src/imm/immnd/immnd_evt.c | 12 ++++++++++++ > >>>>>>> 1 file changed, 12 insertions(+) > >>>>>>> > >>>>>>> diff --git a/src/imm/immnd/immnd_evt.c > >> b/src/imm/immnd/immnd_evt.c > >>>>>>> index 228b7dd..43611a3 100644 > >>>>>>> --- a/src/imm/immnd/immnd_evt.c > >>>>>>> +++ b/src/imm/immnd/immnd_evt.c > >>>>>>> @@ -10766,6 +10766,18 @@ static uint32_t > >>>>>> immnd_evt_proc_fevs_rcv(IMMND_CB *cb, IMMND_EVT *evt, > >>>>>>> (m_IMMSV_UNPACK_HANDLE_LOW(clnt_hdl) == cb- > >>> node_id); > >>>>>>> if (originatedAtThisNd) { > >>>>>>> + /* Get the message comes from local IMMND but not > >> me > >>>>>>> + (cb->immnd_mdest_id). Probably IMMND just restarts > >>>>>>> + (e.g: OUT OF ORDER detection), and this message > >> belongs > >>>>>>> + to previous (dead) IMMND. So, discard this message. > >>>>>>> + */ > >>>>>>> + if (reply_dest && reply_dest != cb->immnd_mdest_id) { > >>>>>>> + LOG_WA("DISCARD FEVS message sent by > >> previous dead > >>>>>> IMMND"); > >>>>>>> + dequeue_outgoing(cb); > >>>>>>> + TRACE_LEAVE(); > >>>>>>> + return NCSCC_RC_SUCCESS; > >>>>>>> + } > >>>>>>> + > >>>>>>> osafassert(!reply_dest || (reply_dest == cb- > >>> immnd_mdest_id) > >>>>>> || > >>>>>> [HansN] in osafassert, isn't > >>>>>> > >>>>>> ... ||(reply_dest == cb->immnd_mdest_id) || > >>>>>> > >>>>>> redundant as > >>>>>> > >>>>>> if (reply_dest && reply_dest != cb->immnd_mdest_id) > >>>>>> > >>>>>> is checked before? > >>>>> [Vu] The check should be there, otherwise assertion could get failed > when > >>>> reply_dest != 0 and isObjSync is FALSE. > >>>>>>> isObjSync); > >>>>>>> if (cb->fevs_replies_pending) { > > ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
