Re: [devel] [PATCH 1/1] mds: not waste 1.5s in waiting Adest already down to send response message [#3102]
Hi Thuan,
1- Can you point out where is the mds code that waits for 1.5 seconds,
is it hard coded within 1.5 secs?
2- Is existing db (mds_c_db.c) in mds not enough so we need to introduce
adest_list? I think mds must have a memory of adest, perhaps in another
implicit form, so mds can validate an adest, a svc_id associated with adest.
thanks
Minh
On 22/10/19 11:14 am, thuan.tran wrote:
- When sending response message which Adest not exist (already down)
current MDS try to wait for 1.5 seconds before conclude no route to
send response message.
- There are 2 scenarios may have:
UP -> DOWN -> receive SNDRSP -> response timeout after 1.5s
UP -> receive SNDRSP -> DOWN -> response timeout after 1.5s
- With this change, MDS will not waste for 1.5s which can cause trouble
for higher layer services, e.g: ntf, imm, etc...
---
src/mds/mds_c_api.c | 70 -
src/mds/mds_c_sndrcv.c | 52 --
src/mds/mds_core.h | 25 +--
src/mds/mds_dt2c.h | 2 +-
src/mds/mds_dt_common.c | 22 -
5 files changed, 162 insertions(+), 9 deletions(-)
diff --git a/src/mds/mds_c_api.c b/src/mds/mds_c_api.c
index 132555b8e..5dd30c536 100644
--- a/src/mds/mds_c_api.c
+++ b/src/mds/mds_c_api.c
@@ -1900,6 +1900,32 @@ uint32_t mds_mcm_svc_up(PW_ENV_ID pwe_id, MDS_SVC_ID
svc_id, V_DEST_RL role,
/*** Validation for SCOPE **/
+ if (adest != m_MDS_GET_ADEST) {
+ MDS_ADEST_INFO *adest_info =
+ (MDS_ADEST_INFO *)ncs_patricia_tree_get(
+ _mds_mcm_cb->adest_list,
+ (uint8_t *));
+ if (!adest_info) {
+ /* Add adest to adest list */
+ adest_info = m_MMGR_ALLOC_ADEST_INFO;
+ memset(adest_info, 0, sizeof(MDS_ADEST_INFO));
+ adest_info->adest = adest;
+ adest_info->node.key_info =
+ (uint8_t *)_info->adest;
+ adest_info->svc_cnt = 1;
+ adest_info->tmr_start = false;
+ ncs_patricia_tree_add(
+ _mds_mcm_cb->adest_list,
+ (NCS_PATRICIA_NODE *)adest_info);
+ m_MDS_LOG_DBG("MCM:API: Adest=%" PRIu64
+ " svc_cnt=%u", adest, adest_info->svc_cnt);
+ } else {
+ adest_info->svc_cnt++;
+ m_MDS_LOG_DBG("MCM:API: Adest=%" PRIu64
+ " svc_cnt=%u", adest, adest_info->svc_cnt);
+ }
+ }
+
status = mds_get_subtn_res_tbl_by_adest(local_svc_hdl, svc_id, vdest_id,
adest, _subtn_result_info);
@@ -3571,6 +3597,24 @@ uint32_t mds_mcm_svc_down(PW_ENV_ID pwe_id, MDS_SVC_ID svc_id, V_DEST_RL role,
/* Discard : Getting down before getting up */
} else { /* Entry exist in subscription result table */
+ /* If adest exist and no sndrsp, start a timer */
+ MDS_ADEST_INFO *adest_info =
+ (MDS_ADEST_INFO *)ncs_patricia_tree_get(
+ _mds_mcm_cb->adest_list,
+ (uint8_t *));
+ if (adest_info) {
+ adest_info->svc_cnt--;
+ if (adest_info->svc_cnt == 0 &&
+ adest_info->sndrsp_cnt == 0) {
+ m_MDS_LOG_INFO("MCM:API: Adest=%" PRIu64
+ " down timer start", adest);
+ if (adest_info->tmr_start == false) {
+ adest_info->tmr_start = true;
+ start_mds_down_tmr(adest, svc_id);
+ }
+ }
+ }
+
if (vdest_id == m_VDEST_ID_FOR_ADEST_ENTRY) {
status = mds_subtn_res_tbl_del(
local_svc_hdl, svc_id, vdest_id, adest,
@@ -4956,6 +5000,17 @@ uint32_t mds_mcm_init(void)
return NCSCC_RC_FAILURE;
}
+ /* ADEST TREE */
+ memset(_tree_params, 0, sizeof(NCS_PATRICIA_PARAMS));
+ pat_tree_params.key_size = sizeof(MDS_DEST);
+ if (NCSCC_RC_SUCCESS !=
+ ncs_patricia_tree_init(_mds_mcm_cb->adest_list,
+ _tree_params)) {
+ m_MDS_LOG_ERR(
+ "MCM:API: patricia_tree_init: adest :failure, L
mds_mcm_init");
+ return NCSCC_RC_FAILURE;
+ }
+
/* SERVICE TREE */
memset(_tree_params, 0, sizeof(NCS_PATRICIA_PARAMS));
pat_tree_params.key_size = sizeof(MDS_SVC_HDL);
@@ -4966,7 +5021,12 @@ uint32_t mds_mcm_init(void)
if
Re: [devel] [PATCH 1/1] smf: Improve SmfAdminStateHandler() Return false if Fail [#3104]
Hi Phuc,
Instead of add more code, you can change default value to reduce code.
And releaseAdminOwnerOf() should be called base on "admset_rc" check, I
think.
Example:
--- a/src/smf/smfd/SmfAdminState.cc
+++ b/src/smf/smfd/SmfAdminState.cc
@@ -858,7 +858,7 @@ bool SmfAdminStateHandler::deleteNodeGroup() {
bool SmfAdminStateHandler::nodeGroupAdminOperation(
SaAmfAdminOperationIdT adminOp) {
- bool method_rc = true;
+ bool method_rc = false;
TRACE_ENTER();
@@ -920,20 +920,17 @@ bool SmfAdminStateHandler::nodeGroupAdminOperation(
} else if (imm_rc != SA_AIS_OK) {
LOG_NO("%s adminOpTimeout Fail %s", __FUNCTION__,
saf_error(imm_rc));
errno_ = imm_rc;
-method_rc = false;
} else {
LOG_NO("%s adminOpTimeout Fail %s", __FUNCTION__,
saf_error(oi_rc));
errno_ = oi_rc;
-method_rc = false;
}
}
} else {
LOG_NO("%s: becomeAdminOwnerOf(%s) Fail", __FUNCTION__,
nodeGroupName_s.c_str());
-method_rc = false;
}
- if (method_rc == true) {
+ if (admset_rc == true) {
TRACE("%s Admin operation is done. Release ownership if nodegroup",
__FUNCTION__);
if (releaseAdminOwnerOf(nodeGroupName_s) == false) {
Best Regards,
ThuanTr
-Original Message-
From: phuc.h.chau
Sent: Monday, October 21, 2019 6:23 PM
To: thuan.t...@dektech.com.au; thang.d.ngu...@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net; phuc.h.chau
Subject: [PATCH 1/1] smf: Improve SmfAdminStateHandler() Return false if
Fail [#3104]
SW upgrade testing, if found that if a service unit is in
INSTANTIATION_FAILED,
one_step upgrade will not continue with the software installation.
---
src/smf/smfd/SmfAdminState.cc | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/smf/smfd/SmfAdminState.cc b/src/smf/smfd/SmfAdminState.cc
index 076f9f0..a54f47f 100644
--- a/src/smf/smfd/SmfAdminState.cc
+++ b/src/smf/smfd/SmfAdminState.cc
@@ -900,11 +900,13 @@ bool SmfAdminStateHandler::nodeGroupAdminOperation(
LOG_NO(
"%s: saImmOmAdminOperationInvoke_2 Fail %s",
__FUNCTION__, saf_error(imm_rc));
+method_rc = false;
errno_ = imm_rc;
break;
} else if (oi_rc != SA_AIS_OK) {
LOG_NO("%s: SaAmfAdminOperationId %d Fail %s", __FUNCTION__,
adminOp,
saf_error(oi_rc));
+method_rc = false;
errno_ = oi_rc;
break;
} else {
--
2.7.4
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 1/1] base: add serial number arithmetic (RFC1982) [#3074]
- Adapt MDS with this SNA implementation.
---
src/base/Makefile.am | 6 +-
src/base/sna.h | 126 +++
src/base/tests/sna_test.cc | 117
src/mds/mds_tipc_fctrl_portid.cc | 45 ++-
src/mds/mds_tipc_fctrl_portid.h | 79 +++
5 files changed, 278 insertions(+), 95 deletions(-)
create mode 100644 src/base/sna.h
create mode 100644 src/base/tests/sna_test.cc
diff --git a/src/base/Makefile.am b/src/base/Makefile.am
index 025fb86a2..5082175cf 100644
--- a/src/base/Makefile.am
+++ b/src/base/Makefile.am
@@ -173,7 +173,8 @@ noinst_HEADERS += \
src/base/unix_client_socket.h \
src/base/unix_server_socket.h \
src/base/unix_socket.h \
- src/base/usrbuf.h
+ src/base/usrbuf.h \
+ src/base/sna.h
TESTS += bin/testleap bin/libbase_test bin/core_common_test
@@ -237,7 +238,8 @@ bin_libbase_test_SOURCES = \
src/base/tests/time_compare_test.cc \
src/base/tests/time_convert_test.cc \
src/base/tests/time_subtract_test.cc \
- src/base/tests/unix_socket_test.cc
+ src/base/tests/unix_socket_test.cc \
+ src/base/tests/sna_test.cc
bin_libbase_test_LDADD = \
$(GTEST_DIR)/lib/libgtest.la \
diff --git a/src/base/sna.h b/src/base/sna.h
new file mode 100644
index 0..b231fb134
--- /dev/null
+++ b/src/base/sna.h
@@ -0,0 +1,126 @@
+/* -*- OpenSAF -*-
+ *
+ * Copyright Ericsson AB 2019 - All Rights Reserved.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are licensed
+ * under the GNU Lesser General Public License Version 2.1, February 1999.
+ * The complete license can be accessed from the following location:
+ * http://opensource.org/licenses/lgpl-license.php
+ * See the Copying file included with the OpenSAF distribution for full
+ * licensing terms.
+ *
+ * Reference: Serial Number Arithmetic from RFC1982
+ *
+ */
+
+#ifndef BASE_SNA_H_
+#define BASE_SNA_H_
+
+#include
+#include
+
+#define MAX_16BITS 65536 // 2^16
+#define MAX_32BITS 4294967296 // 2^32
+
+template
+class _sna {
+ private:
+ T i;
+ uint64_t max() {
+if (typeid(T) == typeid(uint64_t)) {
+ return MAX_32BITS;
+} else if (typeid(T) == typeid(uint32_t)) {
+ return MAX_16BITS;
+} else {
+ printf("Type:%s\n", typeid(T).name());
+ throw std::out_of_range("Invalid type");
+}
+ }
+
+ public:
+ _sna(): i(0) {}
+ _sna(const _sna ) {
+i = t.i;
+ }
+ explicit _sna(const uint64_t ) {
+if ((n < 0) || (n > (max()-1)))
+ throw std::out_of_range("SNA assign with invalid value");
+i = n;
+ }
+ _sna& operator=(const _sna ) {
+// check for self-assignment
+if ( == this)
+ return *this;
+i = t.i;
+return *this;
+ }
+ T v() const {
+return i;
+ }
+ _sna& operator+=(const uint64_t& n) {
+if ((n < 0) || (n > (max()/2 - 1)))
+ throw std::out_of_range("SNA received invalid addition value");
+i = (i + n) % max();
+return *this;
+ }
+ friend _sna operator+(_sna m, const uint64_t& n) {
+m += n;
+return m;
+ }
+ // prefix ++
+ _sna& operator++() {
+*this += 1;
+return *this;
+ }
+ // postfix ++
+ _sna operator++(int) {
+_sna tmp(*this);
+operator++();
+return tmp;
+ }
+ bool operator==(const _sna& rhs) {
+return i == rhs.i;
+ }
+ bool operator==(const uint32_t val) {
+return i == val;
+ }
+ bool operator!=(const _sna& rhs) {
+return i != rhs.i;
+ }
+ bool operator<(const _sna& rhs) {
+return (i < rhs.i && rhs.i - i < max()/2) || \
+ (i > rhs.i && i - rhs.i > max()/2);
+ }
+ bool operator>=(const _sna& rhs) {
+return !(*this < rhs);
+ }
+ bool operator>(const _sna& rhs) {
+return (i < rhs.i && rhs.i - i > max()/2) || \
+ (i > rhs.i && i - rhs.i < max()/2);
+ }
+ bool operator<=(const _sna& rhs) {
+return !(*this > rhs);
+ }
+ int64_t operator-(const _sna& rhs) {
+if (*this >= rhs) {
+ if (i >= rhs.v()) {
+return i - rhs.v();
+ } else {
+return (i + max()) - rhs.v();
+ }
+} else {
+ if (i < rhs.v()) {
+return i - rhs.v();
+ } else {
+return i - (rhs.v() + max());
+ }
+}
+ }
+};
+
+typedef _sna sna16_t;
+typedef _sna sna32_t;
+
+#endif // BASE_SNA_H_
diff --git a/src/base/tests/sna_test.cc b/src/base/tests/sna_test.cc
new file mode 100644
index 0..4b7eb83e3
--- /dev/null
+++ b/src/base/tests/sna_test.cc
@@ -0,0 +1,117 @@
+/* -*- OpenSAF -*-
+ *
+ * Copyright Ericsson AB 2019 - All Rights Reserved.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. This file and program
[devel] [PATCH 0/1] Review Request for base: add serial number arithmetic (RFC1982) [#3074]
Summary: base: add serial number arithmetic (RFC1982) [#3074] Review request for Ticket(s): 3074 Peer Reviewer(s): Minh, Gary Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE *** Affected branch(es): develop Development branch: ticket-3074 Base revision: 9b1c588a4f244cb34a304b03c7568e04fa6cd8a7 Personal repository: git://git.code.sf.net/u/thuantr/review Impacted area Impact y/n Docsn Build systemn RPM/packaging n Configuration files n Startup scripts n SAF servicesy OpenSAF servicesn Core libraries n Samples n Tests n Other n Comments (indicate scope for each "y" above): - N/A revision a2a6c6c19d758b5652e0729f0ab839a3444fe09e Author: thuan.tran Date: Tue, 22 Oct 2019 09:29:01 +0700 base: add serial number arithmetic (RFC1982) [#3074] - Adapt MDS with this SNA implementation. Added Files: src/base/sna.h src/base/tests/sna_test.cc Complete diffstat: -- src/base/Makefile.am | 6 +- src/base/sna.h | 126 +++ src/base/tests/sna_test.cc | 117 src/mds/mds_tipc_fctrl_portid.cc | 45 +++--- src/mds/mds_tipc_fctrl_portid.h | 79 +++- 5 files changed, 278 insertions(+), 95 deletions(-) Testing Commands: - N/A Testing, Expected Results: -- N/A Conditions of Submission: - ACK by reviewers Arch Built StartedLinux distro --- mipsn n mips64 n n x86 n n x86_64 y y powerpc n n powerpc64 n n Reviewer Checklist: --- [Submitters: make sure that your review doesn't trigger any checkmarks!] Your checkin has not passed review because (see checked entries): ___ Your RR template is generally incomplete; it has too many blank entries that need proper data filled in. ___ You have failed to nominate the proper persons for review and push. ___ Your patches do not have proper short+long header ___ You have grammar/spelling in your header that is unacceptable. ___ You have exceeded a sensible line length in your headers/comments/text. ___ You have failed to put in a proper Trac Ticket # into your commits. ___ You have incorrectly put/left internal data in your comments/files (i.e. internal bug tracking tool IDs, product names etc) ___ You have not given any evidence of testing beyond basic build tests. Demonstrate some level of runtime or other sanity testing. ___ You have ^M present in some of your files. These have to be removed. ___ You have needlessly changed whitespace or added whitespace crimes like trailing spaces, or spaces before tabs. ___ You have mixed real technical changes with whitespace and other cosmetic code cleanup changes. These have to be separate commits. ___ You need to refactor your submission into logical chunks; there is too much content into a single commit. ___ You have extraneous garbage in your review (merge commits etc) ___ You have giant attachments which should never have been sent; Instead you should place your content in a public tree to be pulled. ___ You have too many commits attached to an e-mail; resend as threaded commits, or place in a public tree for a pull. ___ You have resent this content multiple times without a clear indication of what has changed between each re-send. ___ You have failed to adequately and individually address all of the comments and change requests that were proposed in the initial review. ___ You have a misconfigured ~/.gitconfig file (i.e. user.name, user.email etc) ___ Your computer have a badly configured date and time; confusing the the threaded patch review. ___ Your changes affect IPC mechanism, and you don't present any results for in-service upgradability test. ___ Your changes affect user manual and documentation, your patch series do not contain the patch that updates the Doxygen manual. ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 1/1] mds: not waste 1.5s in waiting Adest already down to send response message [#3102]
- When sending response message which Adest not exist (already down)
current MDS try to wait for 1.5 seconds before conclude no route to
send response message.
- There are 2 scenarios may have:
UP -> DOWN -> receive SNDRSP -> response timeout after 1.5s
UP -> receive SNDRSP -> DOWN -> response timeout after 1.5s
- With this change, MDS will not waste for 1.5s which can cause trouble
for higher layer services, e.g: ntf, imm, etc...
---
src/mds/mds_c_api.c | 70 -
src/mds/mds_c_sndrcv.c | 52 --
src/mds/mds_core.h | 25 +--
src/mds/mds_dt2c.h | 2 +-
src/mds/mds_dt_common.c | 22 -
5 files changed, 162 insertions(+), 9 deletions(-)
diff --git a/src/mds/mds_c_api.c b/src/mds/mds_c_api.c
index 132555b8e..5dd30c536 100644
--- a/src/mds/mds_c_api.c
+++ b/src/mds/mds_c_api.c
@@ -1900,6 +1900,32 @@ uint32_t mds_mcm_svc_up(PW_ENV_ID pwe_id, MDS_SVC_ID
svc_id, V_DEST_RL role,
/*** Validation for SCOPE **/
+ if (adest != m_MDS_GET_ADEST) {
+ MDS_ADEST_INFO *adest_info =
+ (MDS_ADEST_INFO *)ncs_patricia_tree_get(
+ _mds_mcm_cb->adest_list,
+ (uint8_t *));
+ if (!adest_info) {
+ /* Add adest to adest list */
+ adest_info = m_MMGR_ALLOC_ADEST_INFO;
+ memset(adest_info, 0, sizeof(MDS_ADEST_INFO));
+ adest_info->adest = adest;
+ adest_info->node.key_info =
+ (uint8_t *)_info->adest;
+ adest_info->svc_cnt = 1;
+ adest_info->tmr_start = false;
+ ncs_patricia_tree_add(
+ _mds_mcm_cb->adest_list,
+ (NCS_PATRICIA_NODE *)adest_info);
+ m_MDS_LOG_DBG("MCM:API: Adest=%" PRIu64
+ " svc_cnt=%u", adest, adest_info->svc_cnt);
+ } else {
+ adest_info->svc_cnt++;
+ m_MDS_LOG_DBG("MCM:API: Adest=%" PRIu64
+ " svc_cnt=%u", adest, adest_info->svc_cnt);
+ }
+ }
+
status = mds_get_subtn_res_tbl_by_adest(local_svc_hdl, svc_id, vdest_id,
adest, _subtn_result_info);
@@ -3571,6 +3597,24 @@ uint32_t mds_mcm_svc_down(PW_ENV_ID pwe_id, MDS_SVC_ID
svc_id, V_DEST_RL role,
/* Discard : Getting down before getting up */
} else { /* Entry exist in subscription result table */
+ /* If adest exist and no sndrsp, start a timer */
+ MDS_ADEST_INFO *adest_info =
+ (MDS_ADEST_INFO *)ncs_patricia_tree_get(
+ _mds_mcm_cb->adest_list,
+ (uint8_t *));
+ if (adest_info) {
+ adest_info->svc_cnt--;
+ if (adest_info->svc_cnt == 0 &&
+ adest_info->sndrsp_cnt == 0) {
+ m_MDS_LOG_INFO("MCM:API: Adest=%" PRIu64
+ " down timer start", adest);
+ if (adest_info->tmr_start == false) {
+ adest_info->tmr_start = true;
+ start_mds_down_tmr(adest, svc_id);
+ }
+ }
+ }
+
if (vdest_id == m_VDEST_ID_FOR_ADEST_ENTRY) {
status = mds_subtn_res_tbl_del(
local_svc_hdl, svc_id, vdest_id, adest,
@@ -4956,6 +5000,17 @@ uint32_t mds_mcm_init(void)
return NCSCC_RC_FAILURE;
}
+ /* ADEST TREE */
+ memset(_tree_params, 0, sizeof(NCS_PATRICIA_PARAMS));
+ pat_tree_params.key_size = sizeof(MDS_DEST);
+ if (NCSCC_RC_SUCCESS !=
+ ncs_patricia_tree_init(_mds_mcm_cb->adest_list,
+ _tree_params)) {
+ m_MDS_LOG_ERR(
+ "MCM:API: patricia_tree_init: adest :failure, L
mds_mcm_init");
+ return NCSCC_RC_FAILURE;
+ }
+
/* SERVICE TREE */
memset(_tree_params, 0, sizeof(NCS_PATRICIA_PARAMS));
pat_tree_params.key_size = sizeof(MDS_SVC_HDL);
@@ -4966,7 +5021,12 @@ uint32_t mds_mcm_init(void)
if (NCSCC_RC_SUCCESS !=
ncs_patricia_tree_destroy(_mds_mcm_cb->vdest_list)) {
m_MDS_LOG_ERR(
- "MCM:API: patricia_tree_destroy: service :failure,
L mds_mcm_init");
+ "MCM:API: patricia_tree_destroy: vdest :failure, L
mds_mcm_init");
+ }
+ if (NCSCC_RC_SUCCESS !=
+
[devel] [PATCH 0/1] Review Request for mds: not waste 1.5s in waiting Adest already down to send response message [#3102] V2
Summary: mds: not waste 1.5s in waiting Adest already down to send response message [#3102] Review request for Ticket(s): 3102 Peer Reviewer(s): Minh, Vu, Gary, Hans Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE *** Affected branch(es): develop Development branch: ticket-3102 Base revision: e668475b78f3e91db037607755c5c9d92842b445 Personal repository: git://git.code.sf.net/u/thuantr/review Impacted area Impact y/n Docsn Build systemn RPM/packaging n Configuration files n Startup scripts n SAF servicesn OpenSAF servicesn Core libraries y Samples n Tests n Other n NOTE: Patch(es) contain lines longer than 80 characers Comments (indicate scope for each "y" above): - N/A revision 38f10bb8bb5db407ac2c151df9eb439aab0dc6d8 Author: thuan.tran Date: Tue, 22 Oct 2019 07:05:56 +0700 mds: not waste 1.5s in waiting Adest already down to send response message [#3102] - When sending response message which Adest not exist (already down) current MDS try to wait for 1.5 seconds before conclude no route to send response message. - There are 2 scenarios may have: UP -> DOWN -> receive SNDRSP -> response timeout after 1.5s UP -> receive SNDRSP -> DOWN -> response timeout after 1.5s - With this change, MDS will not waste for 1.5s which can cause trouble for higher layer services, e.g: ntf, imm, etc... Complete diffstat: -- src/mds/mds_c_api.c | 70 - src/mds/mds_c_sndrcv.c | 52 +--- src/mds/mds_core.h | 25 +++--- src/mds/mds_dt2c.h | 2 +- src/mds/mds_dt_common.c | 22 +++- 5 files changed, 162 insertions(+), 9 deletions(-) Testing Commands: - N/A Testing, Expected Results: -- N/A Conditions of Submission: - ACK by reviewers Arch Built StartedLinux distro --- mipsn n mips64 n n x86 n n x86_64 y y powerpc n n powerpc64 n n Reviewer Checklist: --- [Submitters: make sure that your review doesn't trigger any checkmarks!] Your checkin has not passed review because (see checked entries): ___ Your RR template is generally incomplete; it has too many blank entries that need proper data filled in. ___ You have failed to nominate the proper persons for review and push. ___ Your patches do not have proper short+long header ___ You have grammar/spelling in your header that is unacceptable. ___ You have exceeded a sensible line length in your headers/comments/text. ___ You have failed to put in a proper Trac Ticket # into your commits. ___ You have incorrectly put/left internal data in your comments/files (i.e. internal bug tracking tool IDs, product names etc) ___ You have not given any evidence of testing beyond basic build tests. Demonstrate some level of runtime or other sanity testing. ___ You have ^M present in some of your files. These have to be removed. ___ You have needlessly changed whitespace or added whitespace crimes like trailing spaces, or spaces before tabs. ___ You have mixed real technical changes with whitespace and other cosmetic code cleanup changes. These have to be separate commits. ___ You need to refactor your submission into logical chunks; there is too much content into a single commit. ___ You have extraneous garbage in your review (merge commits etc) ___ You have giant attachments which should never have been sent; Instead you should place your content in a public tree to be pulled. ___ You have too many commits attached to an e-mail; resend as threaded commits, or place in a public tree for a pull. ___ You have resent this content multiple times without a clear indication of what has changed between each re-send. ___ You have failed to adequately and individually address all of the comments and change requests that were proposed in the initial review. ___ You have a misconfigured ~/.gitconfig file (i.e. user.name, user.email etc) ___ Your computer have a badly configured date and time; confusing the the threaded patch review. ___ Your changes affect IPC mechanism, and you don't present any results for in-service upgradability test. ___ Your changes affect user manual and documentation, your patch series do not contain the patch that updates the Doxygen manual. ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 2/2] msg: fully support SC absence [#3083]
It becomes trickier to handle timeout scenarios within the agent library
threads.
The application designer may not like asserts from OpenSAF library.
Returning an error code (SA_AIS_ERR_NO_RESOURCES) could be an option, but
the error code doesn't accurately reflects the situation if the timeout
occurred because of a transient situation even though the user an decide to
reInitialize the library at a later point of time.
As you mentioned, we should perhaps just revisit this topic if such a
timeout situation arises(at all)...
Ack,
Mathi.
On Tue, Oct 15, 2019 at 8:59 PM Jones, Alex wrote:
> Hi Mathi,
>
> See my comments inline.
>
> Alex
>
> On 10/15/19 12:45 PM, Mathi N P wrote:
>
> NOTICE: This email was received from an EXTERNAL sender
>
>
>
> Hi Alex,
>
> I only have a couple of things to discuss, the rest of the changes are
> fine.
>
> 1) Performing mqa_mds_reregister_queues() in a separate thread(inside the
> application process), iam not yet sure about
> synchronization issues this might throw with the MDS thread
> [Alex] I tried this originally, but this is currently on the mds thread.
> So, because we are using a sync call we can never get the return because we
> are calling this from the mds thread. That's why I created a separate
> thread. I did create a sync with msgnd, so that API calls can only continue
> when we know that msgnd has synced all of its queues with msgd.
> 2) In the below call, there is likelihood of the MDS send timing out
> (extreme case?). Should we handle the timeout error code from the MDS send?
> + /* send the request to the MQND */
> + uint32_t rc(mqa_mds_msg_sync_send(mqa_cb->mqa_mds_hdl,
> + _cb->mqnd_mds_dest,
> + _evt, _evt, MQSV_WAIT_TIME));
> +
> + mqa_cb = m_MQSV_MQA_RETRIEVE_MQA_CB;
> +
> + if (rc != NCSCC_RC_SUCCESS)
> [Alex] How should we handle a timeout? Retry a few times? Assert? I put
> the LOG_ER in there, so we can see if it ever happens, and deal with it at
> that point. I haven't seen this in my testing.
>
> Thanks,
> Mathi.
>
>
> On Thu, Oct 10, 2019 at 6:19 PM Jones, Alex ajo...@rbbn.com>> wrote:
> Fix whitespace issues
> ---
> src/msg/apitest/test_CapacityThresholds.cc | 2 +-
> src/msg/msgd/mqd_saf.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/msg/apitest/test_CapacityThresholds.cc
> b/src/msg/apitest/test_CapacityThresholds.cc
> index eb5dda1dc..971c8ae90 100644
> --- a/src/msg/apitest/test_CapacityThresholds.cc
> +++ b/src/msg/apitest/test_CapacityThresholds.cc
> @@ -50,7 +50,7 @@ static SaAisErrorT msgInitialize(SaMsgHandleT *msgHandle,
> SaVersionT *version)
> {
> SaAisErrorT rc(SA_AIS_OK);
> -
> +
> while (true) {
> rc = saMsgInitialize(msgHandle, 0, version);
>
> diff --git a/src/msg/msgd/mqd_saf.c b/src/msg/msgd/mqd_saf.c
> index 846755ba7..895a4852d 100644
> --- a/src/msg/msgd/mqd_saf.c
> +++ b/src/msg/msgd/mqd_saf.c
> @@ -64,7 +64,7 @@ static void get_q_groups_from_imm(MQD_CB *pMqd)
> SaVersionT version = { 'A', 2, 15 };
>
> error = immutil_saImmOmInitialize(, 0, );
> -
> +
> if (error != SA_AIS_OK) {
> LOG_ER("saImmOmInitialize failed %u", error);
> break;
> --
> 2.20.1
>
>
>
> Notice: This e-mail together with any attachments may contain information
> of Ribbon Communications Inc. that is confidential and/or proprietary for
> the sole use of the intended recipient. Any review, disclosure, reliance or
> distribution by others or forwarding without express permission is strictly
> prohibited. If you are not the intended recipient, please notify the sender
> immediately and then delete all copies, including any attachments.
>
>
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 1/1] smf: Improve SmfAdminStateHandler() Return false if Fail [#3104]
SW upgrade testing, if found that if a service unit is in INSTANTIATION_FAILED,
one_step upgrade will not continue with the software installation.
---
src/smf/smfd/SmfAdminState.cc | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/smf/smfd/SmfAdminState.cc b/src/smf/smfd/SmfAdminState.cc
index 076f9f0..a54f47f 100644
--- a/src/smf/smfd/SmfAdminState.cc
+++ b/src/smf/smfd/SmfAdminState.cc
@@ -900,11 +900,13 @@ bool SmfAdminStateHandler::nodeGroupAdminOperation(
LOG_NO(
"%s: saImmOmAdminOperationInvoke_2 Fail %s",
__FUNCTION__, saf_error(imm_rc));
+method_rc = false;
errno_ = imm_rc;
break;
} else if (oi_rc != SA_AIS_OK) {
LOG_NO("%s: SaAmfAdminOperationId %d Fail %s", __FUNCTION__, adminOp,
saf_error(oi_rc));
+method_rc = false;
errno_ = oi_rc;
break;
} else {
--
2.7.4
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 0/1] Review Request for smf: Improve SmfAdminStateHandler() Return false if Fail [#3104]
Summary: smf: Improve SmfAdminStateHandler() Return false if Fail [#3104] Review request for Ticket(s): 3104 Peer Reviewer(s): Thang, Thuan Pull request to: Thang Affected branch(es): develop Development branch: ticket-3104 Base revision: a6781e424c8963d03009e555bffbb323a4e290c4 Personal repository: git://git.code.sf.net/u/zchxphc/review Impacted area Impact y/n Docsn Build systemn RPM/packaging n Configuration files n Startup scripts n SAF servicesn OpenSAF servicesy Core libraries n Samples n Tests n Other n Comments (indicate scope for each "y" above): - *** EXPLAIN/COMMENT THE PATCH SERIES HERE *** revision 08707e9f71f4cdf48e33c3ba1413d517cebf2488 Author: phuc.h.chau Date: Mon, 21 Oct 2019 17:12:19 +0700 smf: Improve SmfAdminStateHandler() Return false if Fail [#3104] SW upgrade testing, if found that if a service unit is in INSTANTIATION_FAILED, one_step upgrade will not continue with the software installation. Complete diffstat: -- src/smf/smfd/SmfAdminState.cc | 2 ++ 1 file changed, 2 insertions(+) Testing Commands: - N/A Testing, Expected Results: -- N/A Conditions of Submission: - ACK from reviewers Arch Built StartedLinux distro --- mipsn n mips64 n n x86 n n x86_64 y y powerpc n n powerpc64 n n Reviewer Checklist: --- [Submitters: make sure that your review doesn't trigger any checkmarks!] Your checkin has not passed review because (see checked entries): ___ Your RR template is generally incomplete; it has too many blank entries that need proper data filled in. ___ You have failed to nominate the proper persons for review and push. ___ Your patches do not have proper short+long header ___ You have grammar/spelling in your header that is unacceptable. ___ You have exceeded a sensible line length in your headers/comments/text. ___ You have failed to put in a proper Trac Ticket # into your commits. ___ You have incorrectly put/left internal data in your comments/files (i.e. internal bug tracking tool IDs, product names etc) ___ You have not given any evidence of testing beyond basic build tests. Demonstrate some level of runtime or other sanity testing. ___ You have ^M present in some of your files. These have to be removed. ___ You have needlessly changed whitespace or added whitespace crimes like trailing spaces, or spaces before tabs. ___ You have mixed real technical changes with whitespace and other cosmetic code cleanup changes. These have to be separate commits. ___ You need to refactor your submission into logical chunks; there is too much content into a single commit. ___ You have extraneous garbage in your review (merge commits etc) ___ You have giant attachments which should never have been sent; Instead you should place your content in a public tree to be pulled. ___ You have too many commits attached to an e-mail; resend as threaded commits, or place in a public tree for a pull. ___ You have resent this content multiple times without a clear indication of what has changed between each re-send. ___ You have failed to adequately and individually address all of the comments and change requests that were proposed in the initial review. ___ You have a misconfigured ~/.gitconfig file (i.e. user.name, user.email etc) ___ Your computer have a badly configured date and time; confusing the the threaded patch review. ___ Your changes affect IPC mechanism, and you don't present any results for in-service upgradability test. ___ Your changes affect user manual and documentation, your patch series do not contain the patch that updates the Doxygen manual. ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 1/1] dtm: rotate logtraces on demand [#3086]
Hi,
Any comments on this patch? I will push it by this week if no comments.
Regards, Vu
On 10/4/19 5:30 PM, Vu Minh Nguyen wrote:
Introducing a new option '--rotate' to rotate given logtrace stream(s).
This patch also cleans the code of LogServer::ExecuteCommand().
---
src/base/log_writer.h | 2 +-
src/dtm/tools/osaflog.cc| 25 ++-
src/dtm/transport/log_server.cc | 125 +---
src/dtm/transport/log_server.h | 11 ++-
4 files changed, 115 insertions(+), 48 deletions(-)
diff --git a/src/base/log_writer.h b/src/base/log_writer.h
index 0a03e9253..ab2bf32ae 100644
--- a/src/base/log_writer.h
+++ b/src/base/log_writer.h
@@ -45,13 +45,13 @@ class LogWriter {
void Write(size_t size);
void Write(const char* bytes, size_t size);
void Flush();
+ void RotateLog();
void SetLogFile(const std::string& log_file) { log_file_ = log_file; }
private:
constexpr static const size_t kBufferSize = 128 * size_t{1024};
void Open();
void Close();
- void RotateLog();
std::string log_file(size_t backup) const;
diff --git a/src/dtm/tools/osaflog.cc b/src/dtm/tools/osaflog.cc
index 4e0956aa2..f6fa16801 100644
--- a/src/dtm/tools/osaflog.cc
+++ b/src/dtm/tools/osaflog.cc
@@ -54,6 +54,7 @@ base::UnixServerSocket* CreateSocket();
uint64_t Random64Bits(uint64_t seed);
bool PrettyPrint(const std::string& log_stream);
bool Delete(const std::string& log_stream);
+bool Rotate(const std::string& log_stream);
std::list OpenLogFiles(const std::string& log_stream);
std::string PathName(const std::string& log_stream, int suffix);
uint64_t GetInode(int fd);
@@ -70,6 +71,7 @@ int main(int argc, char** argv) {
{"flush", no_argument, 0, 'f'},
{"print", no_argument, nullptr, 'p'},
{"delete", no_argument, nullptr, 'd'},
+ {"rotate", no_argument, nullptr, 'r'},
{"extract-trace", required_argument, 0,
'e'},
{"max-idle", required_argument, 0, 'i'},
{0, 0, 0, 0}};
@@ -83,12 +85,14 @@ int main(int argc, char** argv) {
bool flush_result = true;
bool print_result = true;
bool delete_result = true;
+ bool rotate_result = true;
bool max_file_size_result = true;
bool number_of_backups_result = true;
bool max_idle_result = true;
bool flush_set = false;
bool pretty_print_set = false;
bool delete_set = false;
+ bool rotate_set = false;
bool max_file_size_set = false;
bool max_backups_set = false;
bool max_idle_set = false;
@@ -101,7 +105,7 @@ int main(int argc, char** argv) {
exit(EXIT_FAILURE);
}
- while ((option = getopt_long(argc, argv, "m:b:p:f:e:",
+ while ((option = getopt_long(argc, argv, "m:b:p:f:e:i:r",
long_options, _index)) != -1) {
switch (option) {
case 'p':
@@ -114,6 +118,9 @@ int main(int argc, char** argv) {
case 'f':
flush_set = true;
break;
+ case 'r':
+rotate_set = true;
+break;
case 'm':
max_file_size = base::StrToUint64(optarg,
_file_size_set);
@@ -164,12 +171,12 @@ int main(int argc, char** argv) {
if (thread_trace) exit(ExtractTrace(input_core, output_trace));
- if (argc > optind && !pretty_print_set && !delete_set) {
+ if (argc > optind && !pretty_print_set && !delete_set && !rotate_set) {
pretty_print_set = true;
flush_set = true;
}
- if ((argc <= optind && (pretty_print_set || delete_set)) ||
+ if ((argc <= optind && (pretty_print_set || delete_set || rotate_set)) ||
(pretty_print_set && delete_set)) {
PrintUsage(argv[0]);
exit(EXIT_FAILURE);
@@ -188,6 +195,11 @@ int main(int argc, char** argv) {
delete_result = Delete(argv[optind++]);
}
}
+ if (rotate_set == true) {
+while (rotate_result && optind < argc) {
+ rotate_result = Rotate(argv[optind++]);
+}
+ }
if (max_backups_set == true) {
number_of_backups_result = NoOfBackupFiles(max_backups);
}
@@ -197,7 +209,7 @@ int main(int argc, char** argv) {
if (max_idle_set == true) {
max_idle_result = SetMaxIdleTime(max_idle);
}
- if (flush_result && print_result && max_file_size_result &&
+ if (flush_result && print_result && max_file_size_result && rotate_result &&
delete_result && number_of_backups_result && max_idle_result)
exit(EXIT_SUCCESS);
exit(EXIT_FAILURE);
@@ -224,6 +236,7 @@ void PrintUsage(const char* program_name) {
"--delete Delete the specified LOGSTREAM(s) by\n"
" removing allocated resources in the log\n"
" server. Does not delete log files from
disk.\n"
+
