Hi Thang,
Some comments inline with [Thuan]
Best Regards,
ThuanTr
-----Original Message-----
From: thang.d.nguyen <thang.d.ngu...@dektech.com.au>
Sent: Tuesday, January 29, 2019 4:53 AM
To: gary....@dektech.com.au; minh.c...@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net
Subject: [devel] [PATCH 1/1] clmd: not send sync respond to client if node
down [#3004]
clmd will not send sync respond to client if the node that client resided on
down. This will avoid timeout when clmd send via mds.
---
src/clm/clmd/clms_cb.h | 5 ++++
src/clm/clmd/clms_evt.cc | 35 ++++++++++++++++++++++++++-
src/clm/clmd/clms_evt.h | 1 +
src/clm/clmd/clms_main.cc | 4 ++++
src/clm/clmd/clms_mds.cc | 61
+++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 105 insertions(+), 1 deletion(-)
diff --git a/src/clm/clmd/clms_cb.h b/src/clm/clmd/clms_cb.h index
4d7fdc7..6999761 100644
--- a/src/clm/clmd/clms_cb.h
+++ b/src/clm/clmd/clms_cb.h
@@ -22,6 +22,7 @@
#include "osaf/config.h"
#endif
#include <pthread.h>
+#include <list>
#include <saImm.h>
#include <saImmOi.h>
#include <saPlm.h>
@@ -238,6 +239,8 @@ typedef struct clms_cb_t {
*node_down_list_head; /*NODE_DOWN record - Fix when active node goes
down
*/
NODE_DOWN_LIST *node_down_list_tail;
+ // Node down list - Updated by MDS thread std::list<SaClmNodeIdT *>
+ mds_node_down_list;
[Thuan]: The element is simple and small, I suggest to use
std::set<NODE_ID>. We can avoid new/delete and SET to avoid duplicate
element
bool is_impl_set;
bool nid_started; /**< true if started by NID */
NCS_PATRICIA_TREE iplist; /* To temporarily store ipaddress information
@@ -245,6 +248,8 @@ typedef struct clms_cb_t {
/* Mutex protecting shared data used by the scale-out functionality */
pthread_mutex_t scale_out_data_mutex;
+ /* Mutex protecting shared data used by the delete/add node-id */
+ pthread_mutex_t node_down_list_mutex;
/* Number of occupied indices in the vectors pending_nodes[] and
* pending_node_ids[] */
size_t no_of_pending_nodes;
diff --git a/src/clm/clmd/clms_evt.cc b/src/clm/clmd/clms_evt.cc index
c2b83c2..08d4acd 100644
--- a/src/clm/clmd/clms_evt.cc
+++ b/src/clm/clmd/clms_evt.cc
@@ -17,7 +17,6 @@
*
*/
-#include "osaf/configmake.h"
#include <limits.h>
#include <pthread.h>
#include <sys/resource.h>
@@ -31,6 +30,9 @@
#include <cstdio>
#include <cstdlib>
#include <cstring>
+#include <list>
+#include <algorithm>
+#include "osaf/configmake.h"
#include "base/logtrace.h"
#include "base/ncsgl_defs.h"
#include "base/osaf_utility.h"
@@ -1514,6 +1516,31 @@ static uint32_t proc_node_get_async_msg(CLMS_CB *cb,
CLMSV_CLMS_EVT *evt) { }
/**
+ * Return true if mds node down exist
+ * @param node id
+ *
+ * @return bool
+ */
+bool clms_is_node_down(uint32_t node_id) {
+ TRACE_ENTER();
+ bool found = false;
+ std::list<NODE_ID *>::iterator it;
+ osaf_mutex_lock_ordie(&clms_cb->node_down_list_mutex);
+
+ for (it = clms_cb->mds_node_down_list.begin();
+ it != clms_cb->mds_node_down_list.end(); ++it) {
+ if (*(*it) == node_id) {
+ found = true;
+ break;
+ }
+ }
+
+ osaf_mutex_unlock_ordie(&clms_cb->node_down_list_mutex);
+ TRACE_LEAVE();
+ return found;
+}
+
+/**
* Handle a initialize message
* @param cb
* @param evt
@@ -1556,6 +1583,12 @@ static uint32_t proc_initialize_msg(CLMS_CB *cb,
CLMSV_CLMS_EVT *evt) {
if (client != nullptr)
msg.info.api_resp_info.param.client_id = client->client_id;
+ if (clms_is_node_down(node_id) == true) {
+ LOG_NO("node_id = %d already down, no need sending sync respond",
node_id);
+ if (client != nullptr) clms_client_delete(client->client_id);
+ return (uint32_t)ais_rc;
+ }
+
[Thuan] Why don't move this block before clms_client_new() then not need to
clms_client_delete()
rc = clms_mds_msg_send(cb, &msg, &evt->fr_dest, &evt->mds_ctxt,
MDS_SEND_PRIORITY_HIGH, NCSMDS_SVC_ID_CLMA);
if (rc != NCSCC_RC_SUCCESS) {
diff --git a/src/clm/clmd/clms_evt.h b/src/clm/clmd/clms_evt.h index
1005456..ef35cbc 100644
--- a/src/clm/clmd/clms_evt.h
+++ b/src/clm/clmd/clms_evt.h
@@ -92,6 +92,7 @@ extern uint32_t clms_clmresp_ok(CLMS_CB *cb,
CLMS_CLUSTER_NODE *op_node,
CLMS_TRACK_INFO *trkrec); extern uint32_t
clms_remove_clma_down_rec(CLMS_CB *cb, MDS_DEST mds_dest); extern void
clms_remove_node_down_rec(SaClmNodeIdT node_id);
+extern bool clms_is_node_down(SaClmNodeIdT node_id);
extern uint32_t clms_node_add(CLMS_CLUSTER_NODE *node, int i); extern void
clms_clmresp_error_timeout(CLMS_CB *cb, CLMS_CLUSTER_NODE *node); extern
bool clms_clma_entry_valid(CLMS_CB *cb, MDS_DEST mds_dest); diff --git
a/src/clm/clmd/clms_main.cc b/src/clm/clmd/clms_main.cc index
ad6e12e..e2c4f21 100644
--- a/src/clm/clmd/clms_main.cc
+++ b/src/clm/clmd/clms_main.cc
@@ -245,6 +245,10 @@ uint32_t clms_cb_init(CLMS_CB *clms_cb) {
if (pthread_mutex_init(&clms_cb->scale_out_data_mutex, nullptr) != 0) {
return NCSCC_RC_FAILURE;
}
+ if (pthread_mutex_init(&clms_cb->node_down_list_mutex, nullptr) != 0) {
+ return NCSCC_RC_FAILURE;
+ }
+
clms_cb->no_of_pending_nodes = 0;
clms_cb->no_of_inprogress_nodes = 0;
for (int i = 0; i != (MAX_PENDING_NODES + 1); ++i) { diff --git
a/src/clm/clmd/clms_mds.cc b/src/clm/clmd/clms_mds.cc index 58552cc..a9e004b
100644
--- a/src/clm/clmd/clms_mds.cc
+++ b/src/clm/clmd/clms_mds.cc
@@ -18,10 +18,13 @@
#include <cinttypes>
#include <cstring>
+#include <list>
+#include <algorithm>
#include "base/logtrace.h"
#include "base/ncsencdec_pub.h"
#include "clm/clmd/clms.h"
#include "clm/common/clmsv_enc_dec.h"
+#include "base/osaf_utility.h"
#define CLMS_SVC_PVT_SUBPART_VERSION 1
#define CLMS_WRT_CLMA_SUBPART_VER_AT_MIN_MSG_FMT 1 @@ -962,6 +965,60 @@
static uint32_t clms_mds_enc_flat(struct ncsmds_callback_info *info) { }
/***************************************************************************
*
+ * Name : clms_add_mds_node_down
+ *
+ * Description : This function adds a node when MDS node down arrives.
+ *
+ * Arguments : node_id : node down id.
+ *
+ * Return Values : None.
+ *
+ * Notes : None.
+
+***********************************************************************
+******/ void clms_add_mds_node_down(uint32_t node_id) {
+ TRACE_ENTER();
+ osaf_mutex_lock_ordie(&clms_cb->node_down_list_mutex);
+
+ NODE_ID *node = new NODE_ID;
+ *node = node_id;
+ clms_cb->mds_node_down_list.push_back(node);
+
+ osaf_mutex_unlock_ordie(&clms_cb->node_down_list_mutex);
+ TRACE_LEAVE();
+}
+
+/**************************************************************************
**
+ * Name : clms_delete_mds_node_down
+ *
+ * Description : This function adds a node when MDS node down arrives.
+ *
+ * Arguments : node_id : node down id.
+ *
+ * Return Values : None.
+ *
+ * Notes : None.
+
+***********************************************************************
+******/ void clms_delete_mds_node_down(uint32_t node_id) {
+ TRACE_ENTER();
+ std::list<NODE_ID *>::iterator it;
+ osaf_mutex_lock_ordie(&clms_cb->node_down_list_mutex);
+
+ for (it = clms_cb->mds_node_down_list.begin(); it !=
+ clms_cb->mds_node_down_list.end(); ++it) {
+ NODE_ID *node = *it;
+ if (*node == node_id) {
+ clms_cb->mds_node_down_list.erase(it);
+ TRACE_2("Deleted:%x", *node);
+ delete node;
+ break;
+ }
+ }
+
+ osaf_mutex_unlock_ordie(&clms_cb->node_down_list_mutex);
+ TRACE_LEAVE();
+}
+
+/**********************************************************************
+******
* Name : clms_mds_dec_flat
*
* Description : MDS decode and flatten
@@ -1026,6 +1083,9 @@ static uint32_t clms_mds_node_event(struct
ncsmds_callback_info *mds_info) {
TRACE("Adding ipinformation to the ip list: %u", node_id);
TRACE("addr_family:%u", mds_info->info.node_evt.addr_family);
TRACE("ip_addr:%s", mds_info->info.node_evt.ip_addr);
+
+ clms_delete_mds_node_down(mds_info->info.node_evt.node_id);
+
[Thuan] why not direct input local var node_id is already set before, e.g:
clms_delete_mds_node_down(node_id)
node_id = mds_info->info.node_evt.node_id;
if ((ip = (IPLIST *)ncs_patricia_tree_get(
&clms_cb->iplist, (uint8_t *)&node_id)) == nullptr) { @@
-1097,6 +1157,7 @@ static uint32_t clms_mds_node_event(struct
ncsmds_callback_info *mds_info) {
clmsv_evt->info.node_mds_info.node_id =
mds_info->info.node_evt.node_id;
clmsv_evt->info.node_mds_info.nodeup = SA_TRUE;
+ clms_add_mds_node_down(mds_info->info.node_evt.node_id);
[Thuan] can be shorten like clms_add_mds_node_down(node_id);
rc = m_NCS_IPC_SEND(&clms_cb->mbx, clmsv_evt, NCS_IPC_PRIORITY_HIGH);
if (rc != NCSCC_RC_SUCCESS) {
TRACE("IPC send failed %d", rc);
--
2.7.4
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
