Skip to content
This repository was archived by the owner on Dec 2, 2021. It is now read-only.

CaRT-788 : Added new API to allow for reset of fabric interfaces on PSM2 when Eager message problems happen due to client reboots.#344

Open
vschhabra wants to merge 13 commits intomasterfrom
vchhabra/psm2patch
Open

CaRT-788 : Added new API to allow for reset of fabric interfaces on PSM2 when Eager message problems happen due to client reboots.#344
vschhabra wants to merge 13 commits intomasterfrom
vchhabra/psm2patch

Conversation

@vschhabra
Copy link
Copy Markdown
Contributor

No description provided.

…SM2 when Eager message problems happen due to client reboots.

Signed-off-by: Vikram Chhabra <vikram.chhabra@intel.com>
Copy link
Copy Markdown
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style warning(s) for job https://build.hpdd.intel.com/job/daos-stack/job/cart/job/PR-344/1/
Please review https://wiki.hpdd.intel.com/display/DC/Coding+Rules

Note: Error annotation limited to the first 31 errors. Remaining unannotated errors:

src/cart/crt_hg.c:546:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:1116:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:46:
(style) do not initialise globals to 0
(style) spaces required around that '=' (ctx:VxV)

src/cart/crt_hg.c:50:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:51:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:55:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:56:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:57:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:59:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:60:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:62:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:68:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:69:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:1094:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:1095:
(style) trailing whitespace
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:1097:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:1098:
(style) line over 80 characters

src/cart/crt_hg.c:75:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:76:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:1101:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:78:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:79:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:80:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:81:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:82:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:1107:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:84:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:85:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:86:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:88:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:89:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:90:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:91:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:92:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:1117:
(style) trailing whitespace
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:94:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:95:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:1120:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:1121:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:98:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:1124:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:1125:
(style) line over 80 characters

src/cart/crt_hg.c:1127:
(style) line over 80 characters
(style) code indent should use tabs where possible
(style) please, no space before tabs
(style) type 'long unsigned int' should be specified in [[un]signed] [short|int|long|long long] order
(style) space prohibited before that close parenthesis ')'

src/cart/crt_hg.c:104:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:1129:
(style) trailing whitespace

src/cart/crt_hg.c:106:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:107:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:113:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:115:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:116:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:121:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:123:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:125:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:127:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:133:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:135:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:136:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:137:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:138:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:144:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:145:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:147:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:148:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:149:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:150:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:152:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:158:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:159:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:160:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:162:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:163:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:169:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:175:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:176:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:177:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:178:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:179:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:180:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:801:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:802:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:803:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:804:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:805:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:806:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:807:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:808:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:1110:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:1119:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:848:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:1103:
(style) else should follow close brace '}'

src/cart/crt_hg.c:1092:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:1093:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:1122:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:1105:
(style) line over 80 characters

src/cart/crt_hg.c:1128:
(style) line over 80 characters
(style) code indent should use tabs where possible
(style) please, no space before tabs
(style) type 'long unsigned int' should be specified in [[un]signed] [short|int|long|long long] order
(style) space required after that ',' (ctx:VxV)
(style) space prohibited before that close parenthesis ')'

src/cart/crt_hg.c:1106:
(style) line over 80 characters
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:1102:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:471:
(style) space prohibited between function name and open parenthesis '('

src/cart/crt_hg.c:473:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:475:
(style) code indent should use tabs where possible
(style) space prohibited before that close parenthesis ')'
(style) unnecessary whitespace before a quoted newline

src/cart/crt_hg.c:476:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:477:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:478:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:479:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:480:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:1104:
(style) trailing whitespace

src/cart/crt_hg.c:482:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:483:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:484:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:485:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:486:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:487:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:490:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:493:
(style) space prohibited between function name and open parenthesis '('

src/cart/crt_hg.c:495:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:497:
(style) please, no space before tabs

src/cart/crt_hg.c:498:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:500:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:501:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:502:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:503:
(style) line over 80 characters
(style) code indent should use tabs where possible

src/cart/crt_hg.c:504:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:505:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:507:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:508:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:510:
(style) code indent should use tabs where possible

Comment thread src/cart/crt_rpc.c Outdated
RPC_ADDREF(rpc_priv);

/* Insert the local job id in the rpc request. */
rpc_priv->crp_req_hdr.cch_clid = crt_gdata.cg_clid;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) code indent should use tabs where possible
(style) please, no space before tabs

Comment thread src/cart/crt_rpc.h Outdated
uint32_t cch_xid;
/* used in crp_reply_hdr to propagate rpc failure back to sender */
uint32_t cch_rc;
/* client id */
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) code indent should use tabs where possible

Comment thread src/cart/crt_rpc.h Outdated
/* used in crp_reply_hdr to propagate rpc failure back to sender */
uint32_t cch_rc;
/* client id */
uint64_t cch_clid;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) code indent should use tabs where possible

Comment thread src/include/cart/api.h Outdated
int crt_hg_remove_client_id (uint64_t client_id);

/* Test Function. */
int crt_hg_remove_all_client_ids (void);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) space prohibited between function name and open parenthesis '('

Comment thread src/include/cart/api.h Outdated
* return DER_SUCCESS on success, negative value on
* failure.
*/
int crt_hg_remove_client_id (uint64_t client_id);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) space prohibited between function name and open parenthesis '('

Comment thread src/cart/crt_hg.c Outdated
while (hg_addr_table_index > 0) {
ret = crt_hg_remove_client_id (hg_addr_table[--hg_addr_table_index]);
if (ret != HG_SUCCESS) {
D_ERROR("crt_remove_addr_all failed, hg_ret %d.\n", ret);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) line over 80 characters
(style) code indent should use tabs where possible
(style) please, no space before tabs

Comment thread src/cart/crt_hg.c Outdated
ret = crt_hg_remove_client_id (hg_addr_table[--hg_addr_table_index]);
if (ret != HG_SUCCESS) {
D_ERROR("crt_remove_addr_all failed, hg_ret %d.\n", ret);
D_GOTO(out, ret = -DER_HG);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) code indent should use tabs where possible
(style) please, no space before tabs

Comment thread src/cart/crt_hg.c Outdated
if (ret != HG_SUCCESS) {
D_ERROR("crt_remove_addr_all failed, hg_ret %d.\n", ret);
D_GOTO(out, ret = -DER_HG);
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) code indent should use tabs where possible
(style) please, no space before tabs

Comment thread src/cart/crt_hg.c Outdated
struct crt_rpc_priv rpc_tmp = {0};

d_list_t *halink;
struct crt_ha_mapping *ha;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) please, no space before tabs

Comment thread src/cart/crt_hg.c Outdated

d_list_t *halink;
struct crt_ha_mapping *ha;
struct ha_entry ha_entry;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) code indent should use tabs where possible
(style) please, no space before tabs

@daosbuild1
Copy link
Copy Markdown
Collaborator

@daosbuild1
Copy link
Copy Markdown
Collaborator

Test stage Build on Leap 15 with Intel-C completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/cart/view/change-requests/job/PR-344/1/execution/node/380/log

Comment thread src/cart/crt_hg.c Outdated
d_list_t *halink;
struct crt_ha_mapping *ha;
struct ha_entry ha_value;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the section from here on should be protected by some lock. its possible that client with same client_id connects as you are trying to destroy its entry, which can lead to race conditions

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment thread src/cart/crt_hg.c Outdated
{
hg_return_t ret = HG_SUCCESS;

#ifdef HASH_TEST
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is it conditional? when server shuts down it needs to destroy all tables, which would execute similar code.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thats some test code. I needed to have a copy of the client id's in the test application. Was using it for that purpose. Yes this may transfer into the shutdown case we had discussed.

Comment thread src/cart/crt_hg.c Outdated

/*Create the hg_addr hash table*/
rc = d_hash_table_create_inplace(D_HASH_FT_NOLOCK,
CRT_LOOKUP_CACHE_BITS,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be CRT_HG_ADDR_CACHE_LOOKUP_BITS

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment thread src/cart/crt_hg.c Outdated
D_DEBUG(DB_NET, "in crt_hg_init, listen address: %s.\n", *addr);
crt_gdata.cg_hg = hg_gdata;

/*Create the hg_addr hash table*/
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this section should also be only done if psm2

Comment thread src/cart/crt_hg.c Outdated
int
crt_hg_remove_client_id (uint64_t client_id)
{
hg_return_t ret = HG_SUCCESS;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there need to be checks here:

  1. if (!is_service()) return -- only supported on clients
  2. only for psm2

Comment thread src/cart/crt_hg.c Outdated
int
crt_hg_remove_client_id (uint64_t client_id)
{
hg_return_t ret = HG_SUCCESS;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be int, we shouldnt reuse hg_return_t for our error codes

Comment thread src/cart/crt_hg.c Outdated
int
crt_hg_remove_addr (hg_class_t *hg_class, hg_addr_t hg_addr)
{
hg_return_t ret = HG_SUCCESS;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we shouldnt reuse hg return codes for our own return codes

Comment thread src/cart/crt_hg.c Outdated
#endif

struct ha_entry {
hg_class_t *hg_class;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hg_class is associated with crt_context. As such we should keep the addr table per context and not store hg_class for each entry. In your current implementation (even if client id was unique), you would run into issues, since if client sends to server on address rank=0 tag=0 and on rank=0 tag=1, you will end up with 2 ha_entries for the same client id.

Storing this hash table per each context will save space.

Whenever hg_class is needed it can be derived from context in question from crt_ctx->cc_hg_ctx.chc.chc_hgcla

Comment thread src/cart/crt_hg.c Outdated
}
else {
ha_entry.hg_class = hg_info->hg_class;
HG_Addr_dup(hg_info->hg_class, hg_info->addr, &ha_entry.hg_addr);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line may not work correctly after the mercury nameserver patch. In mercury the receiver side does not have the client's fi_addr. I haven't found the specific code where mercury constructs hg_info before calling this crt_rpc_handler_common(). I haven't found a way to let the receiver side retrieve the client's fi_addr on the fi/mercury level. We probably can do it on the cart level.

@daosbuild1 daosbuild1 dismissed their stale review February 6, 2020 19:12

Updated patch

Copy link
Copy Markdown
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style warning(s) for job https://build.hpdd.intel.com/job/daos-stack/job/cart/job/PR-344/2/
Please review https://wiki.hpdd.intel.com/display/DC/Coding+Rules

Note: Error annotation limited to the first 31 errors. Remaining unannotated errors:

src/cart/crt_hg.c:1207:
(style) please, no space before tabs

src/cart/crt_hg.c:824:
(style) space prohibited before that close parenthesis ')'

src/cart/crt_hg.c:57:
(style) please, no space before tabs

src/cart/crt_hg.c:826:
(style) space prohibited before that close parenthesis ')'

src/cart/crt_hg.c:827:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:60:
(style) please, no space before tabs

src/cart/crt_hg.c:61:
(style) please, no space before tabs

src/cart/crt_hg.c:1214:
(style) please, no space before tabs

src/cart/crt_hg.c:63:
(style) please, no space before tabs

src/cart/crt_hg.c:1344:
(style) do not use C99 // comments

src/cart/crt_hg.c:1217:
(style) please, no space before tabs

src/cart/crt_hg.c:1218:
(style) please, no space before tabs

src/cart/crt_hg.c:1290:
(style) trailing whitespace

src/cart/crt_hg.c:845:
(style) space required after that ',' (ctx:VxV)

src/cart/crt_hg.c:1345:
(style) do not use C99 // comments

src/cart/crt_hg.c:56:
(style) please, no space before tabs

src/cart/crt_hg.c:853:
(style) space prohibited between function name and open parenthesis '('

src/cart/crt_hg.c:1337:
(style) line over 80 characters

src/cart/crt_hg.c:858:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:859:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:476:
(style) type 'long unsigned int' should be specified in [[un]signed] [short|int|long|long long] order
(style) space prohibited before that close parenthesis ')'

src/cart/crt_hg.c:478:
(style) type 'long unsigned int' should be specified in [[un]signed] [short|int|long|long long] order
(style) space prohibited before that close parenthesis ')'

src/cart/crt_hg.c:1317:
(style) else should follow close brace '}'

src/cart/crt_hg.c:1205:
(style) please, no space before tabs

src/cart/crt_hg.c:59:
(style) please, no space before tabs

src/cart/crt_hg.c:868:
(style) please, no space before tabs

src/cart/crt_hg.c:869:
(style) please, no space before tabs

src/cart/crt_hg.c:1255:
(style) trailing whitespace
(style) line over 80 characters

src/cart/crt_hg.c:872:
(style) please, no space before tabs

src/cart/crt_hg.c:1212:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:876:
(style) do not use C99 // comments

src/cart/crt_hg.c:877:
(style) do not use C99 // comments

src/cart/crt_hg.c:1262:
(style) line over 80 characters

src/cart/crt_hg.c:1213:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:500:
(style) please, no space before tabs

src/cart/crt_hg.c:885:
(style) do not use C99 // comments

src/cart/crt_hg.c:1308:
(style) line over 80 characters

src/cart/crt_hg.c:1271:
(style) line over 80 characters

src/cart/crt_hg.c:1144:
(style) line over 80 characters

src/cart/crt_hg.c:890:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:1275:
(style) else should follow close brace '}'

src/cart/crt_hg.c:1276:
(style) line over 80 characters

src/cart/crt_hg.c:1301:
(style) line over 80 characters

src/cart/crt_group.c:2713:
(style) trailing whitespace

Comment thread src/cart/crt_internal_types.h Outdated
/* in-flight endpoint tracking hash table */
struct d_hash_table cc_epi_table;
/* hg addr hash tables */
struct d_hash_table cc_ha_hash_table;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) please, no space before tabs

Comment thread src/cart/crt_internal_types.h Outdated
struct d_hash_table cc_epi_table;
/* hg addr hash tables */
struct d_hash_table cc_ha_hash_table;
struct d_hash_table cc_ha_server_hash_table;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) please, no space before tabs

Comment thread src/cart/crt_internal_types.h Outdated
struct d_hash_table cc_ha_hash_table;
struct d_hash_table cc_ha_server_hash_table;
/* Locks associated with above hg addr tables. */
pthread_rwlock_t cc_ha_hash_table_rwlock;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) please, no space before tabs

Comment thread src/cart/crt_internal_types.h Outdated
struct d_hash_table cc_ha_server_hash_table;
/* Locks associated with above hg addr tables. */
pthread_rwlock_t cc_ha_hash_table_rwlock;
pthread_rwlock_t cc_ha_server_hash_table_rwlock;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) please, no space before tabs

Comment thread src/crt_launch/crt_launch.c Outdated
char *p;
int len;
int rc;
crt_init_options_t opt = {0};
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) code indent should use tabs where possible

Comment thread src/cart/crt_hg.c Outdated
ha = crt_ha_link2ptr(halink);
d_list_for_each_entry(ha_list_entry, &ha->chm_list,
chl_list_link) {
if ((ha_list_entry->chl_entry.ha_class == hg_info->hg_class)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) line over 80 characters

Comment thread src/cart/crt_hg.c Outdated
const struct hg_info *hg_info;
struct crt_rpc_priv *rpc_priv;
crt_rpc_t *rpc_pub;
crt_opcode_t opc;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) please, no space before tabs

Comment thread src/cart/crt_hg.c Outdated
struct crt_rpc_priv *rpc_priv;
crt_rpc_t *rpc_pub;
crt_opcode_t opc;
crt_proc_t proc = NULL;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) please, no space before tabs

Comment thread src/cart/crt_hg.c Outdated

if (crt_gdata.cg_na_plugin == CRT_NA_OFI_PSM2) {
/*Create the hg_addr hash tables*/
rc = d_hash_table_create_inplace(D_HASH_FT_NOLOCK,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) code indent should use tabs where possible
(style) please, no space before tabs

Comment thread src/cart/crt_hg.c Outdated
crt_proc_t proc = NULL;
struct crt_opc_info *opc_info = NULL;
hg_return_t hg_ret = HG_SUCCESS;
bool is_coll_req = false;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) please, no space before tabs

Copy link
Copy Markdown
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style warning(s) for job https://build.hpdd.intel.com/job/daos-stack/job/cart/job/PR-344/2/
Please review https://wiki.hpdd.intel.com/display/DC/Coding+Rules

Note: Error annotation limited to the first 31 errors. Remaining unannotated errors:

src/cart/crt_hg.c:1207:
(style) please, no space before tabs

src/cart/crt_hg.c:824:
(style) space prohibited before that close parenthesis ')'

src/cart/crt_hg.c:57:
(style) please, no space before tabs

src/cart/crt_hg.c:826:
(style) space prohibited before that close parenthesis ')'

src/cart/crt_hg.c:827:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:60:
(style) please, no space before tabs

src/cart/crt_hg.c:61:
(style) please, no space before tabs

src/cart/crt_hg.c:1214:
(style) please, no space before tabs

src/cart/crt_hg.c:63:
(style) please, no space before tabs

src/cart/crt_hg.c:1344:
(style) do not use C99 // comments

src/cart/crt_hg.c:1217:
(style) please, no space before tabs

src/cart/crt_hg.c:1218:
(style) please, no space before tabs

src/cart/crt_hg.c:1290:
(style) trailing whitespace

src/cart/crt_hg.c:845:
(style) space required after that ',' (ctx:VxV)

src/cart/crt_hg.c:1345:
(style) do not use C99 // comments

src/cart/crt_hg.c:56:
(style) please, no space before tabs

src/cart/crt_hg.c:853:
(style) space prohibited between function name and open parenthesis '('

src/cart/crt_hg.c:1337:
(style) line over 80 characters

src/cart/crt_hg.c:858:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:859:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:476:
(style) type 'long unsigned int' should be specified in [[un]signed] [short|int|long|long long] order
(style) space prohibited before that close parenthesis ')'

src/cart/crt_hg.c:478:
(style) type 'long unsigned int' should be specified in [[un]signed] [short|int|long|long long] order
(style) space prohibited before that close parenthesis ')'

src/cart/crt_hg.c:1317:
(style) else should follow close brace '}'

src/cart/crt_hg.c:1205:
(style) please, no space before tabs

src/cart/crt_hg.c:59:
(style) please, no space before tabs

src/cart/crt_hg.c:868:
(style) please, no space before tabs

src/cart/crt_hg.c:869:
(style) please, no space before tabs

src/cart/crt_hg.c:1255:
(style) trailing whitespace
(style) line over 80 characters

src/cart/crt_hg.c:872:
(style) please, no space before tabs

src/cart/crt_hg.c:1212:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:876:
(style) do not use C99 // comments

src/cart/crt_hg.c:877:
(style) do not use C99 // comments

src/cart/crt_hg.c:1262:
(style) line over 80 characters

src/cart/crt_hg.c:1213:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:500:
(style) please, no space before tabs

src/cart/crt_hg.c:885:
(style) do not use C99 // comments

src/cart/crt_hg.c:1308:
(style) line over 80 characters

src/cart/crt_hg.c:1271:
(style) line over 80 characters

src/cart/crt_hg.c:1144:
(style) line over 80 characters

src/cart/crt_hg.c:890:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:1275:
(style) else should follow close brace '}'

src/cart/crt_hg.c:1276:
(style) line over 80 characters

src/cart/crt_hg.c:1301:
(style) line over 80 characters

src/cart/crt_group.c:2713:
(style) trailing whitespace

Comment thread src/cart/crt_internal_types.h Outdated
/* in-flight endpoint tracking hash table */
struct d_hash_table cc_epi_table;
/* hg addr hash tables */
struct d_hash_table cc_ha_hash_table;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) please, no space before tabs

Comment thread src/cart/crt_internal_types.h Outdated
struct d_hash_table cc_epi_table;
/* hg addr hash tables */
struct d_hash_table cc_ha_hash_table;
struct d_hash_table cc_ha_server_hash_table;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) please, no space before tabs

Comment thread src/cart/crt_internal_types.h Outdated
struct d_hash_table cc_ha_hash_table;
struct d_hash_table cc_ha_server_hash_table;
/* Locks associated with above hg addr tables. */
pthread_rwlock_t cc_ha_hash_table_rwlock;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) please, no space before tabs

Comment thread src/cart/crt_internal_types.h Outdated
struct d_hash_table cc_ha_server_hash_table;
/* Locks associated with above hg addr tables. */
pthread_rwlock_t cc_ha_hash_table_rwlock;
pthread_rwlock_t cc_ha_server_hash_table_rwlock;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) please, no space before tabs

Comment thread src/crt_launch/crt_launch.c Outdated
char *p;
int len;
int rc;
crt_init_options_t opt = {0};
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) code indent should use tabs where possible

Comment thread src/cart/crt_hg.c Outdated
ha = crt_ha_link2ptr(halink);
d_list_for_each_entry(ha_list_entry, &ha->chm_list,
chl_list_link) {
if ((ha_list_entry->chl_entry.ha_class == hg_info->hg_class)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) line over 80 characters

Comment thread src/cart/crt_hg.c Outdated
const struct hg_info *hg_info;
struct crt_rpc_priv *rpc_priv;
crt_rpc_t *rpc_pub;
crt_opcode_t opc;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) please, no space before tabs

Comment thread src/cart/crt_hg.c Outdated
struct crt_rpc_priv *rpc_priv;
crt_rpc_t *rpc_pub;
crt_opcode_t opc;
crt_proc_t proc = NULL;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) please, no space before tabs

Comment thread src/cart/crt_hg.c Outdated

if (crt_gdata.cg_na_plugin == CRT_NA_OFI_PSM2) {
/*Create the hg_addr hash tables*/
rc = d_hash_table_create_inplace(D_HASH_FT_NOLOCK,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) code indent should use tabs where possible
(style) please, no space before tabs

Comment thread src/cart/crt_hg.c Outdated
crt_proc_t proc = NULL;
struct crt_opc_info *opc_info = NULL;
hg_return_t hg_ret = HG_SUCCESS;
bool is_coll_req = false;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) please, no space before tabs

Copy link
Copy Markdown
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style warning(s) for job https://build.hpdd.intel.com/job/daos-stack/job/cart/job/PR-344/2/
Please review https://wiki.hpdd.intel.com/display/DC/Coding+Rules

Note: Error annotation limited to the first 0 errors. Remaining unannotated errors:

src/cart/crt_internal_types.h:171:
(style) please, no space before tabs

src/cart/crt_internal_types.h:172:
(style) please, no space before tabs

src/cart/crt_internal_types.h:174:
(style) please, no space before tabs

src/cart/crt_internal_types.h:175:
(style) please, no space before tabs

src/crt_launch/crt_launch.c:156:
(style) code indent should use tabs where possible

src/crt_launch/crt_launch.c:158:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:513:
(style) do not use C99 // comments

src/cart/crt_hg.c:899:
(style) line over 80 characters

src/cart/crt_hg.c:1284:
(style) line over 80 characters

src/cart/crt_hg.c:518:
(style) line over 80 characters

src/cart/crt_hg.c:1281:
(style) trailing whitespace

src/cart/crt_hg.c:904:
(style) line over 80 characters

src/cart/crt_hg.c:1132:
(style) space required before the open brace '{'

src/cart/crt_hg.c:906:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:525:
(style) line over 80 characters

src/cart/crt_hg.c:529:
(style) line over 80 characters

src/cart/crt_hg.c:532:
(style) line over 80 characters

src/cart/crt_hg.c:1211:
(style) please, no space before tabs

src/cart/crt_hg.c:1302:
(style) line over 80 characters

src/cart/crt_hg.c:1303:
(style) line over 80 characters

src/cart/crt_hg.c:540:
(style) line over 80 characters

src/cart/crt_hg.c:1312:
(style) line over 80 characters

src/cart/crt_hg.c:902:
(style) line over 80 characters
(style) space prohibited between function name and open parenthesis '('

src/cart/crt_hg.c:1208:
(style) please, no space before tabs

src/cart/crt_hg.c:1289:
(style) trailing whitespace

src/cart/crt_hg.c:1327:
(style) line over 80 characters

src/cart/crt_hg.c:1288:
(style) line over 80 characters

src/cart/crt_hg.c:1202:
(style) please, no space before tabs

src/cart/crt_hg.c:1203:
(style) please, no space before tabs

src/cart/crt_hg.c:990:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:1206:
(style) please, no space before tabs

src/cart/crt_hg.c:1207:
(style) please, no space before tabs

src/cart/crt_hg.c:824:
(style) space prohibited before that close parenthesis ')'

src/cart/crt_hg.c:57:
(style) please, no space before tabs

src/cart/crt_hg.c:826:
(style) space prohibited before that close parenthesis ')'

src/cart/crt_hg.c:827:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:60:
(style) please, no space before tabs

src/cart/crt_hg.c:61:
(style) please, no space before tabs

src/cart/crt_hg.c:1214:
(style) please, no space before tabs

src/cart/crt_hg.c:63:
(style) please, no space before tabs

src/cart/crt_hg.c:1344:
(style) do not use C99 // comments

src/cart/crt_hg.c:1217:
(style) please, no space before tabs

src/cart/crt_hg.c:1218:
(style) please, no space before tabs

src/cart/crt_hg.c:1290:
(style) trailing whitespace

src/cart/crt_hg.c:845:
(style) space required after that ',' (ctx:VxV)

src/cart/crt_hg.c:1345:
(style) do not use C99 // comments

src/cart/crt_hg.c:56:
(style) please, no space before tabs

src/cart/crt_hg.c:853:
(style) space prohibited between function name and open parenthesis '('

src/cart/crt_hg.c:1337:
(style) line over 80 characters

src/cart/crt_hg.c:858:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:859:
(style) code indent should use tabs where possible

src/cart/crt_hg.c:476:
(style) type 'long unsigned int' should be specified in [[un]signed] [short|int|long|long long] order
(style) space prohibited before that close parenthesis ')'

src/cart/crt_hg.c:478:
(style) type 'long unsigned int' should be specified in [[un]signed] [short|int|long|long long] order
(style) space prohibited before that close parenthesis ')'

src/cart/crt_hg.c:1317:
(style) else should follow close brace '}'

src/cart/crt_hg.c:1205:
(style) please, no space before tabs

src/cart/crt_hg.c:59:
(style) please, no space before tabs

src/cart/crt_hg.c:868:
(style) please, no space before tabs

src/cart/crt_hg.c:869:
(style) please, no space before tabs

src/cart/crt_hg.c:1255:
(style) trailing whitespace
(style) line over 80 characters

src/cart/crt_hg.c:872:
(style) please, no space before tabs

src/cart/crt_hg.c:1212:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:876:
(style) do not use C99 // comments

src/cart/crt_hg.c:877:
(style) do not use C99 // comments

src/cart/crt_hg.c:1262:
(style) line over 80 characters

src/cart/crt_hg.c:1213:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:500:
(style) please, no space before tabs

src/cart/crt_hg.c:885:
(style) do not use C99 // comments

src/cart/crt_hg.c:1308:
(style) line over 80 characters

src/cart/crt_hg.c:1271:
(style) line over 80 characters

src/cart/crt_hg.c:1144:
(style) line over 80 characters

src/cart/crt_hg.c:890:
(style) code indent should use tabs where possible
(style) please, no space before tabs

src/cart/crt_hg.c:1275:
(style) else should follow close brace '}'

src/cart/crt_hg.c:1276:
(style) line over 80 characters

src/cart/crt_hg.c:1301:
(style) line over 80 characters

src/cart/crt_group.c:2713:
(style) trailing whitespace

Note: Unable to provide any annotated comments due to GitHub API limitations.

@daosbuild1
Copy link
Copy Markdown
Collaborator

@daosbuild1
Copy link
Copy Markdown
Collaborator

Test stage Build RPM on Leap 15 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/cart/view/change-requests/job/PR-344/2/execution/node/274/log

@daosbuild1
Copy link
Copy Markdown
Collaborator

Test stage Build RPM on CentOS 7 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/cart/view/change-requests/job/PR-344/2/execution/node/272/log

@daosbuild1
Copy link
Copy Markdown
Collaborator

Test stage Build on Ubuntu 18.04 with Clang completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/cart/view/change-requests/job/PR-344/2/execution/node/269/log

@daosbuild1
Copy link
Copy Markdown
Collaborator

Test stage Build on CentOS 7 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/cart/view/change-requests/job/PR-344/2/execution/node/231/log

@daosbuild1
Copy link
Copy Markdown
Collaborator

Test stage Build on CentOS 7 with Clang completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/cart/view/change-requests/job/PR-344/2/execution/node/268/log

@daosbuild1
Copy link
Copy Markdown
Collaborator

Test stage Build on Leap 15 with Intel-C completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/cart/view/change-requests/job/PR-344/2/execution/node/273/log

Signed-off-by: Vikram Chhabra <vikram.chhabra@intel.com>
@daosbuild1 daosbuild1 dismissed stale reviews from themself February 6, 2020 20:02

Updated patch

Copy link
Copy Markdown
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread src/cart/crt_internal_types.h Outdated
struct d_hash_table cc_ha_hash_table;
struct d_hash_table cc_ha_server_hash_table;
/* Locks associated with above hg addr tables. */
pthread_rwlock_t cc_ha_hash_table_rwlock;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) please, no space before tabs

Comment thread src/cart/crt_internal_types.h Outdated
struct d_hash_table cc_ha_server_hash_table;
/* Locks associated with above hg addr tables. */
pthread_rwlock_t cc_ha_hash_table_rwlock;
pthread_rwlock_t cc_ha_server_hash_table_rwlock;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) please, no space before tabs

Comment thread src/cart/crt_hg.c Outdated

if (crt_gdata.cg_na_plugin == CRT_NA_OFI_PSM2) {
/*Create the hg_addr hash tables*/
rc = d_hash_table_create_inplace(D_HASH_FT_NOLOCK,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) code indent should use tabs where possible
(style) please, no space before tabs

Comment thread src/cart/crt_hg.c Outdated
if (halink != NULL) {
/*Traverse the list and add new entry to tail
*if not found.
*/
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) code indent should use tabs where possible
(style) please, no space before tabs

Comment thread src/cart/crt_hg.c Outdated
!= NULL) {
ha_value = entry->chl_entry;
D_FREE(entry);
rc = crt_hg_remove_addr (ha_value.ha_class,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) space prohibited between function name and open parenthesis '('

Comment thread src/cart/crt_hg.c Outdated
int ret = 0;
d_list_t *halink;
struct crt_ha_mapping *ha;
struct ha_entry ha_value;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) please, no space before tabs

Comment thread src/cart/crt_hg.c Outdated
halink = d_hash_rec_find(&crt_ctx->cc_ha_server_hash_table,
(void *)&client_id, sizeof(client_id));
if (!halink) {
D_ERROR("client=%" PRIu64 "not in server hash table\n",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) code indent should use tabs where possible
(style) please, no space before tabs

Comment thread src/cart/crt_hg.c Outdated
if (rc != HG_SUCCESS) {
D_ERROR("crt_remove_addr failed, hg_ret %d.\n", rc);
}
D_FREE(entry);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) code indent should use tabs where possible

Comment thread src/cart/crt_hg.c Outdated
D_ERROR("crt_remove_addr failed, hg_ret %d.\n", rc);
}
D_FREE(entry);
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) code indent should use tabs where possible

Comment thread src/cart/crt_group.c Outdated
d_hash_rec_delete(&grp_priv->gp_uri_lookup_cache,
&rank, sizeof(d_rank_t));
/*If PSM2 then remove the rank from the hash table. */
if (crt_gdata.cg_na_plugin == CRT_NA_OFI_PSM2)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) trailing whitespace

@daosbuild1
Copy link
Copy Markdown
Collaborator

@daosbuild1
Copy link
Copy Markdown
Collaborator

Test stage Build RPM on Leap 15 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/cart/view/change-requests/job/PR-344/3/execution/node/273/log

@daosbuild1 daosbuild1 dismissed their stale review February 6, 2020 20:32

Updated patch

Copy link
Copy Markdown
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread src/cart/crt_hg.c Outdated
!= NULL) {
ha_value = entry->chl_entry;
D_FREE(entry);
rc = crt_hg_remove_addr (ha_value.ha_class,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) space prohibited between function name and open parenthesis '('

Comment thread src/cart/crt_hg.c Outdated
sizeof(client_id));
if (halink != NULL) {
/*Traverse the list and add new entry to tail
*if not found.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(style) trailing whitespace

@daosbuild1
Copy link
Copy Markdown
Collaborator

@daosbuild1
Copy link
Copy Markdown
Collaborator

Test stage Build RPM on Leap 15 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/cart/view/change-requests/job/PR-344/4/execution/node/272/log

@daosbuild1
Copy link
Copy Markdown
Collaborator

Test stage Build RPM on CentOS 7 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/cart/view/change-requests/job/PR-344/4/execution/node/271/log

@daosbuild1
Copy link
Copy Markdown
Collaborator

Test stage Build on CentOS 7 with Clang completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/cart/view/change-requests/job/PR-344/4/execution/node/270/log

@daosbuild1
Copy link
Copy Markdown
Collaborator

Test stage Build on CentOS 7 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/cart/view/change-requests/job/PR-344/4/execution/node/244/log

@daosbuild1
Copy link
Copy Markdown
Collaborator

Test stage Build on Ubuntu 18.04 with Clang completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/cart/view/change-requests/job/PR-344/4/execution/node/274/log

@daosbuild1
Copy link
Copy Markdown
Collaborator

Test stage Build on Leap 15 with Intel-C completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/cart/view/change-requests/job/PR-344/4/execution/node/273/log

Signed-off-by: Vikram Chhabra <vikram.chhabra@intel.com>
@daosbuild1 daosbuild1 dismissed their stale review February 6, 2020 20:50

Updated patch

@daosbuild1
Copy link
Copy Markdown
Collaborator

Test stage Build RPM on Leap 15 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/cart/view/change-requests/job/PR-344/5/execution/node/272/log

@daosbuild1
Copy link
Copy Markdown
Collaborator

Test stage Build RPM on CentOS 7 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/cart/view/change-requests/job/PR-344/5/execution/node/246/log

@daosbuild1
Copy link
Copy Markdown
Collaborator

Test stage Build on CentOS 7 with Clang completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/cart/view/change-requests/job/PR-344/5/execution/node/270/log

@daosbuild1
Copy link
Copy Markdown
Collaborator

Test stage Build on CentOS 7 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/cart/view/change-requests/job/PR-344/5/execution/node/273/log

@daosbuild1
Copy link
Copy Markdown
Collaborator

Test stage Build on Ubuntu 18.04 with Clang completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/cart/view/change-requests/job/PR-344/5/execution/node/271/log

@daosbuild1
Copy link
Copy Markdown
Collaborator

Test stage Build on Leap 15 with Intel-C completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/cart/view/change-requests/job/PR-344/5/execution/node/274/log

Copy link
Copy Markdown
Contributor

@frostedcmos frostedcmos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

first round of comments. General comment - make sure to update copyright dates on all changed files if they arent 2020 already

Comment thread src/include/cart/api.h Outdated
* Remove a client id from the Server hash table of Hg and Fabric Interface
* addresses.
*
* param[in] client_id System wide unique client id
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs to be \param per doxygen format

Comment thread src/include/cart/api.h Outdated
*
* param[in] client_id System wide unique client id
*
* return DER_SUCCESS on success, negative value on
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs to be \return

Comment thread src/include/cart/api.h Outdated

/**
* Remove a client id from the Server hash table of Hg and Fabric Interface
* addresses.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to not get into details of what we do, so remove references to hg/itnerface here.

Just make it something like crt_cleanup_client_id(). In notes to this api you can mention that for now it is only to be called when psm2 is used and client exists abruptly

Comment thread src/cart/crt_hg.c Outdated
Comment on lines +475 to +478
D_DEBUG(DB_TRACE, "removing hg addr %" PRIu64 "\n",
(uint64_t)hg_addr);
D_DEBUG(DB_TRACE, "removing hg class %" PRIu64 "\n",
(uint64_t)hg_class);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better to combine these 2 into single log line. on busy systems this can end up being printed far apart with multiple threads logging stuff, so we might lose track of what things go together

Comment thread src/cart/crt_hg.c Outdated
(uint64_t)hg_class);
ret = HG_Addr_set_remove(hg_class, hg_addr);
if (ret != HG_SUCCESS) {
D_ERROR("HG_Addr_set_remove) failed, hg_ret %d.\n", ret);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing ( in text message

Comment thread src/cart/crt_hg.c
rc = crt_hg_remove_addr(ha_value.ha_class, ha_value.ha_addr);

if (rc != HG_SUCCESS) {
D_ERROR("crt_remove_addr failed, hg_ret %d.\n", rc);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as before - it would be helpful to print ha_class/addr/client id here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its being done in remove_addr call. Would be redundant here?

Comment thread src/cart/crt_hg.c
return rc;
}

int
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it probably would help to add comment here to explain that this function is for server hash tables only

Comment thread src/cart/crt_hg.c Outdated
if (!halink) {
D_ERROR("client=%" PRIu64 "not in server hash table\n",
client_id);
D_GOTO(out, rc = -DER_NONEXIST);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to unlock all locks here before you exit, or do this at 'out/cleanup' lable

Comment thread src/cart/crt_hg.c
D_RWLOCK_WRLOCK(&crt_ctx->cc_ha_server_hash_table_rwlock);
halink = d_hash_rec_find(&crt_ctx->cc_ha_server_hash_table,
(void *)&client_id, sizeof(client_id));
if (!halink) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this an error though? we can have entry in one context table but not in others. if this doesnt find in first table it will skip all other context tables.

The scenario where this can happen:
servers have multiple contexts
server0 sends rpc to server1 on tag=3
this will result in server1's context[3] table to be populated with server0's address/info, but no other context table will be

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i will just log the error and do a continue;

Comment thread src/cart/crt_hg.c
rc = crt_hg_remove_addr(ha_value.ha_class,
ha_value.ha_addr);
if (rc != HG_SUCCESS) {
D_ERROR("crt_remove_addr failed, hg_ret %d.\n",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as before, helpful to print ha_class/addr/cli_id here for later debug

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its being done inside hg_remove_addr. Would be redundant?

Signed-off-by: Vikram Chhabra <vikram.chhabra@intel.com>
Conflicts:
	src/include/cart/api.h
Signed-off-by: Vikram Chhabra <vikram.chhabra@intel.com>
@daosbuild1
Copy link
Copy Markdown
Collaborator

Test stage Build RPM on Leap 15 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/cart/view/change-requests/job/PR-344/6/execution/node/257/log

@daosbuild1
Copy link
Copy Markdown
Collaborator

Test stage Build RPM on CentOS 7 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/cart/view/change-requests/job/PR-344/6/execution/node/256/log

@daosbuild1
Copy link
Copy Markdown
Collaborator

Test stage Build on Ubuntu 18.04 with Clang completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/cart/view/change-requests/job/PR-344/6/execution/node/273/log

Signed-off-by: Vikram Chhabra <vikram.chhabra@intel.com>
@daosbuild1
Copy link
Copy Markdown
Collaborator

Test stage Build RPM on Leap 15 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/cart/view/change-requests/job/PR-344/7/execution/node/272/log

@daosbuild1
Copy link
Copy Markdown
Collaborator

Test stage Build RPM on CentOS 7 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/cart/view/change-requests/job/PR-344/7/execution/node/274/log

Signed-off-by: Vikram Chhabra <vikram.chhabra@intel.com>
@daosbuild1
Copy link
Copy Markdown
Collaborator

Test stage Build RPM on Leap 15 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/cart/view/change-requests/job/PR-344/8/execution/node/274/log

@daosbuild1
Copy link
Copy Markdown
Collaborator

Test stage Build RPM on CentOS 7 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/cart/view/change-requests/job/PR-344/8/execution/node/271/log

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants