Mon: Add health warning for blaum-roth EC profile when w+1 is not prime by Tom-Sollers · Pull Request #65940 · ceph/ceph

Tom-Sollers · 2025-10-14T15:49:28Z

This PR adds a health warning for when a Erasure code profile uses the blaum-roth technique and has a w value such that w+1 is not prime, which can cause data corruption. It also improves the validation for when the user attempts to create a new erasure code profile by throwing a more descriptive warning when an invalid w value is used, and allows the user to override this validation using the --yes-i-really-mean-it flag. In addition to the previous changes this PR also adds a new test to /qa/standalone/erasure-code/test-erasure-code-plugins.sh to check that the health warning is functioning correctly.

Fixes: http://tracker.ceph.com/issues/64419
Signed-off-by: Tom Sollers tom.sollers@ibm.com

Contribution Guidelines

To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

Tracker (select at least one)
- References tracker ticket
- Very recent bug; references commit where it was introduced
- New feature (ticket optional)
- Doc update (no ticket needed)
- Code cleanup (no ticket needed)
Component impact
- Affects Dashboard, opened tracker ticket
- Affects Orchestrator, opened tracker ticket
- No impact that needs to be tracked
Documentation (select at least one)
- Updates relevant documentation
- No doc update is appropriate
Tests (select at least one)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

Show available Jenkins commands

jenkins test classic perf Jenkins Job | Jenkins Job Definition
jenkins test crimson perf Jenkins Job | Jenkins Job Definition
jenkins test signed Jenkins Job | Jenkins Job Definition
jenkins test make check Jenkins Job | Jenkins Job Definition
jenkins test make check arm64 Jenkins Job | Jenkins Job Definition
jenkins test submodules Jenkins Job | Jenkins Job Definition
jenkins test dashboard Jenkins Job | Jenkins Job Definition
jenkins test dashboard cephadm Jenkins Job | Jenkins Job Definition
jenkins test api Jenkins Job | Jenkins Job Definition
jenkins test docs ReadTheDocs | Github Workflow Definition
jenkins test ceph-volume all Jenkins Jobs | Jenkins Jobs Definition
jenkins test windows Jenkins Job | Jenkins Job Definition
jenkins test rook e2e Jenkins Job | Jenkins Job Definition

You must only issue one Jenkins command per-comment. Jenkins does not understand
comments with more than one command.

anthonyeleven

Left several suggestions. Please also change the commit/PR titles to be a present-tense imperative, i.e.

"mon: Add health warning for blaum-roth EC profile when w+1 is not prime"

Also, might it be possible to enhance the ceph osd erasure-code-profile set command to not create such a profile unless --yes-i-really-mean-it and -force are passed?

We would want to keep the health warning still to cover existing profiles/pools and cases where someone modifies existing defs via other means.

doc/rados/operations/health-checks.rst

src/mon/HealthMonitor.cc

qa/standalone/erasure-code/test-erasure-code-plugins.sh

bill-scales

Changes are looking good. This review is just very minor issues - mainly not following the coding standard that I've highlighted to get you into good habits :-)

src/erasure-code/jerasure/ErasureCodeJerasure.cc

src/mon/HealthMonitor.cc

bill-scales · 2025-10-16T07:47:55Z

src/mon/HealthMonitor.cc

+    dout(20) << erasure_code_profile << dendl;
+
+    //This will look at the erasure code profiles technique is blaum_roth and will check that the w key exists 
+      if (erasure_code_profile.second.at("technique") == "blaum_roth" && erasure_code_profile.second.count("w") == 1){


nit: coding standard has space before {
nit: coding standard is 80 character lines (a few exceptions are ok if its hard to break the line) - you could break the line after && here. There are several places in this PR where you need to fix line lengths

src/mon/HealthMonitor.cc

src/mon/OSDMonitor.cc

src/mon/HealthMonitor.cc

bluikko · 2025-10-16T09:04:04Z

The commit message signed-off-by lines are missing < and > around the e-mail address which makes the "Signed-off-by" job fail. I might be mistaken but I believe it might be a requirement to have those lines correct.

Tom-Sollers · 2025-10-16T14:55:21Z

Fixed all the commit messages so they now are all in the present tense and the email tom.sollers@ibm.com is now inside <>

Tom-Sollers · 2025-10-16T14:58:08Z

Amended a commit with a change that should have been a part of it

bill-scales · 2025-10-20T08:41:32Z

jenkins test make check

bill-scales · 2025-10-20T08:41:38Z

jenkins test make check arm64

Tom-Sollers · 2025-10-20T10:10:33Z

Updated all the commit messages again to remove src from the title and to remove the use of I.

…th a w+1 that is not prime This commit adds a new health warning for when a user has an erasure code profile using the blaum-roth technique which has a w+1 value that is not prime. Fixes: http://tracker.ceph.com/issues/64419 Signed-off-by: Tom Sollers <tom.sollers@ibm.com>

…alth warn This commit adds a new test to test-erasure-code-plugins.sh that tests for the health warning caused by having a erasure-code-profile with the blaum-roth technique and a w+1 value that is not prime. Fixes: http://tracker.ceph.com/issues/64419 Signed-off-by: Tom Sollers <tom.sollers@ibm.com>

Tom-Sollers · 2025-10-22T07:59:14Z

jenkins test make check arm64

Tom-Sollers · 2025-10-22T08:03:55Z

jenkins test make check arm64

Tom-Sollers · 2025-10-22T11:30:28Z

jenkins test make check arm64

bill-scales · 2025-10-23T06:51:48Z

jenkins test make check arm64

Tom-Sollers · 2025-10-23T08:52:39Z

jenkins test make check arm64

github-actions · 2025-10-24T10:46:55Z

This is an automated message by src/script/redmine-upkeep.py.

I have resolved the following tracker ticket due to the merge of this PR:

https://tracker.ceph.com/issues/64419

No backports are pending for the ticket. If this is incorrect, please update the tracker
ticket and reset to Pending Backport state.

Update Log: https://github.com/ceph/ceph/actions/runs/18777419898

NitzanMordhai · 2025-10-28T08:48:13Z

@Tom-Sollers that PR was merged before it went to QA, there are some issue that causing ceph-mon to segfault:

    -9> 2025-10-27T21:03:56.953+0000 7fa2013b5640 20 mon.a@0(leader).paxosservice(mgrstat 1..11) maybe_trim 1~6
    -8> 2025-10-27T21:03:56.953+0000 7fa2013b5640 10 mon.a@0(leader).paxosservice(mgrstat 1..11) maybe_trim trim_to 6 would only trim 5 < paxos_service_trim_min 250
    -7> 2025-10-27T21:03:56.953+0000 7fa2013b5640 10 mon.a@0(leader).health tick
    -6> 2025-10-27T21:03:56.953+0000 7fa2013b5640 20 mon.a@0(leader).health check_member_health
    -5> 2025-10-27T21:03:56.953+0000 7fa2013b5640 10 mon.a@0(leader).health check_member_health avail 99% total 15 GiB, used 72 MiB, avail 15 GiB
    -4> 2025-10-27T21:03:56.953+0000 7fa2013b5640 20 mon.a@0(leader).health check_leader_health
    -3> 2025-10-27T21:03:56.953+0000 7fa2013b5640 20 mon.a@0(leader).health check_netsplit
    -2> 2025-10-27T21:03:56.953+0000 7fa2013b5640 20 mon.a@0(leader).health check_erasure_code_profilescheckingdefault,{crush-failure-domain=osd,k=2,m=1,plugin=isa,technique=reed_sol_van}
    -1> 2025-10-27T21:03:56.953+0000 7fa2013b5640 20 mon.a@0(leader).health check_erasure_code_profilescheckinglrcprofile,{crush-device-class=,crush-failure-domain=osd,crush-num-failure-domains=0,crush-osds-per-failure-domain=0,crush-root=default,k=4,l=3,m=2,name=lrcprofile,plugin=lrc}
     0> 2025-10-27T21:03:56.957+0000 7fa2013b5640 -1 *** Caught signal (Aborted) **
 in thread 7fa2013b5640 thread_name:safe_timer

 ceph version 20.3.0-3797-g3375e026 (3375e026de8afb84f410a96280fab68dfd698f08) tentacle (dev - None)
 1: /lib/x86_64-linux-gnu/libc.so.6(+0x42520) [0x7fa206f75520]
 2: pthread_kill()
 3: raise()
 4: abort()
 5: /lib/x86_64-linux-gnu/libstdc++.so.6(+0xa2b9e) [0x7fa207305b9e]
 6: /lib/x86_64-linux-gnu/libstdc++.so.6(+0xae20c) [0x7fa20731120c]
 7: /lib/x86_64-linux-gnu/libstdc++.so.6(+0xae277) [0x7fa207311277]
 8: /lib/x86_64-linux-gnu/libstdc++.so.6(+0xae4d8) [0x7fa2073114d8]
 9: (std::__throw_out_of_range(char const*)+0x40) [0x7fa2073084a0]
 10: (HealthMonitor::check_erasure_code_profiles(health_check_map_t*)+0x16f) [0x55ff3653a25f]
 11: (HealthMonitor::check_leader_health()+0x27d) [0x55ff3653ae3d]
 12: (HealthMonitor::tick()+0x90) [0x55ff36525970]
 13: (Monitor::tick()+0xc0) [0x55ff36496d80]
 14: ceph-mon(+0x27b6ed) [0x55ff3643f6ed]
 15: (CommonSafeTimer<std::mutex>::timer_thread()+0x12f) [0x7fa2079f94ff]
 16: /usr/lib/ceph/libceph-common.so.2(+0x2b5fd1) [0x7fa2079f9fd1]
 17: /lib/x86_64-linux-gnu/libc.so.6(+0x94ac3) [0x7fa206fc7ac3]
 18: /lib/x86_64-linux-gnu/libc.so.6(+0x126850) [0x7fa207059850]
 NOTE: a copy of the executable, or `objdump -rdS <executable>` is needed to interpret this.

please check: /a/nmordech-2025-10-27_18:05:22-rados-wip-sjust-rocky10-mgr-serialize-distro-default-smithi/8570732
cc: @rzarzynski

NitzanMordhai · 2025-10-28T09:04:15Z

src/mon/HealthMonitor.cc

+
+  //This is a loop that will go through all the erasure code profiles 
+  for (auto& erasure_code_profile : mon.osdmon()->osdmap.get_erasure_code_profiles()) {
+    dout(20) << "check_erasure_code_profiles" << "checking" << erasure_code_profile << dendl;


nit: add space so the string won't looks like one word:

2025-10-27T21:03:56.953+0000 7fa2013b5640 20 mon.a@0(leader).health check_erasure_code_profilescheckingdefault,{crush-failure-domain=osd,k=2,m=1,plugin=isa,technique=reed_sol_van}

NitzanMordhai · 2025-10-28T09:05:51Z

src/mon/HealthMonitor.cc

+    dout(20) << "check_erasure_code_profiles" << "checking" << erasure_code_profile << dendl;
+
+    //This will look at the erasure code profiles technique is blaum_roth and will check that the w key exists 
+      if (erasure_code_profile.second.at("technique") == "blaum_roth" && 


We didn't have a technique in lrcprofile, that caused the fail:

2025-10-27T21:03:56.953+0000 7fa2013b5640 20 mon.a@0(leader).health check_erasure_code_profilescheckinglrcprofile,{crush-device-class=,crush-failure-domain=osd,crush-num-failure-domains=0,crush-osds-per-failure-domain=0,crush-root=default,k=4,l=3,m=2,name=lrcprofile,plugin=lrc}

…files with a w+1 that is not prime" This reverts commit b9aca15. See: ceph/ceph#65940 (comment)

…r new health warn" This reverts commit 256e277. See: ceph/ceph#65940 (comment) DISTROS: rocky10 ARCHS: x86_64 FLAVORS: default CEPH-BUILD-BRANCH: r10-container

…r new health warn" This reverts commit 256e277. See: ceph/ceph#65940 (comment)

…oth_for_ec_profiles" This reverts commit a222787, reversing changes made to cf11d9d. This was not QA'd or reviewed by SME for mon code changes. Signed-off-by: Patrick Donnelly <pdonnell@ibm.com>

batrick · 2025-11-03T22:48:49Z

I am reverting this PR in #66114. This should have been QA'd by the core team and reviewed by a Monitor SME.

Please do not merge your own PRs without consulting with a PTL.

* refs/pull/66114/head: Revert "Merge pull request #65940 from Tom-Sollers/fixing_blaum_roth_for_ec_profiles" Reviewed-by: Anthony D Atri <anthony.datri@gmail.com> Reviewed-by: Samuel Just <sjust@redhat.com>

…oth_for_ec_profiles" This reverts commit a222787, reversing changes made to cf11d9d. This was not QA'd or reviewed by SME for mon code changes. Signed-off-by: Patrick Donnelly <pdonnell@ibm.com>

Tom-Sollers self-assigned this Oct 14, 2025

Tom-Sollers requested review from a team as code owners October 14, 2025 15:49

github-actions bot added core documentation mon tests labels Oct 14, 2025

anthonyeleven reviewed Oct 14, 2025

View reviewed changes

Tom-Sollers changed the title ~~Mon: Added health warning for blaum-roth EC profile when w+1 is not prime~~ Mon: Add health warning for blaum-roth EC profile when w+1 is not prime Oct 15, 2025

bill-scales reviewed Oct 16, 2025

View reviewed changes

Tom-Sollers force-pushed the fixing_blaum_roth_for_ec_profiles branch from a071968 to 228a244 Compare October 16, 2025 14:45

Tom-Sollers force-pushed the fixing_blaum_roth_for_ec_profiles branch from 228a244 to 36dfa83 Compare October 16, 2025 14:56

Tom-Sollers requested review from anthonyeleven and bill-scales October 16, 2025 15:04

Tom-Sollers force-pushed the fixing_blaum_roth_for_ec_profiles branch from 36dfa83 to 11f1bc3 Compare October 20, 2025 10:08

Tom-Sollers force-pushed the fixing_blaum_roth_for_ec_profiles branch from 11f1bc3 to c88965d Compare October 21, 2025 15:40

Tom-Sollers added 2 commits October 21, 2025 16:53

Tom-Sollers force-pushed the fixing_blaum_roth_for_ec_profiles branch from c88965d to 256e277 Compare October 21, 2025 15:59

bill-scales approved these changes Oct 22, 2025

View reviewed changes

bill-scales added the needs-qa label Oct 22, 2025

Tom-Sollers merged commit a222787 into ceph:main Oct 24, 2025
13 checks passed

NitzanMordhai reviewed Oct 28, 2025

View reviewed changes

yaarith pushed a commit to ceph/ceph-ci that referenced this pull request Oct 29, 2025

Revert "mon: Add a new health warning for blaum-roth erasure code pro…

ed989da

…files with a w+1 that is not prime" This reverts commit b9aca15. See: ceph/ceph#65940 (comment)

yaarith pushed a commit to ceph/ceph-ci that referenced this pull request Oct 30, 2025

Revert "qa: Add a new test to test-erasure-code-plugins.sh to test fo…

4fb6c60

…r new health warn" This reverts commit 256e277. See: ceph/ceph#65940 (comment)

batrick mentioned this pull request Nov 5, 2025

Mon: Add new health warning for non prime w+1 in blaum-roth EC profiles #66129

Merged

14 tasks

aainscow mentioned this pull request Nov 7, 2025

EC Direct Reads: First PR, background work #65771

Merged

14 tasks

Conversation

Tom-Sollers commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution Guidelines

Checklist

Uh oh!

anthonyeleven left a comment • 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bill-scales left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bill-scales Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bluikko commented Oct 16, 2025

Uh oh!

Tom-Sollers commented Oct 16, 2025

Uh oh!

Tom-Sollers commented Oct 16, 2025

Uh oh!

bill-scales commented Oct 20, 2025

Uh oh!

bill-scales commented Oct 20, 2025

Uh oh!

Tom-Sollers commented Oct 20, 2025

Uh oh!

Tom-Sollers commented Oct 22, 2025

Uh oh!

Tom-Sollers commented Oct 22, 2025

Uh oh!

Tom-Sollers commented Oct 22, 2025

Uh oh!

bill-scales commented Oct 23, 2025

Uh oh!

Tom-Sollers commented Oct 23, 2025

Uh oh!

Uh oh!

github-actions bot commented Oct 24, 2025

Uh oh!

NitzanMordhai commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NitzanMordhai Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

NitzanMordhai Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

batrick commented Nov 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Tom-Sollers commented Oct 14, 2025 •

edited

Loading

anthonyeleven left a comment •

edited

Loading

NitzanMordhai commented Oct 28, 2025 •

edited

Loading