Hi Minh,
We can add a check if there are more than 2 Act SUSI(on this
particular situation) in 2N red model, then we can avoid to call fsm
later(based on headless_validation) of that SG or take some action like locking
the SG itself(we can do it in #2310).
What do you think?
Thanks
-Nagu
> -----Original Message-----
> From: minh chau [mailto:[email protected]]
> Sent: 08 June 2017 06:44
> To: Nagendra Kumar; [email protected]; Praveen Malviya;
> [email protected]
> Cc: [email protected]
> Subject: Re: [PATCH 1/1] amf: Replace osafassert by log_warning to avoid
> cyclic reboot [#2477]
>
> Hi Nagu,
>
> It can be viewed as a defect from perspective of a user who has been using
> the SC absence. In the scenario of reported ticket, there was no admin
> operation on going while stopping both controllers. The user just simply
> stopped both SCs, the problem occurred due to failover during both SC going
> down, and it is from spare SUs deployment in 2N. The RTA mentioned in PR
> doc was mainly meant to the additional attributes in
> #1725 (osafAmfSISUFsmState, osafAmfSGSuOperationList,
> osafAmfSUSwitch), but you are also right that we can view as an
> enhancement since the assignment SUSI is RTA anyway.
> I think the problem is hard and agree that we need more thoughts how to
> handle such cases comprehensively (maybe #2310), for now let's fix the
> problem in a simple way as much as we can.
>
> Thanks,
> Minh
>
> On 07/06/17 19:10, Nagendra Kumar wrote:
> > Hi Minh,
> > I updated the ticket with the following details, we need to
> check with more granular levels, how to handle such cases (may be in an
> enhancement ticket):
> > =====================
> > Also, to note, it is documented as limitations in Amf PR Doc as below, so
> this ticket qualifies as Enhancement (could have been #2416 as well):
> > 2.2.11.3 Limitations
> > . Possible loss of RTA updates and SI assignment messages If both SCs
> > go down abruptly (SCs are immediately powered-off for instance), AMFD
> could fail to update RTA to IMM, the SI assignment messages sent from
> AMFND could not reach to AMFD, or vice versa. In such cases, recovery
> could be impossible, applications may have inappropriate assignment states.
> > ========================
> >
> > What do you think?
> >
> > Thanks
> > -Nagu
> >
> >> -----Original Message-----
> >> From: minh chau [mailto:[email protected]]
> >> Sent: 06 June 2017 16:35
> >> To: Nagendra Kumar; [email protected]; Praveen Malviya;
> >> [email protected]
> >> Cc: [email protected]
> >> Subject: Re: [PATCH 1/1] amf: Replace osafassert by log_warning to
> >> avoid cyclic reboot [#2477]
> >>
> >> Hi Nagu,
> >>
> >> At the beginning, I thought the osafassert(s) were there to prevent a
> >> weird thing continues to happen unpredictably. When those
> >> osafassert(s) are hit, the active controller reboots and the standby
> >> will be ok when the standby takes over active role. But in fact, the
> >> new active also gets into these osafassert(s) and cluster is facing
> >> cyclic reboot of controllers.
> >> I think the osafassert(s) would help if the SU(s) are deployed on
> >> controllers, but if SU(s) are deployed in payloads, the cyclic reboot
> >> should be happening.
> >>
> >> In addition to replacing the osafassert(s), the @stby_susi now
> >> ensures it must always point to a STANDBY susi, callers of
> >> avd_sg_2n_act_susi() are mostly in 2n SG Fsm code that already take
> >> care of @stby_susi as null and not-null. By doing this, the problem
> >> in this ticket has gone, and it should work with existing cases as
> >> @stby_susi should truly point to a STANDBY one.
> >>
> >> If you have another approach, please let me know, I can try it out.
> >>
> >> Thanks,
> >> Minh
> >>
> >> On 06/06/17 19:53, Nagendra Kumar wrote:
> >>> Hi Minh,
> >>> I think, we need to avoid this situation by doing something
> >> else. These logics are kept for putting checks and balances for not
> >> happening such situations, in my opinion, we shouldn't alter these logics.
> >>> Thanks
> >>> -Nagu
> >>>
> >>>> -----Original Message-----
> >>>> From: Minh Chau [mailto:[email protected]]
> >>>> Sent: 02 June 2017 14:54
> >>>> To: [email protected]; [email protected];
> >>>> [email protected]; [email protected]
> >>>> Cc: [email protected]; Minh Chau
> >>>> Subject: [PATCH 1/1] amf: Replace osafassert by log_warning to
> >>>> avoid
> >> cyclic
> >>>> reboot [#2477]
> >>>>
> >>>> In large cluster, the IMM sync calls mostly take no effect when
> >>>> both SCs abruptly go down, thus amfd may leaves the susi
> >>>> assignments in amfnd
> >> and
> >>>> IMM in a very unexpected states.
> >>>>
> >>>> The scenario is same as #2416, but in #2477 amfd can also see 2
> >>>> ACTIVE assignments for both SUs of a 2N SG. That leads to
> >>>> osafassert(), causes node reboot, and the same osafassert()
> >>>> repeatedly happens after node comes up.
> >>>>
> >>>> Patch replaces the osafassert() with LOG_WA, reinforces the
> >>>> checking of @stby_susi in avd_sg_2n_act_susi(). Also, a fix
> >>>> avnd_diq_rec_check_buffered_msg()
> >>>> is needed in this scenario
> >>>> ---
> >>>> src/amf/amfd/sg_2n_fsm.cc | 29 +++++++++++++++++++++++------
> >>>> src/amf/amfd/siass.cc | 20 ++++++++++----------
> >>>> src/amf/amfnd/di.cc | 6 ++++--
> >>>> 3 files changed, 37 insertions(+), 18 deletions(-)
> >>>>
> >>>> diff --git a/src/amf/amfd/sg_2n_fsm.cc b/src/amf/amfd/sg_2n_fsm.cc
> >>>> index 3a7609e07..b9748015e 100644
> >>>> --- a/src/amf/amfd/sg_2n_fsm.cc
> >>>> +++ b/src/amf/amfd/sg_2n_fsm.cc
> >>>> @@ -580,7 +580,8 @@ static AVD_SU_SI_REL
> >>>> *avd_sg_2n_act_susi(AVD_CL_CB *cb, AVD_SG *sg,
> >>>> /* Determining SUSI for su_2 may not be needed, but to make
> >>>> sure we have
> >>>> * correct SUSI.*/
> >>>> a_susi_2 = su_assigned_susi_find(su_2, &s_susi_2);
> >>>> - osafassert(a_susi_1 && s_susi_1);
> >>>> + if (a_susi_1 == nullptr) LOG_WA("a_susi_1 is null");
> >>>> + if (s_susi_1 == nullptr) LOG_WA("s_susi_1 is null");
> >>>> /* There is a case where both the SUs become Standby: When
> >>>> SU1 is locked, it
> >>>> transitions from Act to Quisced, then SU2 goes to Act
> >>>> from Std. Now
> >> if
> >>>> Act assgnment fails, then SU2 will go into Quisced state.
> >>>> Here both
> >> the
> >>>> @@ -590,11 +591,23 @@ static AVD_SU_SI_REL
> >>>> *avd_sg_2n_act_susi(AVD_CL_CB *cb, AVD_SG *sg,
> >>>> standby. */
> >>>> if ((SA_AMF_HA_QUIESCED == avd_su_state_determine(su_1)) &&
> >>>> (SA_AMF_HA_QUIESCED == avd_su_state_determine(su_2))) {
> >>>> - osafassert(a_susi_1->su == s_susi_2->su);
> >>>> - osafassert(a_susi_2->su == s_susi_1->su);
> >>>> + if (a_susi_1 && s_susi_2 && a_susi_1->su != s_susi_2->su) {
> >>>> + LOG_WA("a_susi_1->su:%s != s_susi_2->su:%s",
> >>>> + a_susi_1->su->name.c_str(), s_susi_2->su->name.c_str());
> >>>> + }
> >>>> + if (a_susi_2 && s_susi_1 && a_susi_2->su != s_susi_1->su) {
> >>>> + LOG_WA("a_susi_2->su:%s != s_susi_1->su:%s",
> >>>> + a_susi_2->su->name.c_str(), s_susi_1->su->name.c_str());
> >>>> + }
> >>>> } else {
> >>>> - osafassert(a_susi_1->su == a_susi_2->su);
> >>>> - osafassert(s_susi_1->su == s_susi_2->su);
> >>>> + if (a_susi_1 && a_susi_2 && a_susi_1->su != a_susi_2->su) {
> >>>> + LOG_WA("a_susi_1->su:%s != a_susi_2->su:%s",
> >>>> + a_susi_1->su->name.c_str(), a_susi_2->su->name.c_str());
> >>>> + }
> >>>> + if (s_susi_1 && s_susi_2 && s_susi_1->su != s_susi_2->su) {
> >>>> + LOG_WA("s_susi_1->su:%s != s_susi_2->su:%s",
> >>>> + s_susi_1->su->name.c_str(), s_susi_2->su->name.c_str());
> >>>> + }
> >>>> }
> >>>> a_susi = a_susi_1;
> >>>> s_susi = s_susi_1;
> >>>> @@ -602,7 +615,11 @@ static AVD_SU_SI_REL
> >>>> *avd_sg_2n_act_susi(AVD_CL_CB *cb, AVD_SG *sg,
> >>>> }
> >>>>
> >>>> done:
> >>>> - *stby_susi = s_susi;
> >>>> + if (s_susi && s_susi->state == SA_AMF_HA_STANDBY) {
> >>>> + *stby_susi = s_susi;
> >>>> + } else {
> >>>> + *stby_susi = nullptr;
> >>>> + }
> >>>>
> >>>> TRACE_LEAVE2("act: '%s', stdby: '%s'",
> >>>> a_susi ? a_susi->su->name.c_str() : nullptr, diff
> >>>> --git a/src/amf/amfd/siass.cc b/src/amf/amfd/siass.cc index
> >>>> 6a13836f9..f23f3a947 100644
> >>>> --- a/src/amf/amfd/siass.cc
> >>>> +++ b/src/amf/amfd/siass.cc
> >>>> @@ -218,13 +218,20 @@ void
> >>>> avd_susi_read_headless_cached_rta(AVD_CL_CB *cb) {
> >>>> std::string(strstr(osaf_extended_name_borrow(&dn), "safSi"));
> >>>> assert(si_name.empty() == false);
> >>>> AVD_SI *si = si_db->find(si_name);
> >>>> - osafassert(si);
> >>>> + if (si == nullptr) {
> >>>> + LOG_ER("SI:'%s' does not exist in AMF's sidb", si_name.c_str());
> >>>> + continue;
> >>>> + }
> >>>> SaNameT su_name;
> >>>> avsv_sanamet_init_from_association_dn(&dn, &su_name, "safSu",
> >>>> si->name.c_str());
> >>>> AVD_SU *su = su_db->find(Amf::to_string(&su_name));
> >>>> + if (su == nullptr) {
> >>>> + LOG_ER("SU:'%s' does not exist in AMF's sudb",
> >>>> + Amf::to_string(&su_name).c_str());
> >>>> + continue;
> >>>> + }
> >>>> osaf_extended_name_free(&su_name);
> >>>> - osafassert(su);
> >>>> susi = avd_su_susi_find(cb, su, si->name);
> >>>> rc = immutil_getAttr("osafAmfSISUFsmState", attributes, 0,
> >>>> &imm_susi_fsm);
> >>>> osafassert(rc == SA_AIS_OK); @@ -335,15 +342,8 @@ bool
> >>>> avd_susi_validate_headless_cached_rta(AVD_SU_SI_REL
> *present_susi,
> >>>> bool valid = true;
> >>>> // rule 1: valid ha state
> >>>> if (ha_fr_imm != present_susi->state) {
> >>>> - if (ha_fr_imm == SA_AMF_HA_QUIESCING || ha_fr_imm ==
> >>>> SA_AMF_HA_QUIESCED) {
> >>>> - // That's fine
> >>>> - ;
> >>>> - } else {
> >>>> - LOG_ER("SISU:'%s', old(imm) ha state: %d, new(sync) ha state: %d",
> >>>> + LOG_WA("SISU:'%s', old(imm) ha state: %d, new(sync) ha
> >>>> + state: %d",
> >>>> dn.c_str(), ha_fr_imm, present_susi->state);
> >>>> - valid = false;
> >>>> - goto done;
> >>>> - }
> >>>> }
> >>>> // rule 2: if ha_fr_imm is QUIESCING, one of relevant entities must
> >>>> // have adminState is SHUTTINGDOWN, otherwise re-adjust if
> >>>> possible diff --git a/src/amf/amfnd/di.cc b/src/amf/amfnd/di.cc
> >>>> index 6f0a76cda..0da907566 100644
> >>>> --- a/src/amf/amfnd/di.cc
> >>>> +++ b/src/amf/amfnd/di.cc
> >>>> @@ -1290,6 +1290,7 @@ void avnd_di_msg_ack_process(AVND_CB
> *cb,
> >>>> uint32_t mid) {
> >>>> void avnd_diq_rec_check_buffered_msg(AVND_CB *cb) {
> >>>> if ((cb->dnd_list.head != nullptr)) {
> >>>> AVND_DND_MSG_LIST *rec = 0;
> >>>> + AVND_DND_MSG_LIST *tail = cb->dnd_list.tail;
> >>>> bool found = true;
> >>>> while (found) {
> >>>> found = false;
> >>>> @@ -1318,7 +1319,7 @@ void
> >>>> avnd_diq_rec_check_buffered_msg(AVND_CB *cb) {
> >>>> rec->msg.info.avd->msg_info.n2d_su_si_assign.msg_id);
> >>>> }
> >>>> m_AVND_DIQ_REC_PUSH(cb, rec);
> >>>> - break;
> >>>> + if (tail == cb->dnd_list.tail) break;
> >>>> } else if (rec->msg.info.avd->msg_type ==
> >>>> AVSV_N2D_OPERATION_STATE_MSG) {
> >>>> if (rec->msg.info.avd->msg_info.n2d_opr_state.msg_id
> >>>> != 0) { @@ -1337,11 +1338,12 @@ void
> >>>> avnd_diq_rec_check_buffered_msg(AVND_CB *cb) {
> >>>> .raw);
> >>>> }
> >>>> m_AVND_DIQ_REC_PUSH(cb, rec);
> >>>> - break;
> >>>> + if (tail == cb->dnd_list.tail) break;
> >>>> } else {
> >>>> // delete other messages for now
> >>>> avnd_diq_rec_del(cb, rec);
> >>>> rec = cb->dnd_list.head;
> >>>> + tail = cb->dnd_list.tail;
> >>>> }
> >>>> }
> >>>> }
> >>>> --
> >>>> 2.11.0
>
------------------------------------------------------------------------------
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
