DAOS-18487 rebuild: don't wait for discard by gnailzenh · Pull Request #17621 · daos-stack/daos

gnailzenh · 2026-02-28T10:22:31Z

pool_discard doesn't wait for completion of discard anymore
Make sure no concurrent discards

Steps for the author:

Commit message follows the guidelines.
Appropriate Features or Test-tag pragmas were used.
Appropriate Functional Test Stages were run.
At least two positive code reviews including at least one code owner from each category referenced in the PR.
Testing is complete. If necessary, forced-landing label added and a reason added in a comment.

After all prior steps are complete:

Gatekeeper requested (daos-gatekeeper added as a reviewer).

github-actions · 2026-02-28T10:22:51Z

Errors are Unable to load ticket data
https://daosio.atlassian.net/browse/DAOS-18487

daosbuild3 · 2026-02-28T10:32:48Z

Test stage Build on Leap 15.5 with Intel-C and TARGET_PREFIX completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-17621/1/execution/node/301/log

- pool_discard doesn't wait for completion of discard anymore - Make sure no concurrent discards Signed-off-by: Liang Zhen <gnailzenh@gmail.com>

daosbuild3 · 2026-02-28T12:04:58Z

Test stage Build on Leap 15.5 with Intel-C and TARGET_PREFIX completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-17621/2/execution/node/304/log

daosbuild3 · 2026-02-28T12:05:49Z

Test stage Build on EL 8.8 completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-17621/2/execution/node/320/log

daosbuild3 · 2026-02-28T12:05:51Z

Test stage Build on EL 9.6 completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-17621/2/execution/node/314/log

daosbuild3 · 2026-02-28T12:14:35Z

Test stage Build on Leap 15.5 completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-17621/2/execution/node/408/log

NiuYawei · 2026-02-28T13:25:25Z

src/pool/srv_target.c

+		D_INFO(DF_UUID " XXX: discard is already in progress, \n", DP_UUID(arg->pool_uuid));
+		ds_pool_put(pool);
+		D_GOTO(out, rc = -DER_BUSY);
+	}


Need to query if rebuild is already running as well.

NiuYawei · 2026-02-28T13:26:32Z

src/pool/srv_target.c

+	}
+
 	pool->sp_discard_status = 0;
 	rc = dss_ult_execute(ds_pool_tgt_discard_ult, arg, NULL, NULL, DSS_XS_SYS, 0, 0);


Changing this to dss_ult_create() looks more straightforward then creating a ULT in ds_pool_collective() callback. Or simply call ds_pool_collective() here instead of creating a new ULT.

I tended to do that but just wanted to minimize the change for now, using dss_pool_collective() requires change to dss_pool_collective to count ULTs being created, and add eventual too.
Let's just do this for validation, and I will clean it up if it can help

hmm, found another issue, might have to change it now.

The minimal change is to simply replace above dss_ult_execute() to dss_ult_create(), isn't it?

NiuYawei · 2026-02-28T13:39:04Z

src/pool/srv_target.c

 	struct d_backoff_seq	backoff_seq;
 	int			rc;

+	D_ASSERTF(!ds_pool_is_rebuilding(pool), DF_UUID " is already being reintegrated!\n",


It's not from this patch, but writing 'sp_rebuild_scan' from A xstream, but reading it from another xstream for barrier purpose looks not correct.

same as above, I would be very cautious about adding assertion, but this is for debugging & validation.

NiuYawei · 2026-02-28T13:40:28Z

src/pool/srv_target.c


-static int
+static void
 pool_child_discard(void *data)


It's better to add D_INFO messages before & after discard, so that we can tell if there is any unexpected discard in following tests.

Sure, will add.

liuxuezhao · 2026-02-28T14:38:08Z

src/pool/srv_target.c

 	}

-	pool->sp_need_discard = 1;
+	if (atomic_fetch_add(&pool->sp_need_discard, 1) > 1) {


atomic_fetch_add()
Atomically replaces the value pointed by obj with the result of addition of arg to the old value of obj, and returns the value obj held previously.
So should be if (atomic_fetch_add(, 1) > 0)?

hmm, you are right, let me fix it. There is no such call like test_and_set, otherwise I'd just use that.

liuxuezhao · 2026-02-28T14:42:31Z

src/pool/srv_target.c

+	 */
+	ds_pool_collective(arg->pool_uuid, ex_status, pool_child_discard_async, arg, 0, true);
+
+	ref = atomic_fetch_sub(&pool->sp_need_discard, 1);


atomicaly subtracts a value from an atomic object and returns the previous value
so should assert(ref >= 1)?

liuxuezhao · 2026-02-28T14:53:27Z

src/pool/srv_target.c

 	addr.pta_target = dss_get_module_info()->dmi_tgt_id;
 	if (!pool_target_addr_found(&arg->tgt_list, &addr)) {
 		D_DEBUG(DB_TRACE, "skip discard %u/%u.\n", addr.pta_rank,
 			addr.pta_target);


ds_pool_child_put

right, thanks

Signed-off-by: Liang Zhen <gnailzenh@gmail.com>

NiuYawei · 2026-03-01T06:48:55Z

src/pool/srv_target.c

+	}
+
 	pool->sp_discard_status = 0;
 	rc = dss_ult_execute(ds_pool_tgt_discard_ult, arg, NULL, NULL, DSS_XS_SYS, 0, 0);


The minimal change is to simply replace above dss_ult_execute() to dss_ult_create(), isn't it?

NiuYawei · 2026-03-02T01:43:29Z

src/pool/srv_target.c

+
+	/* XXX just return EAGAIN/EPERM? */
+	D_ASSERTF(!ds_pool_is_rebuilding(pool), DF_UUID " is already being reintegrated!\n",
+		  DP_UUID(arg->pool_uuid));


This assert could be triggered in current implementation. I think we'd just return error when rebuild is going.

Signed-off-by: Liang Zhen <gnailzenh@gmail.com>

daosbuild3 · 2026-03-02T09:19:12Z

Test stage Functional Hardware Medium MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-17621/4/execution/node/1352/log

daosbuild3 · 2026-03-02T10:39:34Z

Test stage NLT on EL 8.8 completed with status UNSTABLE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos//view/change-requests/job/PR-17621/5/testReport/

daosbuild3 · 2026-03-02T12:26:55Z

Test stage Test RPMs on EL 8.6 completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-17621/5/execution/node/1035/log

DAOS-18487 rebuild: don't wait for discard

df0f726

- pool_discard doesn't wait for completion of discard anymore - Make sure no concurrent discards Signed-off-by: Liang Zhen <gnailzenh@gmail.com>

gnailzenh force-pushed the liang/rebuild/b_discard_serialize branch from dd4a7f9 to df0f726 Compare February 28, 2026 11:54

NiuYawei reviewed Feb 28, 2026

View reviewed changes

liuxuezhao reviewed Feb 28, 2026

View reviewed changes

DAOS-18487 rebuild: code cleanup

02dd643

Signed-off-by: Liang Zhen <gnailzenh@gmail.com>

gnailzenh requested a review from wangshilong March 1, 2026 12:00

DAOS-18487 rebuild: move assert from ULT to RPC handler

e4e3545

Signed-off-by: Liang Zhen <gnailzenh@gmail.com>

NiuYawei reviewed Mar 2, 2026

View reviewed changes

DAOS-18487 rebuild: remove the assertion

33e12bf

Signed-off-by: Liang Zhen <gnailzenh@gmail.com>

Conversation

gnailzenh commented Feb 28, 2026

Steps for the author:

After all prior steps are complete:

Uh oh!

github-actions bot commented Feb 28, 2026

Uh oh!

daosbuild3 commented Feb 28, 2026

Uh oh!

daosbuild3 commented Feb 28, 2026

Uh oh!

daosbuild3 commented Feb 28, 2026

Uh oh!

daosbuild3 commented Feb 28, 2026

Uh oh!

daosbuild3 commented Feb 28, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daosbuild3 commented Mar 2, 2026

Uh oh!

daosbuild3 commented Mar 2, 2026

Uh oh!

daosbuild3 commented Mar 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants