DAOS-17935 control: Remove spdk lockfiles before and after local scan by tanabarr · Pull Request #16769 · daos-stack/daos

tanabarr · 2025-08-26T20:46:15Z

Remove SPDK device lockfiles before and after ControlService performs
a scan on engine-assigned bdevs when an engine is not running. This
removes SPDK lockfiles for a device only when the engine has not
claimed that device.

Features: control

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 · 2025-08-26T20:46:29Z

Ticket title is 'SPDK lockfiles need to be removed in storage format and scan'
Status is 'In Review'
https://daosio.atlassian.net/browse/DAOS-17935

ryon-jensen

lgtm

daosbuild3 · 2025-08-27T09:18:32Z

Test stage Functional 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-16769/1/testReport/

liw · 2025-08-29T08:09:26Z

+		if err := cs.cleanSpdkResources(); err != nil {
+			return nil, errors.Wrap(err, "clean spdk resources")
+		}
+


@NiuYawei, do you know if this is safe? I think for StorageFormat it's okay, but my knowledge is insufficient in the StorageScan case.

In response to the question:

Does the clean up call promise to keep data intact?

On format, lockfiles for engine should be removed before and after because the engine in question has not been started yet. On scan, lockfiles should be removed before and after if the relevant engine(s) have not started (this requires a change to the current PR, thanks for prompting). If during scan the engine has already started then lockfiles should not be touched as the scan goes through dRPC to the engine. The orphan-hugepage part of cleanup doesn't affect hugepages owned by an existing process. So if the above is considered the call should promise to keep data intact.

Thanks, sounds fine to me then.

liw · 2025-08-29T08:30:41Z

+		if err := cs.cleanSpdkResources(); err != nil {
+			return nil, errors.Wrap(err, "clean spdk resources")
+		}
+


Thanks, sounds fine to me then.

daosbuild3 · 2025-08-29T10:38:34Z

Test stage Functional 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-16769/2/execution/node/477/log

daosbuild3 · 2025-08-30T14:41:30Z

Test stage Unit Test with memcheck on EL 8.8 completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos/job/PR-16769/5/display/redirect

daosbuild3 · 2025-08-30T20:08:16Z

Test stage Unit Test bdev with memcheck on EL 8.8 completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos/job/PR-16769/6/display/redirect

daosbuild3 · 2025-08-30T20:41:03Z

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-16769/6/testReport/

daosbuild3 · 2025-08-31T10:47:51Z

Test stage Functional 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-16769/7/testReport/

Features: control Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com>

kjacque

Looks reasonable to me. We have some unit tests related to cleaning up lockfiles, don't we? It would be good to verify this logic works as expected.

tanabarr · 2025-09-04T12:29:13Z

Looks reasonable to me. We have some unit tests related to cleaning up lockfiles, don't we? It would be good to verify this logic works as expected.

The clean methods are already unit tested. Adding these calls shouldn't necessitate new unit tests IMO but if you insist I will add them.

kjacque

The main area that could benefit from a new test case is the conditional in spdkBackend.Scan. If you've verified this manually, though, I can approve it as-is. IMO any change in logic that produces some kind of verifiable behavior change is interesting to unit test, whenever that's possible anyway.

…dk-clean-on-scanfmt Features: control Allow-unstable-test: true Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com>

tanabarr · 2025-09-06T09:13:42Z

@kjacque the unit tests weren't trivial, there is some problem in passing call stats from the MockNvmeImpl up to the scan call level that I have to figure out, I would like to land this and then figure that out as a subsequent change. The strange thing is that it works for the MockEnvImpl!

daosbuild3 · 2025-09-06T09:59:50Z

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-16769/12/testReport/

daosbuild3 · 2025-09-07T17:23:43Z

Test stage Functional Hardware Large MD on SSD completed with status UNSTABLE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos//view/change-requests/job/PR-16769/13/testReport/

daosbuild3 · 2025-09-08T22:59:25Z

Test stage Functional Hardware Large MD on SSD completed with status UNSTABLE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos//view/change-requests/job/PR-16769/15/testReport/

tanabarr · 2025-09-09T13:17:10Z

Can I ask gatekeepers to consider force landing #16769 despite the config generate failures? I'm working on resolving those separately and I don't think they are introduced by PR-16769 and are a separate issue. PR-16769 will hopefully help with SRE-3150 and I think it's required for PR-16761 to work properly.

I've restarted the hardware large stage but I think I'm still going to get the config generate failures.

@daos-stack/daos-gatekeeper I hit rerun from stage hardware but please use results from run 15 when considering landing it.

…#16769) Remove SPDK device lockfiles before and after ControlService performs a scan on engine-assigned bdevs when an engine is not running. This removes SPDK lockfiles for a device only when the engine has not claimed that device. Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com>

…#16769) (#17160) Remove SPDK device lockfiles before and after ControlService performs a scan on engine-assigned bdevs when an engine is not running. This removes SPDK lockfiles for a device only when the engine has not claimed that device. Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com>

* DAOS-18065 ci: empty /sys/class/nvme/ on leap15 when no NVMe device attached (#16956) (#17013) On leap15 the /sys/class/nvme/ folder exists but it is empty when no NVMe device attached. Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@hpe.com> * SRE-3562 ci: update MLNX_VER_NUM to 3.2.1 (#17463) (#17592) Required for EL9.7 Signed-off-by: Jan Michalski <jan-marian.michalski@hpe.com> * DAOS-17341 control: Remove spdk lockfiles on engine exit (#16181) (#16765) PR-15799 introduced a change which means SIGKILL is used to terminate engine processes during dmg system stop. One result of this is that SPDK processes don't cleanup NVMe SSD lockfiles that are intended to prevent multiple processes from accessing the same NVMe device. This change enables the control plane to: - Cleanup SPDK lockfiles and orphan hugepages on engine storage-ready, termination and server start-up. - Cleanup SPDK lockfiles and orphan hugepages on daos_server nvme prepare|reset. - Improve handling of disable_hugepages server config file parameter. - Clean all SPDK lockfiles when daos_server nvme prepare|reset is called without specifying any allow or block lists either via cli or config file. Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com> * DAOS-17935 control: Remove spdk lockfiles before and after local scan (#16769) (#17160) Remove SPDK device lockfiles before and after ControlService performs a scan on engine-assigned bdevs when an engine is not running. This removes SPDK lockfiles for a device only when the engine has not claimed that device. Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com> --------- Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@hpe.com> Signed-off-by: Jan Michalski <jan-marian.michalski@hpe.com> Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com> Co-authored-by: Tomasz Gromadzki <tomasz.gromadzki@hpe.com> Co-authored-by: Phil Henderson <phillip.henderson@hpe.com> Co-authored-by: Tom Nabarro <tom.nabarro@intel.com>

tanabarr requested review from a team as code owners August 26, 2025 20:46

tanabarr self-assigned this Aug 26, 2025

tanabarr requested a review from ryon-jensen August 26, 2025 22:06

ryon-jensen previously approved these changes Aug 26, 2025

View reviewed changes

tanabarr added bug control-plane work on the management infrastructure of the DAOS Control Plane labels Aug 27, 2025

tanabarr requested review from kccain, kjacque, knard38 and liw August 27, 2025 10:31

This was referenced Aug 27, 2025

DAOS-17935 control: Remove spdk lockfiles before scan or format (DON"T LAND) #16775

Draft

For testing #16762 & #16769 combined on HW #16782

Draft

liw reviewed Aug 29, 2025

View reviewed changes

liw previously approved these changes Aug 29, 2025

View reviewed changes

tanabarr dismissed stale reviews from liw and ryon-jensen via e78b4b2 August 29, 2025 15:47

tanabarr requested review from a team as code owners August 29, 2025 15:47

tanabarr force-pushed the tanabarr/control-spdk-clean-on-scanfmt branch from e78b4b2 to b84dfe1 Compare August 30, 2025 14:25

liw previously approved these changes Sep 1, 2025

View reviewed changes

tanabarr dismissed liw’s stale review via 906a63d September 2, 2025 11:56

DAOS-17935 control: Remove spdk lockfiles before scan or format

1d8f137

Features: control Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com>

move scan cleanup to backend

5f914d5

Features: control Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com>

tanabarr requested review from liw and ryon-jensen September 3, 2025 09:21

tanabarr changed the title ~~DAOS-17935 control: Remove spdk lockfiles before scan or format~~ DAOS-17935 control: Remove spdk lockfiles before and after local scan Sep 3, 2025

tanabarr force-pushed the tanabarr/control-spdk-clean-on-scanfmt branch from 906a63d to 2f1716a Compare September 3, 2025 09:54

revert changes no longer necessary

74719e7

Features: control Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com>

tanabarr force-pushed the tanabarr/control-spdk-clean-on-scanfmt branch from 2f1716a to 74719e7 Compare September 3, 2025 11:00

liw approved these changes Sep 3, 2025

View reviewed changes

kjacque reviewed Sep 4, 2025

View reviewed changes

tanabarr requested a review from kjacque September 4, 2025 18:25

kjacque approved these changes Sep 4, 2025

View reviewed changes

Merge remote-tracking branch 'origin/master' into tanabarr/control-sp…

bf763c6

…dk-clean-on-scanfmt Features: control Allow-unstable-test: true Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com>

tanabarr requested a review from a team September 9, 2025 13:17

tanabarr added the forced-landing The PR has known failures or has intentionally reduced testing, but should still be landed. label Sep 9, 2025

daltonbohning merged commit 51e7d20 into master Sep 9, 2025
29 of 30 checks passed

daltonbohning deleted the tanabarr/control-spdk-clean-on-scanfmt branch September 9, 2025 13:54

tanabarr mentioned this pull request Nov 21, 2025

DAOS-17935 control: Remove spdk lockfiles before and after local scan… #17160

Merged

6 tasks

Conversation

tanabarr commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Steps for the author:

After all prior steps are complete:

Uh oh!

github-actions Bot commented Aug 26, 2025

Uh oh!

ryon-jensen left a comment

Choose a reason for hiding this comment

Uh oh!

daosbuild3 commented Aug 27, 2025

Uh oh!

liw Aug 29, 2025

Choose a reason for hiding this comment

Uh oh!

tanabarr Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liw Aug 29, 2025

Choose a reason for hiding this comment

Uh oh!

liw Aug 29, 2025

Choose a reason for hiding this comment

Uh oh!

daosbuild3 commented Aug 29, 2025

Uh oh!

daosbuild3 commented Aug 30, 2025

Uh oh!

daosbuild3 commented Aug 30, 2025

Uh oh!

daosbuild3 commented Aug 30, 2025

Uh oh!

daosbuild3 commented Aug 31, 2025

Uh oh!

kjacque left a comment

Choose a reason for hiding this comment

Uh oh!

tanabarr commented Sep 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kjacque left a comment

Choose a reason for hiding this comment

Uh oh!

tanabarr commented Sep 6, 2025

Uh oh!

daosbuild3 commented Sep 6, 2025

Uh oh!

daosbuild3 commented Sep 7, 2025

Uh oh!

daosbuild3 commented Sep 8, 2025

Uh oh!

tanabarr commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Milestone

Development

Uh oh!

6 participants

tanabarr commented Aug 26, 2025 •

edited

Loading

tanabarr Aug 29, 2025 •

edited

Loading

tanabarr commented Sep 4, 2025 •

edited

Loading

tanabarr commented Sep 9, 2025 •

edited

Loading