Skip to content

DAOS-19496 control: remove per-engine sysdb on format replace - #18860

Open
tanabarr wants to merge 3 commits into
masterfrom
tanabarr/control-mdonssd-fmtreplace-perenginesysdb
Open

DAOS-19496 control: remove per-engine sysdb on format replace#18860
tanabarr wants to merge 3 commits into
masterfrom
tanabarr/control-mdonssd-fmtreplace-perenginesysdb

Conversation

@tanabarr

@tanabarr tanabarr commented Aug 13, 2026

Copy link
Copy Markdown
Contributor
 Allow selective per-engine control metadata formatting during storage
 format --replace operations. Previously, --replace would skip control
 metadata formatting entirely or format all engines. Now only engines
 with missing metadata directories have their control_metadata
 subdirectories reformatted, preserving healthy engines while clearing
 old rank metadata from replaced engines.

 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).

@tanabarr
tanabarr requested review from a team as code owners August 13, 2026 16:33
@tanabarr tanabarr self-assigned this Aug 13, 2026
@tanabarr
tanabarr requested review from knard38 and removed request for a team August 13, 2026 16:34
@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown

Ticket title is 'Enable single rank format replace in MD-on-SSD'
Status is 'In Review'
https://daosio.atlassian.net/browse/DAOS-19496

// Format control metadata first, if needed
if needs, err := cs.storage.ControlMetadataNeedsFormat(); err != nil {
return false, errors.Wrap(err, "detecting if metadata format is needed")
} else if needs || reformat {

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.

From my understanding, the engine-selection logic in the new replace branch relies solely on eng.GetStorage().ScmNeedsFormat(), which is purely an SCM/tmpfs mount check and has no relation to whether that engine's own control_metadata subdirectory is actually intact.

If I am correct, that means: if engine X's control-metadata subdirectory is corrupted but its SCM still reads as mounted/formatted, ScmNeedsFormat() returns false for it, it's excluded from needFormatIdxs, and its corrupted metadata is never wiped via --replace — silently, with no error or indication anything was skipped.

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've refactor the PR please review whether this comment is still relevant thanks

if err := p.sys.Chown(req.DataPath, req.OwnerUID, req.OwnerGID); err != nil {
return errors.Wrapf(err, "setting ownership of control metadata subdirectory to %d/%d", req.OwnerUID, req.OwnerGID)
// If specific engine indices are provided and DataPath exists, only delete those engines
if len(req.EngineIdxs) > 0 {

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.

From my understanding, formatMetadata()'s 'full format' branch (needs-format, or --force reformat) always builds engineIdxs from every currently-configured instance, so it's never empty when there's at least one engine.
If I am correct, that means this part of the code is not reachable any more. Maybe the condition should be changed?

@tanabarr
tanabarr marked this pull request as draft August 15, 2026 20:07
Features:  control
Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com>
@tanabarr
tanabarr force-pushed the tanabarr/control-mdonssd-fmtreplace-perenginesysdb branch from a18eb82 to e00ba3a Compare August 18, 2026 22:48
…onssd-fmtreplace-perenginesysdb

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

Copy link
Copy Markdown
Collaborator

@tanabarr
tanabarr requested a review from knard38 August 19, 2026 12:00
@tanabarr
tanabarr marked this pull request as ready for review August 19, 2026 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants