Hi Nagu,
In the situation of this ticket, we have 2 active susi(s), one is absent
susi from sc, another one is real susi from pl. We can have a check to
avoid restoring the active absent susi if there is a real active susi
already, but I feel doing this is not the way that amfd has been
encouraged to use sg fsm to recover from headless. The skeleton of
headless recovery has built up on using sg fsm node_fail(), we can add a
check of 2 active susi(s), but it is not the only form of rta problem,
there should be many variant symptom. We need to enhance the sg fsm code
for a wider coverage.
Locking the sg itself is one option, my feeling is that locking sg may
not work as expected, since locking sequence must go through the sg fsm
code while rta problem is still there. I think we need to failover all
absent susi(s) (by using sg fsm node_fail()) before amfd can do anything
further.
I have just attached to ticket the trace files for your reference, amfd
has succeeded to remove the active absent susi (for this ticket) with
the patch of #2477
Thanks,
Minh
On 08/06/17 21:25, Nagendra Kumar wrote:
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
