Hi Vu, Ack with minor comments inline I have not tested as I am not able to reproduce the scenario
Thanks, Ravi ----- Original Message ----- From: [email protected] To: [email protected], [email protected] Cc: [email protected], [email protected] Sent: Friday, December 29, 2017 3:38:48 PM GMT +05:30 Chennai, Kolkata, Mumbai, New Delhi Subject: [PATCH 1/1] imm: fix IMMND assert at veteran nodes during SYNC [#2748] During sync, if saImmOmAdminOwnerInitialize or saImmOmCcbInitialize message comes to active IMMD just right after IMMD_EVT_ND2D_SYNC_START message and before IMMND_EVT_D2ND_SYNC_START message is arrived at IMMNDs, there is possibily the request(s) is accepted at IMMND coord but is rejected at veterans. This fix introduces a flag to say abort sync has been sent by coord but not yet received the abort sync response from active IMMD. Based on that information, IMMND coord will reject such messages to align the result at veterans. --- src/imm/immnd/immnd_cb.h | 2 ++ src/imm/immnd/immnd_evt.c | 65 +++++++++++++++++++++++++++++++++++++++++----- src/imm/immnd/immnd_main.c | 1 + src/imm/immnd/immnd_proc.c | 12 +++++++++ 4 files changed, 74 insertions(+), 6 deletions(-) diff --git a/src/imm/immnd/immnd_cb.h b/src/imm/immnd/immnd_cb.h index 7614d27..e89966c 100644 --- a/src/imm/immnd/immnd_cb.h +++ b/src/imm/immnd/immnd_cb.h @@ -129,6 +129,8 @@ typedef struct immnd_cb_tag { uint8_t mIntroduced; // Ack received on introduce message uint8_t mSyncRequested; // true=> I am coord, other req sync uint8_t mPendSync; // 1=>sync announced but not received. + struct timespec mSyncAbortSentAt; // Store the start time of sync abort sent + bool mSyncAborting; // true if sync abort sent but not yet get respnse. uint8_t mSyncFinalizing; // 1=>finalizeSync sent but not received. uint8_t mSync; // true => this node is being synced (client). uint8_t mCanBeCoord; // If!=0 then SC, 2 => 2pbe arbitration, 4 => diff --git a/src/imm/immnd/immnd_evt.c b/src/imm/immnd/immnd_evt.c index 52d33dc..6b1ce43 100644 --- a/src/imm/immnd/immnd_evt.c +++ b/src/imm/immnd/immnd_evt.c @@ -10056,6 +10056,10 @@ uint32_t immnd_evt_proc_abort_sync(IMMND_CB *cb, IMMND_EVT *evt, osafassert(cb->mRulingEpoch <= evt->info.ctrl.rulingEpoch); cb->mRulingEpoch = evt->info.ctrl.rulingEpoch; + cb->mSyncAborting = false; + cb->mSyncAbortSentAt.tv_sec = 0; + cb->mSyncAbortSentAt.tv_nsec = 0; + LOG_WA("Global ABORT SYNC received for epoch %u", cb->mRulingEpoch); if (cb->mIsCoord) { /* coord should already be up to date. */ @@ -10948,6 +10952,36 @@ static void immnd_evt_proc_discard_node(IMMND_CB *cb, IMMND_EVT *evt, TRACE_LEAVE(); } [Ravi]: As per general naming convention followed in opensaf, we use "_evt" in the routine name if it is a routine processing events from IMMD or IMMND here as it is utility routine, its better not to use "_evt". You can name it as "immnd_is_sync_aborting". Also you added good description as comments in the beginning of the routing, please add function header and move the below comments as description in the header +static bool immnd_evt_is_sync_aborting(IMMND_CB* cb) +{ + /* + * When mSyncAborting = true, it means the SYNC has been aborted + * by coord, and that abort message not yet broadcasted to IMMNDs. + * Therefore, NODE STATE at IMMND coord (IMM_NODE_FULLY_AVAILABLE) is + * different with veteran node(s) (IMM_NODE_R_AVAILABLE) at the moment. + * So, IMMND_EVT_D2ND_ADMINIT or IMMND_EVT_D2ND_CCBINIT msg comes to + * IMMND coord has to be rejected to synchronize with the result + * at veterans. + * + * In addition, we check the time here to avoid the worst cases + * such as the abort message arrived at active IMMD, but later on + * it fails to send the response (e.g: hang IMMD, or reboot IMMD, etc.) + * For that reason, if we don't receive the response within 6 seconds, + * consider sending abort failed. + */ + const unsigned kTimeout = 6; /* Inherit from DEFAULT_TIMEOUT_SEC */ + time_t duration_in_second = 0xffffffff; + if (cb->mIsCoord && cb->mSyncAborting == true) { + struct timespec now, duration; + osaf_clock_gettime(CLOCK_MONOTONIC, &now); + osaf_timespec_subtract(&now, &cb->mSyncAbortSentAt, &duration); + duration_in_second = duration.tv_sec; + if (duration_in_second > kTimeout) cb->mSyncAborting = false; + } + + return (duration_in_second <= kTimeout); +} + /**************************************************************************** * Name : immnd_evt_proc_adminit_rsp * @@ -10981,9 +11015,19 @@ static void immnd_evt_proc_adminit_rsp(IMMND_CB *cb, IMMND_EVT *evt, conn = m_IMMSV_UNPACK_HANDLE_HIGH(clnt_hdl); nodeId = m_IMMSV_UNPACK_HANDLE_LOW(clnt_hdl); ownerId = evt->info.adminitGlobal.globalOwnerId; - err = - immModel_adminOwnerCreate(cb, &(evt->info.adminitGlobal.i), ownerId, - (originatedAtThisNd) ? conn : 0, nodeId); + [Ravi]: The below routine will be called in all IMMNDs in the cluster, but sync_aboring check is needed only in Coordinator So its better to check if it is CoOrdinator and if true then call the routine. And within the routine, you can remove if (cb->mIsCoord) and just call if( cb->mSyncAborting == true ) + + if (immnd_evt_is_sync_aborting(cb)) { + LOG_NO("The SYNC abort is not yet completed." + " Return TRY_AGAIN to user."); + err = SA_AIS_ERR_TRY_AGAIN; + } else { + err = immModel_adminOwnerCreate(cb, + &(evt->info.adminitGlobal.i), + ownerId, + (originatedAtThisNd) ? conn : 0, + nodeId); + } if (originatedAtThisNd) { /*Send reply to client from this ND. */ immnd_client_node_get(cb, clnt_hdl, &cl_node); @@ -12010,9 +12054,18 @@ static void immnd_evt_proc_ccbinit_rsp(IMMND_CB *cb, IMMND_EVT *evt, /* Remember latest ccb_id for IMMD recovery. */ cb->mLatestCcbId = evt->info.ccbinitGlobal.globalCcbId; - err = immModel_ccbCreate(cb, evt->info.ccbinitGlobal.i.adminOwnerId, - evt->info.ccbinitGlobal.i.ccbFlags, ccbId, - nodeId, (originatedAtThisNd) ? conn : 0); + if (immnd_evt_is_sync_aborting(cb)) { + LOG_NO("The SYNC abort is not yet completed." + " Return TRY_AGAIN to user."); + err = SA_AIS_ERR_TRY_AGAIN; + } else { + err = immModel_ccbCreate(cb, + evt->info.ccbinitGlobal.i.adminOwnerId, + evt->info.ccbinitGlobal.i.ccbFlags, + ccbId, + nodeId, + (originatedAtThisNd) ? conn : 0); + } if (originatedAtThisNd) { /*Send reply to client from this ND. */ immnd_client_node_get(cb, clnt_hdl, &cl_node); diff --git a/src/imm/immnd/immnd_main.c b/src/imm/immnd/immnd_main.c index 421e94e..8c96ff4 100644 --- a/src/imm/immnd/immnd_main.c +++ b/src/imm/immnd/immnd_main.c @@ -149,6 +149,7 @@ static uint32_t immnd_initialize(char *progname) immnd_cb->mFile = getenv("IMMSV_LOAD_FILE"); immnd_cb->clm_hdl = 0; immnd_cb->clmSelectionObject = -1; + immnd_cb->mSyncAborting = false; /* isClmNodeJoined will be intially set to true, untill CLMS service is up. from there isClmNodeJoined will be controlled by CLM membership join/left. diff --git a/src/imm/immnd/immnd_proc.c b/src/imm/immnd/immnd_proc.c index f525548..c53d249 100644 --- a/src/imm/immnd/immnd_proc.c +++ b/src/imm/immnd/immnd_proc.c @@ -1132,6 +1132,18 @@ void immnd_abortSync(IMMND_CB *cb) if (rc != NCSCC_RC_SUCCESS) { LOG_ER("Coord failed to send ABORT_SYNC over MDS err:%u", rc); } + + /* + * This mark is to say sync abort has been sent to active IMMD + * and not yet broadcasted to all IMMNDs. In other words, the NODE STATE + * at IMMND coord (IMM_NODE_FULLY_AVAILABLE) is different with veterans + * (IMM_NODE_R_AVAILABLE) at the moment. So, based on this information, + * IMMND_EVT_D2ND_ADMINIT or IMMND_EVT_D2ND_CCBINIT msg comes to IMMND + * coord has to be rejected to synchronize with the result at veterans. + */ + cb->mSyncAborting = true; + osaf_clock_gettime(CLOCK_MONOTONIC, &cb->mSyncAbortSentAt); + TRACE_LEAVE(); } -- 1.9.1 ------------------------------------------------------------------------------ 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
