DAOS-16916 container: check inflight open #15682

wangdi1 · 2025-01-03T18:03:45Z

Check inflight container open, which might be stucked in IV fetch, then the following cont open will just increase the open count, then if the previous container open failed, it will get the assertion failure.

So let's retry if there are inflight container open.

Before requesting gatekeeper:

Two review approvals and any prior change requests have been resolved.
Testing is complete and all tests passed or there is a reason documented in the PR why it should be force landed and forced-landing tag is set.
Features: (or Test-tag*) commit pragma was used or there is a reason documented that there are no appropriate tags for this PR.
Commit messages follows the guidelines outlined here.
Any tests skipped by the ticket being addressed have been run and passed in the PR.

Gatekeeper:

Check inflight container open, which might be stucked in IV fetch, then the following cont open will just increase the open count, then if the previous container open failed, it will get the assertion failure. So let's retry if there are inflight container open. Signed-off-by: Di Wang <[email protected]>

github-actions · 2025-01-03T18:04:03Z

Ticket title is 'ds_cont_local_open() Assertion 'hdl->sch_cont->sc_open == 1' failed: Unexpected open count for cont c6822a9a: 307'
Status is 'Open'
https://daosio.atlassian.net/browse/DAOS-16916

daosbuild1 · 2025-01-04T00:47:44Z

Test stage Functional Hardware Medium completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15682/1/execution/node/1468/log

daosbuild1 · 2025-01-07T02:57:13Z

Test stage Functional Hardware Medium completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15682/2/execution/node/468/log

fix wording Signed-off-by: Di Wang <[email protected]>

further update Signed-off-by: Di Wang <[email protected]>

fix spelling Signed-off-by: Di Wang <[email protected]>

jxiong · 2025-01-10T17:19:57Z

src/include/daos_srv/container.h

+	    sc_dtx_delay_reset : 1, sc_dtx_registered : 1, sc_props_fetched : 1, sc_stopping : 1,
+	    sc_destroying : 1, sc_vos_agg_active : 1, sc_ec_agg_active : 1,
+	    /* flag of CONT_CAPA_READ_DATA/_WRITE_DATA disabled */
+	    sc_rw_disabled : 1, sc_scrubbing : 1, sc_rebuilding : 1, sc_open_initializing : 1;


Is this the recommended coding style?

this is actually only suppose to change 1 line, not sure why it messed up like this, which might due to git-hook sth.

This might be messed up by clang-format hooks.

yeah, clang-format does this. I believe if you use ; instead of comma, you still use same space and don't have the formatting issue

verified (though we do this in other places in DAOS)

#include <stdint.h> #include <stdio.h> struct foo { uint64_t foo; uint32_t bar:1; uint32_t fub:1; uint32_t other; }; int main(int argc, char ** argv) { printf("%zu\n", sizeof(struct foo)); }

This prints 16 which means bar and fub are placed together.

in fact, you can make this change without any formatting and clang-format will fix it and make it look nice

Nasf-Fan · 2025-01-14T08:49:45Z

src/container/srv_target.c

-	if (rc != 0)
+	if (rc != 0) {
+		if (rc == -DER_AGAIN)
+			goto retry;


Busy retry? It may be better to wait for former inflight opening to return and then check again.

this is a collective operation. which will yield, but I assume wait a few ms might be ok.

wait 50ms before retry. Signed-off-by: Di Wang <[email protected]>

jolivier23 · 2025-01-16T19:41:36Z

src/include/daos_srv/container.h

@@ -1,5 +1,5 @@
 /*
- * (C) Copyright 2015-2024 Intel Corporation.
+ * (C) Copyright 2015-2025 Intel Corporation.


This should be Google and no update to Intel

jolivier23 · 2025-01-16T19:41:49Z

src/container/srv_target.c

@@ -1,5 +1,5 @@
 /**
- * (C) Copyright 2016-2024 Intel Corporation.
+ * (C) Copyright 2016-2025 Intel Corporation.


Google but no update to Intel

jolivier23 · 2025-01-16T19:45:07Z

src/include/daos_srv/container.h

+	    sc_dtx_delay_reset : 1, sc_dtx_registered : 1, sc_props_fetched : 1, sc_stopping : 1,
+	    sc_destroying : 1, sc_vos_agg_active : 1, sc_ec_agg_active : 1,
+	    /* flag of CONT_CAPA_READ_DATA/_WRITE_DATA disabled */
+	    sc_rw_disabled : 1, sc_scrubbing : 1, sc_rebuilding : 1, sc_open_initializing : 1;


verified (though we do this in other places in DAOS)

#include <stdint.h> #include <stdio.h> struct foo { uint64_t foo; uint32_t bar:1; uint32_t fub:1; uint32_t other; }; int main(int argc, char ** argv) { printf("%zu\n", sizeof(struct foo)); }

This prints 16 which means bar and fub are placed together.

jolivier23 · 2025-01-16T19:46:05Z

src/include/daos_srv/container.h

+	    sc_dtx_delay_reset : 1, sc_dtx_registered : 1, sc_props_fetched : 1, sc_stopping : 1,
+	    sc_destroying : 1, sc_vos_agg_active : 1, sc_ec_agg_active : 1,
+	    /* flag of CONT_CAPA_READ_DATA/_WRITE_DATA disabled */
+	    sc_rw_disabled : 1, sc_scrubbing : 1, sc_rebuilding : 1, sc_open_initializing : 1;


in fact, you can make this change without any formatting and clang-format will fix it and make it look nice

Signed-off-by: Jeff Olivier <[email protected]>

NiuYawei

Unfortunately this PR has been landed, but looks there is a defect in the PR.

NiuYawei · 2025-01-17T03:45:32Z

src/container/srv_target.c

@@ -1663,6 +1678,7 @@ ds_cont_local_open(uuid_t pool_uuid, uuid_t cont_hdl_uuid, uuid_t cont_uuid,
 	dtx_cont_close(hdl->sch_cont, true);

 err_cont:
+	hdl->sch_cont->sc_open_initializing = 0;


This "sc_open_initializing" flag will be cleared when second open return -DER_AGAIN? Why don't we simply use a ABT_Mutex to serialize above open procedures?

Check inflight container open, which might be stucked in IV fetch, then the following cont open will just increase the open count, then if the previous container open failed, it will get the assertion failure. So let's retry if there are inflight container open. Signed-off-by: Di Wang <[email protected]>

wangdi1 requested review from a team as code owners January 3, 2025 18:03

wangdi1 requested review from wangshilong and Nasf-Fan and removed request for a team January 3, 2025 18:03

wangdi1 requested review from jolivier23 and liw January 3, 2025 18:04

wangdi1 added 2 commits January 8, 2025 04:10

DAOS-16916 container: fix word

6f14810

fix wording Signed-off-by: Di Wang <[email protected]>

DAOS-16916 container: further update

676dc46

further update Signed-off-by: Di Wang <[email protected]>

wangdi1 requested a review from liuxuezhao January 10, 2025 02:56

DAOS-16916 container: fix spelling

8eeafe7

fix spelling Signed-off-by: Di Wang <[email protected]>

jxiong reviewed Jan 10, 2025

View reviewed changes

Nasf-Fan reviewed Jan 14, 2025

View reviewed changes

DAOS-16916 container: wait 50ms before retry

b0dff0f

wait 50ms before retry. Signed-off-by: Di Wang <[email protected]>

wangdi1 requested review from Nasf-Fan and jxiong January 15, 2025 06:00

wangshilong previously approved these changes Jan 15, 2025

View reviewed changes

Nasf-Fan previously approved these changes Jan 15, 2025

View reviewed changes

wangdi1 requested a review from a team January 16, 2025 17:37

jolivier23 requested changes Jan 16, 2025

View reviewed changes

Update srv_target.c

d581ddf

Signed-off-by: Jeff Olivier <[email protected]>

jolivier23 dismissed stale reviews from Nasf-Fan and wangshilong via d581ddf January 16, 2025 22:09

Update container.h

d1bda9f

Signed-off-by: Jeff Olivier <[email protected]>

jolivier23 merged commit 15842c6 into master Jan 16, 2025
27 of 29 checks passed

jolivier23 deleted the wangdi/daos_16916 branch January 16, 2025 22:10

NiuYawei reviewed Jan 17, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DAOS-16916 container: check inflight open #15682

DAOS-16916 container: check inflight open #15682

wangdi1 commented Jan 3, 2025

github-actions bot commented Jan 3, 2025

daosbuild1 commented Jan 4, 2025

daosbuild1 commented Jan 7, 2025

jxiong Jan 10, 2025

wangdi1 Jan 15, 2025

wangshilong Jan 15, 2025

jolivier23 Jan 16, 2025

jolivier23 Jan 16, 2025

jolivier23 Jan 16, 2025

Nasf-Fan Jan 14, 2025

wangdi1 Jan 15, 2025

jolivier23 Jan 16, 2025

jolivier23 Jan 16, 2025

jolivier23 Jan 16, 2025

jolivier23 Jan 16, 2025

NiuYawei left a comment

NiuYawei Jan 17, 2025

DAOS-16916 container: check inflight open #15682

DAOS-16916 container: check inflight open #15682

Conversation

wangdi1 commented Jan 3, 2025

Before requesting gatekeeper:

Gatekeeper:

github-actions bot commented Jan 3, 2025

daosbuild1 commented Jan 4, 2025

daosbuild1 commented Jan 7, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NiuYawei left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment