Hi Lennart,
Reviewed the patch.
Following are the comments:
1. The timeout for each admin operation must be "smfd_cb->adminOpTimeout".
The smfd_cb->adminOpTimeout is configurable, if the adminoperation
timeout passed is less than smfd_cb->adminOpTimeout there is a chance,
to get TIMEOUT.
2. The maximum number of retries can be 25 with a sleep of
500millseconds(will be ideal).
Regards,
Neel.
On 2016/10/07 05:55 PM, Lennart Lund wrote:
> osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc | 253
> ++++++++++++------------
> 1 files changed, 126 insertions(+), 127 deletions(-)
>
>
> Fix handling of AMF admin operation so that adminOpTimeout is
> respected both for timeout of the synchronous saImmOmAdminOperationInvoke_2
> IMM API and when handling TRY AGAIN replies from both
> saImmOmAdminOperationInvoke_2
> and AMF OI
>
> diff --git a/osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc
> b/osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc
> --- a/osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc
> +++ b/osaf/services/saf/smfsv/smfd/SmfUpgradeStep.cc
> @@ -55,7 +55,7 @@
> #include "immutil.h"
> #include "smfd_smfnd.h"
> #include "smfd.h"
> -#include "osaf_time.h"
> +#include "base/time.h"
>
> /* ========================================================================
> * DEFINITIONS
> @@ -3536,52 +3536,52 @@ bool SmfAdminOperation::createNodeGroup(
> // ------------------------------------
> // Create the node group
> bool method_rc = false;
> - const uint32_t MAX_NO_RETRIES = 2;
> - uint32_t retry_cnt = 0;
> - while (++retry_cnt <= MAX_NO_RETRIES) {
> - // Creating the node group object will fail if a previously
> - // created node group was for some reason not deleted
> - // If that's the case (SA_AIS_ERR_EXIST) try to delete the
> - // node group and try again.
> - m_errno = immutil_saImmOmCcbObjectCreate_2(
> - m_ccbHandle,
> - className,
> - &nodeGroupParentDn,
> - attrValues);
> - TRACE("%s: immutil_saImmOmCcbObjectCreate_2 %s",
> - __FUNCTION__, saf_error(m_errno));
> -
> - if (m_errno == SA_AIS_ERR_EXIST) {
> - // A node group with the same name already exist
> - // May happen if a previous delete after usage has
> - // failed
> - LOG_NO("%s: saImmOmCcbObjectCreate_2 Fail %s",
> - __FUNCTION__, saf_error(m_errno));
> - bool rc = deleteNodeGroup();
> - if (rc == false) {
> - LOG_NO("%s: deleteNodeGroup() Fail",
> - __FUNCTION__);
> - method_rc = false;
> - break;
> - }
> - continue;
> - } else if (m_errno != SA_AIS_OK) {
> - LOG_NO("%s: saImmOmCcbObjectCreate_2() '%s' Fail %s",
> - __FUNCTION__, nGnodeName,
> saf_error(m_errno));
> - method_rc = false;
> - break;
> - } else {
> - m_errno = saImmOmCcbApply(m_ccbHandle);
> - if (m_errno != SA_AIS_OK) {
> - LOG_NO("%s: saImmOmCcbApply() Fail '%s'",
> - __FUNCTION__, saf_error(m_errno));
> - method_rc = false;
> - } else {
> - method_rc = true;
> - }
> - break;
> - }
> - }
> + const uint32_t MAX_NO_RETRIES = 2;
> + uint32_t retry_cnt = 0;
> + while (++retry_cnt <= MAX_NO_RETRIES) {
> + // Creating the node group object will fail if a previously
> + // created node group was for some reason not deleted
> + // If that's the case (SA_AIS_ERR_EXIST) try to delete the
> + // node group and try again.
> + m_errno = immutil_saImmOmCcbObjectCreate_2(
> + m_ccbHandle,
> + className,
> + &nodeGroupParentDn,
> + attrValues);
> + TRACE("%s: immutil_saImmOmCcbObjectCreate_2 %s",
> + __FUNCTION__, saf_error(m_errno));
> +
> + if (m_errno == SA_AIS_ERR_EXIST) {
> + // A node group with the same name already exist
> + // May happen if a previous delete after usage has
> + // failed
> + LOG_NO("%s: saImmOmCcbObjectCreate_2 Fail %s",
> + __FUNCTION__, saf_error(m_errno));
> + bool rc = deleteNodeGroup();
> + if (rc == false) {
> + LOG_NO("%s: deleteNodeGroup() Fail",
> + __FUNCTION__);
> + method_rc = false;
> + break;
> + }
> + continue;
> + } else if (m_errno != SA_AIS_OK) {
> + LOG_NO("%s: saImmOmCcbObjectCreate_2() '%s' Fail %s",
> + __FUNCTION__, nGnodeName, saf_error(m_errno));
> + method_rc = false;
> + break;
> + } else {
> + m_errno = saImmOmCcbApply(m_ccbHandle);
> + if (m_errno != SA_AIS_OK) {
> + LOG_NO("%s: saImmOmCcbApply() Fail '%s'",
> + __FUNCTION__, saf_error(m_errno));
> + method_rc = false;
> + } else {
> + method_rc = true;
> + }
> + break;
> + }
> + }
>
> if (nodeName != NULL)
> free(nodeName);
> @@ -3607,51 +3607,51 @@ bool SmfAdminOperation::deleteNodeGroup(
>
> TRACE("\t Deleting nodeGroup '%s'", nodeGroupName_s.c_str());
> bool method_rc = false;
> - const uint32_t MAX_NO_RETRIES = 2;
> - uint32_t retry_cnt = 0;
> - while (++retry_cnt <= MAX_NO_RETRIES) {
> - // Handles including ccb handle may have been invalidated by
> - // IMM resulting in SA_AIS_ERR_BAD_HANDLE response on the
> - // delete object request.
> - // If that's the case: Try to create new handles and try
> again
> - ais_rc = immutil_saImmOmCcbObjectDelete(m_ccbHandle,
> - &nodeGroupName);
> - TRACE("%s: immutil_saImmOmCcbObjectDelete %s",
> - __FUNCTION__, saf_error(m_errno));
> -
> - if (ais_rc == SA_AIS_ERR_BAD_HANDLE) {
> - LOG_NO("%s: saImmOmCcbObjectDelete Fail %s",
> - __FUNCTION__, saf_error(m_errno));
> - finalizeAllImmHandles();
> - bool rc = initAllImmHandles();
> - if (rc == false) {
> - LOG_NO("%s: getAllImmHandles() Fail",
> - __FUNCTION__);
> - method_rc = false;
> - break;
> - }
> - continue;
> - } else if (ais_rc != SA_AIS_OK) {
> - LOG_NO("%s: saImmOmCcbObjectDelete() '%s' Fail %s",
> - __FUNCTION__, nodeGroupName_s.c_str(),
> - saf_error(ais_rc));
> - method_rc = false;
> - break;
> - } else {
> - ais_rc = saImmOmCcbApply(m_ccbHandle);
> - if (ais_rc != SA_AIS_OK) {
> - LOG_NO("%s: saImmOmCcbApply() Fail '%s'",
> - __FUNCTION__, saf_error(ais_rc));
> - method_rc = false;
> - } else {
> - method_rc = true;
> - }
> - break;
> - }
> - }
> -
> - TRACE_LEAVE2("rc %s, ais_rc %s", method_rc? "Ok":"Fail",
> - saf_error(ais_rc));
> + const uint32_t MAX_NO_RETRIES = 2;
> + uint32_t retry_cnt = 0;
> + while (++retry_cnt <= MAX_NO_RETRIES) {
> + // Handles including ccb handle may have been invalidated by
> + // IMM resulting in SA_AIS_ERR_BAD_HANDLE response on the
> + // delete object request.
> + // If that's the case: Try to create new handles and try again
> + ais_rc = immutil_saImmOmCcbObjectDelete(m_ccbHandle,
> + &nodeGroupName);
> + TRACE("%s: immutil_saImmOmCcbObjectDelete %s",
> + __FUNCTION__, saf_error(m_errno));
> +
> + if (ais_rc == SA_AIS_ERR_BAD_HANDLE) {
> + LOG_NO("%s: saImmOmCcbObjectDelete Fail %s",
> + __FUNCTION__, saf_error(m_errno));
> + finalizeAllImmHandles();
> + bool rc = initAllImmHandles();
> + if (rc == false) {
> + LOG_NO("%s: getAllImmHandles() Fail",
> + __FUNCTION__);
> + method_rc = false;
> + break;
> + }
> + continue;
> + } else if (ais_rc != SA_AIS_OK) {
> + LOG_NO("%s: saImmOmCcbObjectDelete() '%s' Fail %s",
> + __FUNCTION__, nodeGroupName_s.c_str(),
> + saf_error(ais_rc));
> + method_rc = false;
> + break;
> + } else {
> + ais_rc = saImmOmCcbApply(m_ccbHandle);
> + if (ais_rc != SA_AIS_OK) {
> + LOG_NO("%s: saImmOmCcbApply() Fail '%s'",
> + __FUNCTION__, saf_error(ais_rc));
> + method_rc = false;
> + } else {
> + method_rc = true;
> + }
> + break;
> + }
> + }
> +
> + TRACE_LEAVE2("rc %s, ais_rc %s", method_rc ? "Ok" : "Fail",
> + saf_error(ais_rc));
> return method_rc;
> }
>
> @@ -3660,10 +3660,6 @@ bool SmfAdminOperation::deleteNodeGroup(
> ///
> bool SmfAdminOperation::nodeGroupAdminOperation(SaAmfAdminOperationIdT
> adminOp)
> {
> - bool rc = true;
> - SaAisErrorT oi_rc = SA_AIS_OK;
> - m_errno = SA_AIS_OK;
> -
> TRACE_ENTER();
>
> // Create the object name
> @@ -3679,42 +3675,45 @@ bool SmfAdminOperation::nodeGroupAdminOp
> // There are no operation parameters
> const SaImmAdminOperationParamsT_2 *params[1] = {NULL};
>
> - int retry = 100;
> - do {
> - TRACE("immutil_saImmOmAdminOperationInvoke_2 %s", __FUNCTION__);
> - m_errno = immutil_saImmOmAdminOperationInvoke_2(
> - m_ownerHandle,
> - &nodeGroupName, 0, adminOp, params,
> - &oi_rc, smfd_cb->adminOpTimeout);
> -
> - if (m_errno == SA_AIS_OK && oi_rc == SA_AIS_OK)
> - break;
> -
> - if (retry <= 0) {
> - LOG_NO("Fail to invoke admin operation, too many OI "
> - "TRY_AGAIN, giving up. %s", __FUNCTION__);
> + // ===================================
> + // Create the node group
> + SaAisErrorT oi_rc = SA_AIS_OK;
> + m_errno = SA_AIS_OK;
> + bool method_rc = false;
> + base::Timer adminOpTimeout(smfd_cb->adminOpTimeout);
> +
> + while (adminOpTimeout.isTimeout() == false) {
> + TRACE("saImmOmAdminOperationInvoke_2 %s", __FUNCTION__);
> + m_errno = saImmOmAdminOperationInvoke_2(
> + m_ownerHandle,
> + &nodeGroupName, 0, adminOp, params,
> + &oi_rc, adminOpTimeout.timeLeft());
> + if ((m_errno == SA_AIS_ERR_TRY_AGAIN) ||
> + (m_errno == SA_AIS_OK && oi_rc == SA_AIS_ERR_TRY_AGAIN)) {
> + base::Sleep(base::MillisToTimespec(1000));
> + continue;
> + } else if (m_errno != SA_AIS_OK) {
> + LOG_NO("%s: saImmOmAdminOperationInvoke_2 "
> + "Fail %s", __FUNCTION__, saf_error(m_errno));
> + break;
> + } else if (oi_rc != SA_AIS_OK) {
> + LOG_NO("%s: SaAmfAdminOperationId %d Fail %s",
> + __FUNCTION__, adminOp, saf_error(oi_rc));
> + m_errno = oi_rc;
> + break;
> + } else {
> + // Operation success
> + method_rc = true;
> break;
> }
> - sleep(2);
> - retry--;
> - } while (m_errno == SA_AIS_OK && oi_rc == SA_AIS_ERR_TRY_AGAIN);
> -
> - if (m_errno != SA_AIS_OK) {
> - LOG_NO("%s: saImmOmAdminOperationInvoke_2 Fail %s",
> - __FUNCTION__, saf_error(m_errno));
> - rc = false;
> }
> -
> - if ((m_errno == SA_AIS_OK) && (oi_rc != SA_AIS_OK)){
> - /* IMM om admin operation was ok but amf oi reported an error */
> - LOG_NO("%s: Set adminstate (%d) Fail %s",
> - __FUNCTION__, adminOp, saf_error(oi_rc));
> - m_errno = oi_rc;
> - rc = false;
> + if (adminOpTimeout.isTimeout()) {
> + LOG_NO("%s adminOpTimeout", __FUNCTION__);
> + method_rc = false;
> }
>
> - TRACE_LEAVE();
> - return rc;
> + TRACE_LEAVE2("%s", method_rc? "OK":"FAIL");
> + return method_rc;
> }
>
> /// Set given admin state to all units in the given unitList
------------------------------------------------------------------------------
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
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
