Hi Minh,
I am fine with the patch, but have couple of queries, please see my comments
inline
Regards,
Ravi
-----Original Message-----
From: Minh Chau [mailto:[email protected]]
Sent: Friday, October 20, 2017 4:06 AM
To: [email protected]; [email protected];
[email protected]
Cc: [email protected]; Minh Chau <[email protected]>
Subject: [PATCH 1/1] amfd: Add retry mechanism for ClmTrackStart/Stop as job
queue [#2631]
Currenty amfd does not retry ClmTrackStart/Stop after swicthover if the CLM
APIs are still unavailable due to high loaded system or connectivity issue.
This patch adds a retry mechanism for CLM APIs as other existing IMM/NTF APIs
---
src/amf/amfd/clm.cc | 83 +++++++++++++++++++++++++++++++++++++++++------
src/amf/amfd/clm.h | 27 +++++++++++++--
src/amf/amfd/imm.h | 1 +
src/amf/amfd/role.cc | 15 +++------
src/amf/amfd/sg_2n_fsm.cc | 3 +-
src/amf/amfd/sgproc.cc | 3 +-
6 files changed, 108 insertions(+), 24 deletions(-)
diff --git a/src/amf/amfd/clm.cc b/src/amf/amfd/clm.cc index d8342ca..1c82620
100644
--- a/src/amf/amfd/clm.cc
+++ b/src/amf/amfd/clm.cc
@@ -475,13 +475,16 @@ done:
return error;
}
-SaAisErrorT avd_clm_track_start(void) {
+SaAisErrorT avd_clm_track_start(AVD_CL_CB* cb) {
SaUint8T trackFlags = SA_TRACK_CURRENT | SA_TRACK_CHANGES_ONLY |
SA_TRACK_VALIDATE_STEP | SA_TRACK_START_STEP;
TRACE_ENTER();
- SaAisErrorT error =
- saClmClusterTrack_4(avd_cb->clmHandle, trackFlags, nullptr);
+ SaAisErrorT error = SA_AIS_ERR_TRY_AGAIN;
+
+ if (cb->is_clm_track_started == true) goto done;
+
+ error = saClmClusterTrack_4(cb->clmHandle, trackFlags, nullptr);
if (error != SA_AIS_OK) {
if (error == SA_AIS_ERR_TRY_AGAIN || error == SA_AIS_ERR_TIMEOUT ||
error == SA_AIS_ERR_UNAVAILABLE) { @@ -490,15 +493,20 @@ SaAisErrorT
avd_clm_track_start(void) {
LOG_ER("Failed to start cluster tracking %u", error);
}
} else {
- avd_cb->is_clm_track_started = true;
+ cb->is_clm_track_started = true;
}
+done:
TRACE_LEAVE();
return error;
}
-SaAisErrorT avd_clm_track_stop(void) {
+SaAisErrorT avd_clm_track_stop(AVD_CL_CB* cb) {
TRACE_ENTER();
- SaAisErrorT error = saClmClusterTrackStop(avd_cb->clmHandle);
+ SaAisErrorT error = SA_AIS_ERR_TRY_AGAIN;
+
+ if (cb->is_clm_track_started == false) goto done;
+
+ error = saClmClusterTrackStop(cb->clmHandle);
if (error != SA_AIS_OK) {
if (error == SA_AIS_ERR_TRY_AGAIN || error == SA_AIS_ERR_TIMEOUT ||
error == SA_AIS_ERR_UNAVAILABLE) { @@ -506,15 +514,16 @@ SaAisErrorT
avd_clm_track_stop(void) {
} else if (error == SA_AIS_ERR_NOT_EXIST) {
/* track changes was not started or stopped successfully */
LOG_WA("Failed to stop cluster tracking %u", error);
- avd_cb->is_clm_track_started = false;
+ cb->is_clm_track_started = false;
+ error = SA_AIS_OK;
} else {
LOG_ER("Failed to stop cluster tracking %u", error);
}
} else {
TRACE("Sucessfully stops cluster tracking");
- avd_cb->is_clm_track_started = false;
+ cb->is_clm_track_started = false;
}
-
+done:
TRACE_LEAVE();
return error;
}
@@ -550,7 +559,7 @@ static void *avd_clm_init_thread(void *arg) {
if (cb->avail_state_avd == SA_AMF_HA_ACTIVE) {
for (;;) {
- error = avd_clm_track_start();
+ error = avd_clm_track_start(cb);
if (error == SA_AIS_ERR_TRY_AGAIN || error == SA_AIS_ERR_TIMEOUT ||
error == SA_AIS_ERR_UNAVAILABLE) {
osaf_nanosleep(&kHundredMilliseconds);
@@ -584,3 +593,57 @@ SaAisErrorT avd_start_clm_init_bg(void) {
pthread_attr_destroy(&attr);
return SA_AIS_OK;
}
+
+AvdJobDequeueResultT ClmTrackStart::exec(const AVD_CL_CB* cb) {
+ AvdJobDequeueResultT res;
+ TRACE_ENTER();
+
+ SaAisErrorT rc = avd_clm_track_start(const_cast<AVD_CL_CB*>(cb));
+ if (rc == SA_AIS_OK) {
+ delete Fifo::dequeue();
+ res = JOB_EXECUTED;
+ } else if (rc == SA_AIS_ERR_TRY_AGAIN) {
+ TRACE("TRY-AGAIN");
+ res = JOB_ETRYAGAIN;
+ } else if (rc == SA_AIS_ERR_TIMEOUT) {
+ TRACE("TIMEOUT");
+ res = JOB_ETRYAGAIN;
+ } else if (rc == SA_AIS_ERR_UNAVAILABLE) {
+ TRACE("UNAVAILABLE");
+ res = JOB_ETRYAGAIN;
+ } else {
+ delete Fifo::dequeue();
+ LOG_ER("%s: ClmTrackStart FAILED %u", __FUNCTION__, rc);
+ res = JOB_ERR;
+ }
+
+ TRACE_LEAVE();
+ return res;
+}
+
+AvdJobDequeueResultT ClmTrackStop::exec(const AVD_CL_CB* cb) {
+ AvdJobDequeueResultT res;
+ TRACE_ENTER();
+
+ SaAisErrorT rc = avd_clm_track_stop(const_cast<AVD_CL_CB*>(cb));
+ if (rc == SA_AIS_OK) {
+ delete Fifo::dequeue();
+ res = JOB_EXECUTED;
+ } else if (rc == SA_AIS_ERR_TRY_AGAIN) {
+ TRACE("TRY-AGAIN");
+ res = JOB_ETRYAGAIN;
+ } else if (rc == SA_AIS_ERR_TIMEOUT) {
+ TRACE("TIMEOUT");
+ res = JOB_ETRYAGAIN;
+ } else if (rc == SA_AIS_ERR_UNAVAILABLE) {
+ TRACE("UNAVAILABLE");
+ res = JOB_ETRYAGAIN;
+ } else {
+ delete Fifo::dequeue();
+ LOG_ER("%s: ClmTrackStop FAILED %u", __FUNCTION__, rc);
+ res = JOB_ERR;
+ }
+
+ TRACE_LEAVE();
+ return res;
+}
diff --git a/src/amf/amfd/clm.h b/src/amf/amfd/clm.h index 6adf796..2bbe320
100644
--- a/src/amf/amfd/clm.h
+++ b/src/amf/amfd/clm.h
@@ -1,6 +1,7 @@
/* -*- OpenSAF -*-
*
* (C) Copyright 2010 The OpenSAF Foundation
+ * Copyright Ericsson AB 2017 - All Rights Reserved.
*
* This program is distributed in the hope that it will be useful, but
* WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
@@ -16,6 +17,7 @@
*/
#include <saClm.h>
+#include "amf/amfd/imm.h"
#ifndef AMF_AMFD_CLM_H_
#define AMF_AMFD_CLM_H_
@@ -23,9 +25,30 @@
struct cl_cb_tag;
extern SaAisErrorT avd_clm_init(struct cl_cb_tag *); -extern SaAisErrorT avd_clm_track_start(void); -extern SaAisErrorT avd_clm_track_stop(void);
+extern SaAisErrorT avd_clm_track_start(struct cl_cb_tag *); extern
+SaAisErrorT avd_clm_track_stop(struct cl_cb_tag *);
extern void clm_node_terminate(AVD_AVND *node); extern SaAisErrorT
avd_start_clm_init_bg(void);
+class ClmJob : public Job {
+public:
+ ClmJob() {}
+ AvdJobTypeT getJobType() { return JOB_TYPE_CLM; }
+ bool isRunnable(const AVD_CL_CB *cb) { return true;} };
+
+class ClmTrackStart : public ClmJob {
+ public:
+ ClmTrackStart() : ClmJob(){};
+ AvdJobDequeueResultT exec(const struct cl_cb_tag *cb);
+ ~ClmTrackStart() {}
+};
+
+class ClmTrackStop : public ClmJob {
+ public:
+ ClmTrackStop() : ClmJob(){};
+ AvdJobDequeueResultT exec(const struct cl_cb_tag *cb);
+ ~ClmTrackStop() {}
+};
+
#endif // AMF_AMFD_CLM_H_
diff --git a/src/amf/amfd/imm.h b/src/amf/amfd/imm.h index f0152ac..4c4f88b
100644
--- a/src/amf/amfd/imm.h
+++ b/src/amf/amfd/imm.h
@@ -56,6 +56,7 @@ typedef enum {
typedef enum {
JOB_TYPE_IMM = 1, /* A IMM job */
JOB_TYPE_NTF = 2, /* A NTF job */
+ JOB_TYPE_CLM = 3, /* A CLM job */
JOB_TYPE_ANY
} AvdJobTypeT;
diff --git a/src/amf/amfd/role.cc b/src/amf/amfd/role.cc index ec13c3b..c08033d 100644
--- a/src/amf/amfd/role.cc
+++ b/src/amf/amfd/role.cc
@@ -1066,7 +1066,8 @@ uint32_t amfd_switch_actv_qsd(AVD_CL_CB *cb) {
/* Mark AVD as Quiesced. */
cb->avail_state_avd = SA_AMF_HA_QUIESCED;
- avd_clm_track_stop();
+ if (avd_clm_track_stop(cb) != SA_AIS_OK)
+ Fifo::queue(new ClmTrackStop());
[Ravi]: I think we need to queue only if clm returns TRY_AGAIN or TIMEOUT
but here, we are generalizing it to "!= SA_AIS_OK", any reason ?
/* Go ahead and set mds role as already the NCS SU has been switched */
if (NCSCC_RC_SUCCESS !=
@@ -1105,7 +1106,6 @@ uint32_t amfd_switch_actv_qsd(AVD_CL_CB *cb) {
uint32_t amfd_switch_qsd_stdby(AVD_CL_CB *cb) {
uint32_t status = NCSCC_RC_SUCCESS;
- SaAisErrorT ais_rc;
TRACE_ENTER();
LOG_NO("Switching Quiesced --> StandBy");
@@ -1138,12 +1138,8 @@ uint32_t amfd_switch_qsd_stdby(AVD_CL_CB *cb) {
avd_pg_node_csi_del_all(cb, avnd);
}
- if (cb->is_clm_track_started == true) {
- ais_rc = avd_clm_track_stop();
- if (ais_rc != SA_AIS_OK && ais_rc != SA_AIS_ERR_NOT_EXIST) {
- LOG_ER("Failed to stop cluster tracking after switch over");
- }
- }
+ if (avd_clm_track_stop(cb) != SA_AIS_OK)
+ Fifo::queue(new ClmTrackStop());
[Ravi]: same as above
LOG_NO("Controller switch over done");
saflog(LOG_NOTICE, amfSvcUsrName, "Controller switch over done at %x", @@
-1274,8 +1270,7 @@ uint32_t amfd_switch_stdby_actv(AVD_CL_CB *cb) {
if (NCSCC_RC_SUCCESS != avd_rde_set_role(SA_AMF_HA_ACTIVE)) {
LOG_ER("rde role change failed from stdy -> Active");
}
-
- if (avd_clm_track_start() != SA_AIS_OK) {
+ if (avd_clm_track_start(cb) != SA_AIS_OK)
[Ravi]: any reason why queuing is not done when role changes from standby to
active
{
LOG_ER("Switch Standby --> Active, clm track start failed");
avd_d2d_chg_role_rsp(cb, NCSCC_RC_FAILURE, SA_AMF_HA_ACTIVE);
return NCSCC_RC_FAILURE;
diff --git a/src/amf/amfd/sg_2n_fsm.cc b/src/amf/amfd/sg_2n_fsm.cc index
3a7609e..44177d2 100644
--- a/src/amf/amfd/sg_2n_fsm.cc
+++ b/src/amf/amfd/sg_2n_fsm.cc
@@ -1767,7 +1767,8 @@ uint32_t SG_2N::susi_success_sg_realign(AVD_SU *su,
AVD_SU_SI_REL *susi,
if ((state == SA_AMF_HA_ACTIVE) &&
(cb->node_id_avd == su->su_on_node->node_info.nodeId)) {
/* This is as a result of failover, start CLM tracking*/
- (void)avd_clm_track_start();
+ if (avd_clm_track_start(cb) != SA_AIS_OK)
+ Fifo::queue(new ClmTrackStart());
[Ravi]: same as first comment
}
// Set active_services_exist at error conditions e.g. controller fail-over diff --git a/src/amf/amfd/sgproc.cc b/src/amf/amfd/sgproc.cc index d305b22..5f31194 100644
--- a/src/amf/amfd/sgproc.cc
+++ b/src/amf/amfd/sgproc.cc
@@ -2201,7 +2201,8 @@ void avd_node_down_mw_susi_failover(AVD_CL_CB *cb,
AVD_AVND *avnd) {
in avd_sg_2n_susi_sucss_sg_reln, so start here.*/
if ((i_su->sg_of_su->sg_redundancy_model == SA_AMF_2N_REDUNDANCY_MODEL) &&
(i_su->sg_of_su->sg_fsm_state == AVD_SG_FSM_STABLE))
- (void)avd_clm_track_start();
+ if (avd_clm_track_start(cb) != SA_AIS_OK)
+ Fifo::queue(new ClmTrackStart());
/* Free all the SU SI assignments*/
i_su->delete_all_susis();
--
2.7.4
