osd: Correct scrub analysis for optimised EC by aainscow · Pull Request #62952 · ceph/ceph

aainscow · 2025-04-24T14:25:01Z

This PR changes the scrub code, adjusting for new EC. It does not attempt to fix any bugs which were detected (these will come in EC PRs).

There are two major areas where optimised EC differs:

Shard sizes. The shards are not all the same size, but are algorithmically related to the object size. Scrub now checks intended size.
version. Not all shards are updated on every writes. This means that the object versions on non-updated shards are inaccurate. The "authoritative" shards contains a list of versions which are old. Scrub now uses this to check.
New EC refuses to allow some shards to be primaries. These shards can never be authoritative in scrub.

NOTE: this PR is NOT implementing any deep-scrub CRC checking, this will be a later PR.

Tracker: https://tracker.ceph.com/issues/71285

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

ronen-fr · 2025-04-24T18:08:19Z

(I'll try to get to this on Sunday)

src/osd/scrubber/scrub_backend.h

ronen-fr · 2025-04-25T07:02:40Z

src/osd/scrubber_common.h


  // used to verify our "cleanliness" before scrubbing
  virtual bool is_waiting_for_unreadable_object() const = 0;
+  virtual bool get_is_nonprimary_shard(const pg_shard_t &pg_shard) const = 0;


Will add. The nonprimary shard is one that can never be a primary (or by implication authoritative)

I may be forgetting something here, but: why can't it be authoritative? in replicated pools - it surely can.

The nonprimary shards in new EC contain an old OI and do not have the snapset attributes, as such I don't think they can be authoritative.

Does that mean that we do not have any redundancy regarding the snapset attribute?

The snapset (and all other xattrs) is on shard 0 and all the parity shards. This gives the same redundancy as the data (i.e. m failures for a k+m EC)

ronen-fr · 2025-04-28T13:50:20Z

@aainscow - this is marked as draft, so, just to be sure: this is the correct PR to review? thanks

aainscow · 2025-04-28T13:52:22Z

@aainscow - this is marked as draft, so, just to be sure: this is the correct PR to review? thanks

Yes please. I marked as draft as I was getting some scrub errors, but it turns out they were fast-EC bugs, rather than scrubs.

src/osd/scrubber/scrub_backend.h

ronen-fr · 2025-04-29T15:08:26Z

Can we implement the new mechanism for all EC PGs, "optimized" or not?

src/osd/ECBackend.cc

ronen-fr · 2025-04-30T06:00:15Z

src/osd/scrubber/scrub_backend.cc

-    const bufferlist& auth_bl = auth_attr->second;
+    if (auth_oi.get_version_for_shard(shard.shard) == auth_oi.version) {
+      // The expected version of the shard and the authoritative shard are the
+      // same, so we can do a simple memcmp of the OI attr.


That's a nice idea!

Might be worth a separate PR (which can be backported)

The shard_version in OI does not exist pre-tentacle. I think this whole PR needs backporting to tentacle.

I can split out if you want, however.

I think it's a good idea. Not just for backporting, but it would help there for sure.

src/osd/scrubber/scrub_backend.cc

aainscow · 2025-04-30T10:31:24Z

Can we implement the new mechanism for all EC PGs, "optimized" or not?

I am not sure what is useful for non-optimized EC in here. All the non-primary changes are new-EC specific, as is the shard versioning.

…xactly match. With optimised EC, some shards do not receive every IO updates (partial writes). In such cases, the OI on these "nonprimary" shards is expected to be out of date. This commit allows for the mismatch and instead checks that the OI can be decoded and checks the version against the shard versions in the authoritative OI. Signed-off-by: Alex Ainscow <aainscow@uk.ibm.com>

ronen-fr · 2025-05-13T12:26:43Z

@aainscow - I am still missing something. isn't it that not_usable_no_err is set (also) for the replicas in replicated pools?

src/osd/scrubber/scrub_backend.cc

aainscow · 2025-05-13T13:07:59Z

@aainscow - I am still missing something. isn't it that not_usable_no_err is set (also) for the replicas in replicated pools?

This is the only thing to set not_usable_no_err:
nonprimary?usable_t::not_usable_no_err:usable_t::usable

nonprimary can only be true for new EC profiles. So I don't agree, or am I looking at the wrong thing?

src/osd/osd_types.h

src/osd/ECBackend.h

JonBailey1993 · 2025-05-15T10:15:47Z

I believe open comments should now be addressed. Requesting re-review.

ronen-fr · 2025-05-15T11:22:32Z

LGTM. I have emailed @athanatos , and asked him to go over this PR and the other one.

ronen-fr · 2025-05-15T12:42:21Z

jenkins test make check arm64

This is a collection of minor changes to make scrub compatible with new EC. Signed-off-by: Alex Ainscow <aainscow@uk.ibm.com>

athanatos

Seems reasonable to me.

ronen-fr · 2025-05-17T19:07:40Z

jenkins test make check arm64

aainscow · 2025-05-18T09:02:23Z

jenkins retest this please

Looks like this doesn't clear the default thing!

ronen-fr · 2025-05-18T10:53:13Z

@aainscow

the default thing

is not marked as 'required'. Just ignore it

amathuria · 2025-06-16T12:37:58Z

RADOS approved: https://tracker.ceph.com/projects/rados/wiki/MAIN#httpstrackercephcomissues71410

aainscow requested a review from a team as a code owner April 24, 2025 14:25

github-actions bot added the core label Apr 24, 2025

aainscow marked this pull request as draft April 24, 2025 15:40

ronen-fr reviewed Apr 25, 2025

View reviewed changes

src/osd/scrubber/scrub_backend.h Show resolved Hide resolved

ronen-fr reviewed Apr 25, 2025

View reviewed changes

aainscow marked this pull request as ready for review April 28, 2025 13:54

ronen-fr reviewed Apr 29, 2025

View reviewed changes

src/osd/scrubber/scrub_backend.h Outdated Show resolved Hide resolved

ronen-fr reviewed Apr 30, 2025

View reviewed changes

src/osd/ECBackend.cc Show resolved Hide resolved

ronen-fr reviewed Apr 30, 2025

View reviewed changes

src/osd/scrubber/scrub_backend.cc Show resolved Hide resolved

JonBailey1993 mentioned this pull request May 8, 2025

OSD: Optimised EC Deep Scrubbing Implementation #63184

Merged

14 tasks

github-actions bot added the tests label May 8, 2025

aainscow force-pushed the scrub_improvements branch from 9bc8d43 to ab5284f Compare May 12, 2025 14:06

aainscow requested a review from ronen-fr May 12, 2025 14:17

ronen-fr requested a review from athanatos May 13, 2025 12:26

ronen-fr reviewed May 13, 2025

View reviewed changes

src/osd/scrubber/scrub_backend.cc Show resolved Hide resolved

ronen-fr reviewed May 13, 2025

View reviewed changes

src/osd/scrubber/scrub_backend.cc Outdated Show resolved Hide resolved

ronen-fr reviewed May 13, 2025

View reviewed changes

src/osd/osd_types.h Outdated Show resolved Hide resolved

ronen-fr reviewed May 13, 2025

View reviewed changes

src/osd/ECBackend.h Outdated Show resolved Hide resolved

JonBailey1993 force-pushed the scrub_improvements branch 2 times, most recently from 32cc9bf to 8c7e79f Compare May 15, 2025 10:12

JonBailey1993 self-assigned this May 15, 2025

JonBailey1993 requested a review from ronen-fr May 15, 2025 10:15

JonBailey1993 added the needs-qa label May 15, 2025

osd: Correct scrub analysis for OSDs and fix up written shards.

b762e98

This is a collection of minor changes to make scrub compatible with new EC. Signed-off-by: Alex Ainscow <aainscow@uk.ibm.com>

JonBailey1993 force-pushed the scrub_improvements branch from 8c7e79f to b762e98 Compare May 15, 2025 13:19

athanatos approved these changes May 16, 2025

View reviewed changes

ronen-fr approved these changes May 17, 2025

View reviewed changes

SrinivasaBharath added the wip-bharath5-testing label May 18, 2025

aainscow mentioned this pull request Jun 10, 2025

Optimised EC: Ignore snapshot scrubbing on non-primary shards #63843

Merged

14 tasks

aainscow changed the title ~~osd: Correct scrub analysis for OSDs~~ osd: Correct scrub analysis for optimised EC Jun 11, 2025

ljflores merged commit 46c0d5c into ceph:main Jun 18, 2025
14 of 15 checks passed

SrinivasaBharath removed the wip-bharath5-testing label Jun 19, 2025

aainscow mentioned this pull request Jun 20, 2025

tentacle: osd: Correct scrub analysis for optimised EC #64064

Merged

Conversation

aainscow commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution Guidelines

Checklist

Uh oh!

ronen-fr commented Apr 24, 2025

Uh oh!

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!

aainscow Apr 30, 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!

Choose a reason for hiding this comment

Uh oh!

aainscow May 13, 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!

ronen-fr commented Apr 28, 2025

Uh oh!

aainscow commented Apr 28, 2025

Uh oh!

Uh oh!

ronen-fr commented Apr 29, 2025

Uh oh!

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!

Uh oh!

aainscow commented Apr 30, 2025

Uh oh!

ronen-fr commented May 13, 2025

Uh oh!

Uh oh!

Uh oh!

aainscow commented May 13, 2025

Uh oh!

Uh oh!

Uh oh!

JonBailey1993 commented May 15, 2025

Uh oh!

ronen-fr commented May 15, 2025

Uh oh!

ronen-fr commented May 15, 2025

Uh oh!

athanatos left a comment

Choose a reason for hiding this comment

Uh oh!

ronen-fr commented May 17, 2025

Uh oh!

aainscow commented May 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ronen-fr commented May 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

amathuria commented Jun 16, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

aainscow commented Apr 24, 2025 •

edited

Loading

aainscow Apr 30, 2025 •

edited

Loading

aainscow May 13, 2025 •

edited

Loading

aainscow commented May 18, 2025 •

edited

Loading

ronen-fr commented May 18, 2025 •

edited

Loading