Hi Canh, Ack from my end, did not test.
Thank you Srinivas -----Original Message----- From: Canh Van Truong [mailto:[email protected]] Sent: Monday, December 18, 2017 10:22 AM To: [email protected] Cc: [email protected] Subject: [devel] [PATCH 1/1] ntf: fix [fix ntfd remove client in standby node while not finalize in active node [#2705] The issue happen because the clients are removed in both active and standby node when getting NCSMDS_DOWN event. In standby node, ntfd get NCSMDS_DOWN event is slower than next initialize request. This cause the ntfd will removed all client from data base including new client of next initialze. Any action relate to this client will fail. Before putting NCSMDS_DOWN event down in mbx, the patch set clients down flag. And standby ntfd only remove the clients belong of mds_dest down and have flag down --- src/ntf/ntfd/NtfAdmin.cc | 62 +++++++++++++++++++++++++++++++++++++++++++++++ src/ntf/ntfd/NtfAdmin.h | 6 +++++ src/ntf/ntfd/NtfClient.cc | 11 ++++++--- src/ntf/ntfd/NtfClient.h | 6 +++++ src/ntf/ntfd/ntfs_com.h | 2 ++ src/ntf/ntfd/ntfs_evt.c | 8 ++++-- src/ntf/ntfd/ntfs_mds.c | 11 ++++++++- 7 files changed, 100 insertions(+), 6 deletions(-) diff --git a/src/ntf/ntfd/NtfAdmin.cc b/src/ntf/ntfd/NtfAdmin.cc index dad00383d..cfba836fd 100644 --- a/src/ntf/ntfd/NtfAdmin.cc +++ b/src/ntf/ntfd/NtfAdmin.cc @@ -29,6 +29,7 @@ #include "ntf/ntfd/NtfAdmin.h" #include "base/logtrace.h" +#include "base/osaf_utility.h" #include "ntf/common/ntfsv_mem.h" NtfAdmin *NtfAdmin::theNtfAdmin = NULL; @@ -42,6 +43,9 @@ NtfAdmin::NtfAdmin() { // initilalize variables notificationIdCounter = 0; clientIdCounter = 0; + + // Initialize @client_down_mutex + pthread_mutex_init(&client_down_mutex, nullptr); } NtfAdmin::~NtfAdmin() {} @@ -95,10 +99,13 @@ void NtfAdmin::clientAdded(unsigned int clientId, MDS_DEST mdsDest, rc = SA_AIS_ERR_EXIST; } else { // store new client in clientMap + osaf_mutex_lock_ordie(&client_down_mutex); clientMap[client->getClientId()] = client; + osaf_mutex_unlock_ordie(&client_down_mutex); TRACE_1("NtfAdmin::clientAdded client %u added, clientMap size is %u", client->getClientId(), (unsigned int)clientMap.size()); } + if (NULL != mdsCtxt) { client_added_res_lib(rc, clientId, mdsDest, mdsCtxt, version); } @@ -418,18 +425,22 @@ void NtfAdmin::notificationLoggedConfirmed(SaNtfIdentifierT notificationId) { */ void NtfAdmin::clientRemoved(unsigned int clientId) { // find client + TRACE_ENTER2("client_id: %d", clientId); ClientMap::iterator pos; pos = clientMap.find(clientId); if (pos != clientMap.end()) { // client found + osaf_mutex_lock_ordie(&client_down_mutex); NtfClient *client = pos->second; delete client; // remove client from client map clientMap.erase(pos); + osaf_mutex_unlock_ordie(&client_down_mutex); } else { TRACE_2("NtfAdmin::clientRemoved client %u not found", clientId); return; } + // notifications do not need to be sent to that client, remove them // scan through all notifications, remove subscriptions belonging to // the removed client @@ -467,6 +478,46 @@ void NtfAdmin::clientRemoveMDS(MDS_DEST mds_dest) { } /** + * Remove the clients that belong to the ntfa down at standby node. + * + * @param mds_dest + */ +void NtfAdmin::ClientsDownRemoved(MDS_DEST mds_dest) { + TRACE_ENTER2("MDS_DEST: %" PRIu64, mds_dest); + auto it = clientMap.begin(); + + while (it != clientMap.end()) { + NtfClient *client = it->second; + ++it; // Increase here to avoid invalid iterator after client removed + if (client->getMdsDest() == mds_dest && client->GetClientDownFlag()) { + clientRemoved(client->getClientId()); + } + } + TRACE_LEAVE(); +} + +/** + * Set flag for the clients that belong to the ntfa down. This is set +on mds + * thread and the clients are removed in ntfd thread. + * Help to prevent the disorder of coming between +NTFSV_NTFS_EVT_NTFA_DOWN event + * and checkpoint of NTFSV_NTFS_NTFSV_MSG requests. (Ticket #2705) + * + * @param mds_dest + */ +void NtfAdmin::SetClientsDownFlag(MDS_DEST mds_dest) { + TRACE_ENTER(); + osaf_mutex_lock_ordie(&client_down_mutex); + for (const auto &it : clientMap) { + NtfClient *client = it.second; + if (client->getMdsDest() == mds_dest) { + client->SetClientDownFlag(); + } + } + osaf_mutex_unlock_ordie(&client_down_mutex); + TRACE_LEAVE(); +} + +/** * The node object where the client who had the subscription is notified * so it can delete the appropriate subscription and filter object. * @@ -987,11 +1038,22 @@ void clientRemoved(unsigned int clientId) { osafassert(NtfAdmin::theNtfAdmin != NULL); NtfAdmin::theNtfAdmin->clientRemoved(clientId); } + void clientRemoveMDS(MDS_DEST mds_dest) { osafassert(NtfAdmin::theNtfAdmin != NULL); NtfAdmin::theNtfAdmin->clientRemoveMDS(mds_dest); } +void ClientsDownRemoved(MDS_DEST mds_dest) { + osafassert(NtfAdmin::theNtfAdmin != NULL); + NtfAdmin::theNtfAdmin->ClientsDownRemoved(mds_dest); +} + +void SetClientsDownFlag(MDS_DEST mds_dest) { + osafassert(NtfAdmin::theNtfAdmin != NULL); + NtfAdmin::theNtfAdmin->SetClientsDownFlag(mds_dest); +} + void subscriptionRemoved(unsigned int clientId, SaNtfSubscriptionIdT subscriptionId, MDS_SYNC_SND_CTXT *mdsCtxt) { diff --git a/src/ntf/ntfd/NtfAdmin.h b/src/ntf/ntfd/NtfAdmin.h index 1d51b3c52..d77e7f7a2 100644 --- a/src/ntf/ntfd/NtfAdmin.h +++ b/src/ntf/ntfd/NtfAdmin.h @@ -62,6 +62,8 @@ class NtfAdmin { void notificationLoggedConfirmed(SaNtfIdentifierT notificationId); void clientRemoved(unsigned int clientId); void clientRemoveMDS(MDS_DEST mds_dest); + void ClientsDownRemoved(MDS_DEST mds_dest); void + SetClientsDownFlag(MDS_DEST mds_dest); void subscriptionRemoved(unsigned int clientId, SaNtfSubscriptionIdT subscriptionId, MDS_SYNC_SND_CTXT *mdsCtxt); @@ -118,6 +120,10 @@ class NtfAdmin { unsigned int clientIdCounter; std::list<NODE_ID *> member_node_list; /*To maintain NCS node_ids of CLM memeber nodes.*/ + + // This protects in case modifying the client with down flag and // + adding/removing client client pthread_mutex_t client_down_mutex; }; #endif // NTF_NTFD_NTFADMIN_H_ diff --git a/src/ntf/ntfd/NtfClient.cc b/src/ntf/ntfd/NtfClient.cc index 7e0b2130c..7e8a926dd 100644 --- a/src/ntf/ntfd/NtfClient.cc +++ b/src/ntf/ntfd/NtfClient.cc @@ -36,9 +36,8 @@ * Flag that is set if the client is located on this node. */ NtfClient::NtfClient(unsigned int clientId, MDS_DEST mds_dest) - : readerId_(0), mdsDest_(mds_dest) { - clientId_ = clientId; - mdsDest_ = mds_dest; + : clientId_(clientId), readerId_(0), + mdsDest_(mds_dest), client_down_flag_(false) { TRACE_3("NtfClient::NtfClient NtfClient %u created mdest: %" PRIu64, clientId_, mdsDest_); } @@ -461,3 +460,9 @@ void NtfClient::set_client_version(SaVersionT* ver) { safVersion_ = *ver; } * @return ptr to SaVersionT. */ SaVersionT* NtfClient::getSafVersion() { return &safVersion_; } + + +void NtfClient::SetClientDownFlag() { client_down_flag_ = true; } + + +bool NtfClient::GetClientDownFlag() { return client_down_flag_;} diff --git a/src/ntf/ntfd/NtfClient.h b/src/ntf/ntfd/NtfClient.h index 123d06924..e975d4390 100644 --- a/src/ntf/ntfd/NtfClient.h +++ b/src/ntf/ntfd/NtfClient.h @@ -65,6 +65,8 @@ class NtfClient { bool IsA11Client() const; void set_client_version(SaVersionT *ver); SaVersionT *getSafVersion(); + void SetClientDownFlag(); + bool GetClientDownFlag(); private: void newReaderResponse(SaAisErrorT *error, unsigned int readerId, @@ -79,6 +81,10 @@ class NtfClient { MDS_DEST mdsDest_; SaVersionT safVersion_; + + // The flag to indicate that the client down and is going to deleted + bool client_down_flag_; + typedef std::map<SaNtfSubscriptionIdT, NtfSubscription *> SubscriptionMap; SubscriptionMap subscriptionMap; diff --git a/src/ntf/ntfd/ntfs_com.h b/src/ntf/ntfd/ntfs_com.h index a8ae26c9b..52546f6a1 100644 --- a/src/ntf/ntfd/ntfs_com.h +++ b/src/ntf/ntfd/ntfs_com.h @@ -74,6 +74,8 @@ void notificationSentConfirmed(unsigned int clientId, void notificationLoggedConfirmed(SaNtfIdentifierT notificationId); void clientRemoved(unsigned int clientId); void clientRemoveMDS(MDS_DEST mds_dest); +void ClientsDownRemoved(MDS_DEST mds_dest); void +SetClientsDownFlag(MDS_DEST mds_dest); void subscriptionRemoved(unsigned int clientId, SaNtfSubscriptionIdT subscriptionId, MDS_SYNC_SND_CTXT *mdsCtxt); diff --git a/src/ntf/ntfd/ntfs_evt.c b/src/ntf/ntfd/ntfs_evt.c index 612c124d6..88f58f78d 100644 --- a/src/ntf/ntfd/ntfs_evt.c +++ b/src/ntf/ntfd/ntfs_evt.c @@ -104,8 +104,12 @@ static uint32_t proc_ntfa_updn_mds_msg(ntfsv_ntfs_evt_t *evt) case NTFSV_NTFS_EVT_NTFA_UP: break; case NTFSV_NTFS_EVT_NTFA_DOWN: - /* Remove this NTFA entry from our processing lists */ - clientRemoveMDS(evt->fr_dest); + // Remove the clients belong to the ntfa down with MDS_DEST + if (ntfs_cb->ha_state == SA_AMF_HA_STANDBY) { + ClientsDownRemoved(evt->fr_dest); + } else { + clientRemoveMDS(evt->fr_dest); + } break; default: TRACE("Unknown evt type!!!"); diff --git a/src/ntf/ntfd/ntfs_mds.c b/src/ntf/ntfd/ntfs_mds.c index 425627d5a..59b773002 100644 --- a/src/ntf/ntfd/ntfs_mds.c +++ b/src/ntf/ntfd/ntfs_mds.c @@ -16,8 +16,9 @@ */ #include "base/ncsencdec_pub.h" -#include "ntfs.h" #include "ntf/common/ntfsv_enc_dec.h" +#include "ntf/ntfd/ntfs.h" +#include "ntf/ntfd/ntfs_com.h" #define NTFS_SVC_PVT_SUBPART_VERSION 1 #define NTFS_WRT_NTFA_SUBPART_VER_AT_MIN_MSG_FMT 1 @@ -965,6 +966,14 @@ static uint32_t mds_svc_event(struct ncsmds_callback_info *info) evt->info.mds_info.mds_dest_id = info->info.svc_evt.i_dest; + // Set flag for client down at standby node then ntfd + // just removes only the downed clients. This is helpful + // in some cases ntfd receives this event after + // checkpoint of initializing new client + if (ntfs_cb->ha_state == SA_AMF_HA_STANDBY) { + SetClientsDownFlag(evt->fr_dest); + } + /* Push the event and we are done */ if (m_NCS_IPC_SEND(&ntfs_cb->mbx, evt, NCS_IPC_PRIORITY_HIGH) != -- 2.13.0 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! https://urldefense.proofpoint.com/v2/url?u=http-3A__sdm.link_slashdot&d=DwICAg&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=rU6x356sikQZSi7Ttc2DuiqAgbc0QIeANg72N5AllVc&m=RTTQ47YfD_tMPXvD_cRX6H-nwcMPNdS-9nAo0eBIh4g&s=_cymElGPHFdS0qcZ2WQMXywT0jJeHoJjNSIOT9NvrAs&e= _______________________________________________ Opensaf-devel mailing list [email protected] https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.sourceforge.net_lists_listinfo_opensaf-2Ddevel&d=DwICAg&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=rU6x356sikQZSi7Ttc2DuiqAgbc0QIeANg72N5AllVc&m=RTTQ47YfD_tMPXvD_cRX6H-nwcMPNdS-9nAo0eBIh4g&s=qF0lGVEE_EBaZd7KS3Y5H5SObUbEHPDBMuHF_20oyZU&e= ------------------------------------------------------------------------------ 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
