-----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
