Hi Vu,
Thanks for your questions and comment. I answers the questions below:
1) I will update the comment.
2) I haven't thought of this case yet. I will test and update later.
3) Client just added to client_down_list_head when client hasn't been
initialized and does not exists in database client map.
Best Regards,
Thien
-----Original Message-----
From: Nguyen Minh Vu <vu.m.ngu...@dektech.com.au>
Sent: Thursday, September 26, 2019 12:28 PM
To: thien.m.huynh <thien.m.hu...@dektech.com.au>; minh.c...@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/1] ntfd: Do not send response to client if client down
[#3084]
Hi Thien,
I have a few questions & comments below:
1) Use tab, not spaces to indent C source.
2) What if client down event comes to NTFS while it is handling the
NTFSV_INITIALIZE_REQ msg type?
3) When will a down client be removed from the database client_down_list_head
if the client has successfully done initialization ?
Regards, Vu
On 9/24/19 6:43 PM, thien.m.huynh wrote:
> Ntfd will not send response to a client when client already down.
> This will avoid timeout when ntfd send via mds.
> ---
> src/ntf/ntfd/NtfAdmin.cc | 26 ++++++++++++++++++++++++
> src/ntf/ntfd/NtfAdmin.h | 1 +
> src/ntf/ntfd/ntfs_cb.h | 9 ++++++++
> src/ntf/ntfd/ntfs_com.h | 1 +
> src/ntf/ntfd/ntfs_evt.c | 53
> +++++++++++++++++++++++++++++++++++++++++++++++-
> 5 files changed, 89 insertions(+), 1 deletion(-)
>
> diff --git a/src/ntf/ntfd/NtfAdmin.cc b/src/ntf/ntfd/NtfAdmin.cc index
> 8bbee69..c7754b5 100644
> --- a/src/ntf/ntfd/NtfAdmin.cc
> +++ b/src/ntf/ntfd/NtfAdmin.cc
> @@ -517,6 +517,27 @@ void NtfAdmin::clientRemoveMDS(MDS_DEST mds_dest) {
> }
>
> /**
> + * @brief Check clients are exists in clientMap
> + * @param mds_dest
> + * @return true/false.
> + */
> +bool NtfAdmin::isClientExisted(MDS_DEST mds_dest) {
> + TRACE_ENTER2("mdsDest: %" PRIx64, mds_dest);
> + bool found = false;
> + auto it = clientMap.begin();
> + while (it != clientMap.end()) {
> + NtfClient *client = it->second;
> + ++it;
> + if (client->getMdsDest() == mds_dest) {
> + found = true;
> + break;
> + }
> + }
> + TRACE_LEAVE();
> + return found;
> +}
> +
> +/**
> * Remove the clients that belong to the ntfa down at standby node.
> *
> * @param mds_dest
> @@ -1177,6 +1198,11 @@ void ClientsDownRemoved(MDS_DEST mds_dest) {
> NtfAdmin::theNtfAdmin->ClientsDownRemoved(mds_dest);
> }
>
> +bool isClientExisted(MDS_DEST mds_dest) {
> + osafassert(NtfAdmin::theNtfAdmin != NULL);
> + return (NtfAdmin::theNtfAdmin->isClientExisted(mds_dest));
> +}
> +
> void SearchAndSetClientsDownFlag(MDS_DEST mds_dest) {
> osafassert(NtfAdmin::theNtfAdmin != NULL);
> NtfAdmin::theNtfAdmin->SearchAndSetClientsDownFlag(mds_dest);
> diff --git a/src/ntf/ntfd/NtfAdmin.h b/src/ntf/ntfd/NtfAdmin.h index
> 4808ca9..ff4b178 100644
> --- a/src/ntf/ntfd/NtfAdmin.h
> +++ b/src/ntf/ntfd/NtfAdmin.h
> @@ -65,6 +65,7 @@ class NtfAdmin {
> void notificationLoggedConfirmed(SaNtfIdentifierT notificationId);
> void clientRemoved(unsigned int clientId);
> void clientRemoveMDS(MDS_DEST mds_dest);
> + bool isClientExisted(MDS_DEST mds_dest);
> void ClientsDownRemoved(MDS_DEST mds_dest);
> void SearchAndSetClientsDownFlag(MDS_DEST mds_dest);
> void subscriptionRemoved(unsigned int clientId, diff --git
> a/src/ntf/ntfd/ntfs_cb.h b/src/ntf/ntfd/ntfs_cb.h index
> 96eedc1..3b1d715 100644
> --- a/src/ntf/ntfd/ntfs_cb.h
> +++ b/src/ntf/ntfd/ntfs_cb.h
> @@ -38,6 +38,11 @@ typedef struct {
> MDS_DEST mds_dest;
> } ntf_client_t;
>
> +typedef struct client_down_list {
> + MDS_DEST mds_dest;
> + struct client_down_list *next;
> +} CLIENT_DOWN_LIST;
> +
> typedef struct ntfs_cb {
> SYSF_MBX mbx; /* NTFS's mailbox */
> MDS_HDL mds_hdl; /* PWE Handle for interacting with NTFAs */
> @@ -71,6 +76,10 @@ typedef struct ntfs_cb {
> NCS_SEL_OBJ usr2_sel_obj; /* Selection object for CLM initialization.*/
> uint16_t peer_mbcsv_version; /*Remeber peer NTFS MBCSV version.*/
> bool clm_initialized; // For CLM init status;
> + CLIENT_DOWN_LIST
> + *client_down_list_head; /* client down reccords - fix for not respond to
> + client if client already downs*/ bool
> + client_finalized;
> } ntfs_cb_t;
>
> extern uint32_t ntfs_cb_init(ntfs_cb_t *); diff --git
> a/src/ntf/ntfd/ntfs_com.h b/src/ntf/ntfd/ntfs_com.h index
> b9e37da..884eae5 100644
> --- a/src/ntf/ntfd/ntfs_com.h
> +++ b/src/ntf/ntfd/ntfs_com.h
> @@ -77,6 +77,7 @@ void notificationSentConfirmed(unsigned int clientId,
> void notificationLoggedConfirmed(SaNtfIdentifierT notificationId);
> void clientRemoved(unsigned int clientId);
> void clientRemoveMDS(MDS_DEST mds_dest);
> +bool isClientExisted(MDS_DEST mds_dest);
> void ClientsDownRemoved(MDS_DEST mds_dest);
> void SearchAndSetClientsDownFlag(MDS_DEST mds_dest);
> void subscriptionRemoved(unsigned int clientId, diff --git
> a/src/ntf/ntfd/ntfs_evt.c b/src/ntf/ntfd/ntfs_evt.c index
> 19b2f60..7b44cb2 100644
> --- a/src/ntf/ntfd/ntfs_evt.c
> +++ b/src/ntf/ntfd/ntfs_evt.c
> @@ -108,8 +108,34 @@ static uint32_t proc_ntfa_updn_mds_msg(ntfsv_ntfs_evt_t
> *evt)
> if (ntfs_cb->ha_state == SA_AMF_HA_STANDBY) {
> ClientsDownRemoved(evt->fr_dest);
> } else {
> + if (ntfs_cb->client_finalized == false &&
> + !isClientExisted(evt->fr_dest)) {
> + CLIENT_DOWN_LIST *client_down_rec = NULL;
> +
> + if ((client_down_rec = (CLIENT_DOWN_LIST *)malloc(
> + sizeof(CLIENT_DOWN_LIST))) == NULL) {
> + LOG_ER("memory allocation for the CLIENT_DOWN_LIST
> failed");
> + break;
> + }
> + memset(client_down_rec, 0, sizeof(CLIENT_DOWN_LIST));
> +
> + client_down_rec->mds_dest = evt->fr_dest;
> + client_down_rec->next = NULL;
> +
> + if (ntfs_cb->client_down_list_head == NULL) {
> + ntfs_cb->client_down_list_head = client_down_rec;
> + } else {
> + CLIENT_DOWN_LIST *p = ntfs_cb->client_down_list_head;
> + while (p->next != NULL) {
> + p = p->next;
> + }
> + p->next = client_down_rec;
> + }
> + TRACE_1("MDS dest added: %" PRIx64,
> client_down_rec->mds_dest);
> + }
> + ntfs_cb->client_finalized = false;
> clientRemoveMDS(evt->fr_dest);
> - }
> + }
> break;
> default:
> TRACE("Unknown evt type!!!");
> @@ -234,6 +260,7 @@ uint32_t ntfs_cb_init(ntfs_cb_t *ntfs_cb)
> ntfs_cb->fully_initialized = false;
> ntfs_cb->clm_hdl = 0;
> ntfs_cb->clm_initialized = false;
> + ntfs_cb->client_finalized = false;
> ntfs_cb->clmSelectionObject = -1;
>
> tmp = (char *)getenv("NTFSV_ENV_CACHE_SIZE"); @@ -271,9 +298,32 @@
> static uint32_t proc_initialize_msg(ntfs_cb_t *cb, ntfsv_ntfs_evt_t *evt)
> uint32_t rc = NCSCC_RC_SUCCESS;
> SaAisErrorT ais_rc = SA_AIS_OK;
> SaVersionT *version;
> + CLIENT_DOWN_LIST *client_down_rec = ntfs_cb->client_down_list_head;
> + CLIENT_DOWN_LIST *prev = NULL;
>
> TRACE_ENTER2("dest %" PRIx64, evt->fr_dest);
>
> + while (client_down_rec != NULL) {
> + if (evt->fr_dest == client_down_rec->mds_dest) {
> + if (client_down_rec == ntfs_cb->client_down_list_head) {
> + if (client_down_rec->next == NULL) {
> + ntfs_cb->client_down_list_head = NULL;
> + } else {
> + ntfs_cb->client_down_list_head = client_down_rec->next;
> + }
> + } else if (prev) {
> + prev->next = client_down_rec->next;
> + }
> + TRACE("MDS dest %" PRIx64 " already delete",
> + client_down_rec->mds_dest);
> + free(client_down_rec);
> + client_down_rec = NULL;
> + goto done;
> + }
> + prev = client_down_rec;
> + client_down_rec = client_down_rec->next;
> + }
> +
> /* Validate the version */
> version = &(evt->info.msg.info.api_info.param.init.version);
> if (!ntf_version_is_valid(version)) { @@ -314,6 +364,7 @@ static
> uint32_t proc_finalize_msg(ntfs_cb_t *cb, ntfsv_ntfs_evt_t *evt)
> clientRemoved(client_id);
> client_removed_res_lib(SA_AIS_OK, client_id, evt->fr_dest,
> &evt->mds_ctxt);
> + ntfs_cb->client_finalized = true;
> TRACE_LEAVE();
> return NCSCC_RC_SUCCESS;
> }
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
