Hi Minh,
Sorry, my email was a bit confusing.
>> I still don't get how these osafassert(s) are helpful as a part of SG logic.
In certain situation, it may crash somewhere else. That is the reason, the
logic of checking weird SUSIs should be kept intact along with assert.
Even we are not using avd_sg_validate_headless_cached_rta(), we can use it for
temporary place holder.
In the function, avd_process_state_info_queue(), when
avd_sg_validate_headless_cached_rta() is called, we have SU1 and SU4 having
SUSI as Act.
So, two SUs in the same SG has Act assignments. This check should suffice.
So, we can invalidate it here.
Thanks
-Nagu
From: minh chau [mailto:[email protected]]
Sent: 13 June 2017 09:53
To: Nagendra Kumar
Cc: [email protected]; Praveen Malviya; [email protected];
[email protected]
Subject: Re: [PATCH 1/1] amf: Replace osafassert by log_warning to avoid cyclic
reboot [#2477]
Hi Nagu,
I seem to remember now, the function avd_sg_validate_headless_cached_rta() was
written in essence it is just debugging only, invalidating sg is not a
solution. There is a function header comment
/**
* @brief Validate all cached RTAs read from IMM after headless.
This validation is necessary. If AMFD doesn't have this
validation routine and the cached RTAs are invalid,
that would lead into *unpredictably* wrong states, which
is hard to debug (harder if no trace)
* @param Control block (AVD_CL_CB).
* @Return true if valid, false otherwise.
*/
bool avd_sg_validate_headless_cached_rta(AVD_CL_CB *cb) {
I think we are talking about "2 SUSI", more precisely it is problem of 2
different SU(s) being assigned ACTIVE
In the log, SU4 has 3 active SUSIs from 3 different SI
Jun 2 16:05:14.082046 osafamfd [285:285:src/amf/amfd/siass.cc:0440] >>
avd_susi_create: safSu=SU4,safSg=AmfDemoTwon,safApp=AmfDemoTwon
safSi=AmfDemoTwonDep2,safApp=AmfDemoTwon state=1
Jun 2 16:05:14.082080 osafamfd [285:285:src/amf/amfd/siass.cc:0440] >>
avd_susi_create: safSu=SU4,safSg=AmfDemoTwon,safApp=AmfDemoTwon
safSi=AmfDemoTwonDep1,safApp=AmfDemoTwon state=1
Jun 2 16:05:14.082108 osafamfd [285:285:src/amf/amfd/siass.cc:0440] >>
avd_susi_create: safSu=SU4,safSg=AmfDemoTwon,safApp=AmfDemoTwon
safSi=AmfDemoTwon,safApp=AmfDemoTwon state=1
As you pointed that SU1 also has 3 active SUSIs, that we read from IMM as a
helper to perform node_fail(). In this case, 3 SUSIs of SU1 must be removed and
amfd continues to create 3 standby SUSIs for another SU. When amfd completes to
remove 3 SUSIs of SU1, amfd will be able to create another 3 standby SUSIs as
of existing logic of SG.
The osafassert(s) are there just to prevent amfd to remove SUSIs of SU1 and
continue to create standby SUSIs, it even causes cyclic reboot.
I still don't get how these osafassert(s) are helpful as a part of SG logic.
The patch actually does not change any core logic of sg, it removes the assert
to avoid reboot, and make sure
avd_sg_2n_act_susi() always returns active/standby if found (do not return 2
active SU). The sg logic is still there where sg fsm calls avd_sg_2n_act_susi()
and handling the result of this function.
Thanks,
Minh
On 12/06/17 17:11, Nagendra Kumar wrote:
Hi Minh,
I could see in your trace that 3 SUSI with Act assignments(state'1'
means Act) exists:
Jun 2 16:05:26.441156 osafamfd [285:285:src/amf/amfd/sg_2n_fsm.cc:3379] TEST
>> node_fail: 'safSu=SU1,safSg=AmfDemoTwon,safApp=AmfDemoTwon', 1
Jun 2 16:05:26.441160 osafamfd [285:285:src/amf/amfd/sg_2n_fsm.cc:4095] >>
avd_su_state_determine: SU 'safSu=SU1,safSg=AmfDemoTwon,safApp=AmfDemoTwon'
Jun 2 16:05:26.441163 osafamfd [285:285:src/amf/amfd/sg_2n_fsm.cc:4120] TR
Assigned su'safSu=SU1,safSg=AmfDemoTwon,safApp=AmfDemoTwon',
si'safSi=AmfDemoTwon,safApp=AmfDemoTwon', state'1'
Jun 2 16:05:26.441167 osafamfd [285:285:src/amf/amfd/sg_2n_fsm.cc:4120] TR
Assigned su'safSu=SU1,safSg=AmfDemoTwon,safApp=AmfDemoTwon',
si'safSi=AmfDemoTwonDep1,safApp=AmfDemoTwon', state'1'
Jun 2 16:05:26.441170 osafamfd [285:285:src/amf/amfd/sg_2n_fsm.cc:4120] TR
Assigned su'safSu=SU1,safSg=AmfDemoTwon,safApp=AmfDemoTwon',
si'safSi=AmfDemoTwonDep2,safApp=AmfDemoTwon', state'1'
So, my suggestion is use this information and invalidate the SG as it has more
than one SUSI Act.
Thanks
-Nagu
-----Original Message-----
From: minh chau [mailto:[email protected]]
Sent: 09 June 2017 16:58
To: Nagendra Kumar
Cc: HYPERLINK "mailto:[email protected]"[email protected];
Praveen Malviya;
HYPERLINK "mailto:[email protected]"[email protected]; HYPERLINK
"mailto:[email protected]"[email protected]
Subject: Re: [PATCH 1/1] amf: Replace osafassert by log_warning to avoid
cyclic reboot [#2477]
Hi Nagu,
We may have to create 2 active susi(s) so that we can know the sg invalid. A
question is when the sg gets back to *valid*? We need to remove the absent
active susi, keep the real active susi and create a new standby susi.
Refreshing sg by deleting all susi(s) may get a complaint from application
because application's service has gone.
Thanks,
Minh
On 09/06/17 21:13, Nagendra Kumar wrote:
Hi Minh,
1. I am talking about changes in avd_sg_validate_headless_cached_rta().
Logic could be:
If (assignments are not proper)
{
i_sg->headless_validation = false;
}
2. In avd_cluster_tmr_init_evh():
for (const auto &value : *sg_db) {
AVD_SG *i_sg = value.second;
if ((i_sg->list_of_su.empty() == true) || (i_sg->sg_ncs_spec == true)) {
continue;
}
==============> Add somewhere below:
If (i_sg->headless_validation == false) { Ignore the SG }
<===================
while (i_sg->any_assignment_absent()) {
// failover with ABSENT SUSI, which had already been removed during
// headless, until all ABSENT SUSI(s) are failovered successfully
i_sg->failover_absent_assignment();
}
Thanks
-Nagu
-----Original Message-----
From: HYPERLINK "mailto:[email protected]"[email protected]
[mailto:[email protected]]
Sent: 09 June 2017 16:19
To: Nagendra Kumar
Cc: minh chau; HYPERLINK
"mailto:[email protected]"[email protected]; Praveen
Malviya;
HYPERLINK "mailto:[email protected]"[email protected]; HYPERLINK
"mailto:[email protected]"[email protected]
Subject: RE: [PATCH 1/1] amf: Replace osafassert by log_warning to
avoid cyclic reboot [#2477]
Hi Nagu,
I still think that replacing the osaftassert() does not change the
current logic of sg fsm as of now. Probably I don't understand by
"invalidating"
as you mean, could you please send me the patch based on your idea?
Thanks,
Minh
Hi Minh,
since locking sequence must go through the sg fsm code while rta
problem is still there
May not be; by SG locking, I mean we can just send delete to all
assignments, we can work on it later.
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
I think we should avoid changing the core logic and focus on how to
extract information, which is lost in RTA update.
As of now, we can easily avoid the situation by invalidating SG and
later we can work on permanent solution.
Thanks
-Nagu
-----Original Message-----
From: minh chau [mailto:[email protected]]
Sent: 09 June 2017 07:06
To: Nagendra Kumar; HYPERLINK
"mailto:[email protected]"[email protected]; Praveen
Malviya;
HYPERLINK "mailto:[email protected]"[email protected]
Cc: HYPERLINK
"mailto:[email protected]"[email protected]
Subject: Re: [PATCH 1/1] amf: Replace osafassert by log_warning to
avoid cyclic reboot [#2477]
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; HYPERLINK
"mailto:[email protected]"[email protected]; Praveen
Malviya;
HYPERLINK "mailto:[email protected]"[email protected]
Cc: HYPERLINK
"mailto:[email protected]"[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; HYPERLINK
"mailto:[email protected]"[email protected]; Praveen
Malviya; HYPERLINK "mailto:[email protected]"[email protected]
Cc: HYPERLINK
"mailto:[email protected]"[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: HYPERLINK "mailto:[email protected]"[email protected];
HYPERLINK "mailto:[email protected]"[email protected];
HYPERLINK "mailto:[email protected]"[email protected];
HYPERLINK "mailto:[email protected]"[email protected]
Cc: HYPERLINK
"mailto:[email protected]"[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
