osd: fix for "data digests are inconsistent" by JonBailey1993 · Pull Request #65623 · ceph/ceph

JonBailey1993 · 2025-09-22T13:01:00Z

Fix for "data digests are inconsistent"

It was possible to see "data digests are inconsistent" being output to the logs at incorrect times due to multiple bugs. This code reorganises some of the deep scrubbing code and fixes the issues. The root cause of the issue that is being fixed here is:

We were comparing crc buffers beyond the end of the crcs
There was a double call to logical_to_ondisk_size when creating the crcs for zero buffers, causing them to be mis-sized
The code was not padding smaller shards as its a requirement for shards to be the same sized when used for parity comparison.

All the above are fixed in this commit.

Tracker ticket: https://tracker.ceph.com/issues/72945

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.

github-actions · 2025-09-22T15:29:49Z

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

JonBailey1993 · 2025-09-23T14:34:59Z

Make checks timed out. Will retrigger

JonBailey1993 · 2025-09-23T14:37:48Z

jenkins test make check

JonBailey1993 · 2025-09-23T14:37:59Z

jenkins test make check arm64

src/osd/scrubber/scrub_backend.cc

ronen-fr · 2025-09-30T14:23:52Z

(I hope to complete the review on my next work day - Sunday)

src/osd/scrubber/scrub_backend.cc

rzarzynski · 2025-09-30T23:36:17Z

src/osd/scrubber/scrub_backend.cc

+                                       const hobject_t& ho) {
+  this_chunk->m_ec_digest_map.clear();
+
+  if (auth_selection.auth_oi.version != eversion_t() && !m_is_replicated &&


Do we really need to repeat all the checks for m_is_replicated in a function names setup_ec_digest_map? Perhaps asserting once would be enough?

my suggested changes in #65497
introduced 'm_ec_digest_map_size' that saved us from having to repeat the check.
Might be usable here

I can assert that m_is_replicated is false at the start instead and move the check to outside the function, that would be more logical. The m_ec_digest_map_size does look like it will improve things here in the future too.

src/osd/scrubber/scrub_backend.h

src/osd/scrubber/scrub_backend.cc

ronen-fr

LGTM, assuming comments (esp. Radek's) are addressed.

JonBailey1993 · 2025-10-06T15:33:55Z

All comments should now be addressed/responded to

JonBailey1993 · 2025-10-07T12:58:02Z

jenkins test make check arm64

rzarzynski

LGTM!

rzarzynski · 2025-10-15T08:05:17Z

src/osd/scrubber/scrub_backend.h

+  std::string extract_crcs_from_map(const shard_id_map<bufferlist>& map);
+  std::string extract_crc_from_bufferlist(const bufferlist& crc_buffer);
+  char retrieve_byte(uint32_t value, uint32_t index) {
+    return value >> (8 * index) & 0xFF;


Hmm, it's surprising we don't such a primitive in include/intarith.h.

rzarzynski · 2025-10-15T08:05:48Z

src/osd/scrubber/scrub_backend.cc

    clog.error() << candidates_errors.str();
  }

+  if (!m_is_replicated) {


rzarzynski · 2025-10-15T08:09:35Z

src/osd/scrubber/scrub_backend.cc

+
        for (uint32_t i = 0; i < sizeof(zero_data_crc); i++) {
-          bl.c_str()[i] = bl[i] ^ ((zero_data_crc >> (8 * i)) & 0xff);
+          bl.c_str()[i] ^= retrieve_byte(zero_data_crc, i);


Way more readable.

rzarzynski · 2025-10-15T08:14:00Z

src/osd/scrubber/scrub_backend.cc

+    shard_id_set available_shards;
+
+    for (const auto& [srd, smap] : this_chunk->received_maps) {
+      if (!m_is_replicated && m_pg.get_ec_supports_crc_encode_decode() &&


Tiny nit: there is no (shouldn't be) way m_is_replicated can change here.

rzarzynski · 2025-10-15T08:14:25Z

src/osd/scrubber/scrub_backend.cc


+void ScrubBackend::setup_ec_digest_map(auth_selection_t& auth_selection,
+                                       const hobject_t& ho) {
+  ceph_assert(!m_is_replicated);


ljflores

Hey @JonBailey1993, the teuthology tests still turned up some instances of this error. I ensured the commit was present in these tests. Can you take a look?

/a/skanta-2025-10-09_23:38:36-rados-wip-bharath7-testing-2025-10-09-2128-distro-default-smithi/8543999
/a/skanta-2025-10-09_23:38:36-rados-wip-bharath7-testing-2025-10-09-2128-distro-default-smithi/8543947
/a/skanta-2025-10-09_23:38:36-rados-wip-bharath7-testing-2025-10-09-2128-distro-default-smithi/8543988
/a/skanta-2025-10-09_23:38:36-rados-wip-bharath7-testing-2025-10-09-2128-distro-default-smithi/8544022
/a/skanta-2025-10-09_23:38:36-rados-wip-bharath7-testing-2025-10-09-2128-distro-default-smithi/8544087
/a/skanta-2025-10-09_23:38:36-rados-wip-bharath7-testing-2025-10-09-2128-distro-default-smithi/8543867
/a/skanta-2025-10-09_23:38:36-rados-wip-bharath7-testing-2025-10-09-2128-distro-default-smithi/8543939
/a/skanta-2025-10-09_23:38:36-rados-wip-bharath7-testing-2025-10-09-2128-distro-default-smithi/8543942
/a/skanta-2025-10-09_23:38:36-rados-wip-bharath7-testing-2025-10-09-2128-distro-default-smithi/8544099

Testing ref: https://tracker.ceph.com/issues/73477

JonBailey1993 · 2025-11-04T09:59:02Z

Hey Laura, I'll take a look at this and see what has gone wrong

github-actions · 2025-11-04T14:32:39Z

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

rzarzynski · 2025-11-04T17:37:17Z

@JonBailey1993: it also needs a rebase.

JonBailey1993 · 2025-11-04T17:59:46Z

I will do the rebase with my fixes, thank you @rzarzynski

It was possible to see "data digests are inconsistent" being output to the logs at incorrect times due to multiple bugs. This code reorganises some of the deep scrubbing code and fixes the issues. The root cause of the issue that is being fixed here is: * We were comparing crc buffers beyond the end of the crcs * There was a double call to logical_to_ondisk_size when creating the crcs for zero buffers, causing them to be mis-sized * The code was not padding smaller shards as its a requirement for shards to be the same sized when used for parity comparison. All the above are fixed in this commit Signed-off-by: Jon Bailey <jonathan.bailey1@ibm.com>

…sistency of erasure coded pools Signed-off-by: Jon Bailey <jonathan.bailey1@ibm.com>

JonBailey1993 · 2025-11-14T11:41:47Z

The second issue appeared to be the changes were not guarded against running on shallow scrub. I've run some tests on teuthology and no longer see this error erroneously occurring as before. I've rebased on the latest now as well.

JonBailey1993 · 2025-11-14T14:58:32Z

jenkins test make check

JonBailey1993 · 2025-11-14T15:46:36Z

jenkins test make check

ljflores · 2025-12-02T23:59:44Z

Apologies for the delay on approval; I found an issue with a separate PR in the batch. We're rerunning tests and should have fresh results soon.

github-actions · 2026-02-01T01:31:22Z

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

JonBailey1993 · 2026-02-02T09:08:15Z

Not stale. Currently in QA.

JonBailey1993 · 2026-02-05T16:26:41Z

Rados approved: https://tracker.ceph.com/projects/rados/wiki/MAIN#httpstrackercephcomissues73898

github-actions · 2026-02-05T22:49:06Z

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/72945

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/21731423168

JonBailey1993 requested a review from a team as a code owner September 22, 2025 13:01

github-actions bot added the core label Sep 22, 2025

JonBailey1993 force-pushed the data_digests_are_inconsistent_fix branch from 4c0c4f0 to e2b3571 Compare September 22, 2025 15:29

github-actions bot added the needs-rebase label Sep 22, 2025

JonBailey1993 marked this pull request as draft September 22, 2025 15:41

JonBailey1993 force-pushed the data_digests_are_inconsistent_fix branch from e2b3571 to 8b5e9e6 Compare September 22, 2025 17:05

github-actions bot removed the needs-rebase label Sep 22, 2025

JonBailey1993 marked this pull request as ready for review September 22, 2025 17:06

JonBailey1993 force-pushed the data_digests_are_inconsistent_fix branch from 8b5e9e6 to c143b5f Compare September 22, 2025 17:15

rzarzynski requested review from ronen-fr and rzarzynski September 29, 2025 18:48

ronen-fr reviewed Sep 30, 2025

View reviewed changes

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

ronen-fr reviewed Sep 30, 2025

View reviewed changes

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

rzarzynski reviewed Sep 30, 2025

View reviewed changes

ronen-fr mentioned this pull request Oct 5, 2025

osd/scrub: relocate some EC CRC collection/usage #65497

Draft

ronen-fr reviewed Oct 5, 2025

View reviewed changes

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

ronen-fr reviewed Oct 5, 2025

View reviewed changes

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

ronen-fr approved these changes Oct 5, 2025

View reviewed changes

JonBailey1993 force-pushed the data_digests_are_inconsistent_fix branch from c143b5f to cbfe3da Compare October 6, 2025 15:27

JonBailey1993 added the needs-qa label Oct 7, 2025

SrinivasaBharath added the wip-bharath7-testing label Oct 9, 2025

rzarzynski approved these changes Oct 15, 2025

View reviewed changes

ljflores reviewed Oct 24, 2025

View reviewed changes

SrinivasaBharath removed wip-bharath7-testing needs-qa labels Oct 27, 2025

github-actions bot added the needs-rebase label Nov 4, 2025

JonBailey1993 added 2 commits November 14, 2025 11:35

Ensure scrub is doing a deep-scrub before attempting to check the con…

20623d6

…sistency of erasure coded pools Signed-off-by: Jon Bailey <jonathan.bailey1@ibm.com>

JonBailey1993 force-pushed the data_digests_are_inconsistent_fix branch from cbfe3da to 20623d6 Compare November 14, 2025 11:37

github-actions bot removed the needs-rebase label Nov 14, 2025

amathuria added the needs-qa label Nov 17, 2025

SrinivasaBharath added the wip-bharath10-testing label Nov 18, 2025

github-actions bot added the stale label Feb 1, 2026

ronen-fr removed the stale label Feb 2, 2026

SrinivasaBharath merged commit dfa42f1 into ceph:main Feb 5, 2026
13 checks passed

SrinivasaBharath removed the wip-bharath10-testing label Feb 5, 2026

Conversation

JonBailey1993 commented Sep 22, 2025

Contribution Guidelines

Checklist

Uh oh!

github-actions bot commented Sep 22, 2025

Uh oh!

JonBailey1993 commented Sep 23, 2025

Uh oh!

JonBailey1993 commented Sep 23, 2025

Uh oh!

JonBailey1993 commented Sep 23, 2025

Uh oh!

Uh oh!

Uh oh!

ronen-fr commented Sep 30, 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!

Uh oh!

Uh oh!

Uh oh!

ronen-fr left a comment

Choose a reason for hiding this comment

Uh oh!

JonBailey1993 commented Oct 6, 2025

Uh oh!

JonBailey1993 commented Oct 7, 2025

Uh oh!

rzarzynski left a comment

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!

ljflores 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!

JonBailey1993 commented Nov 4, 2025

Uh oh!

github-actions bot commented Nov 4, 2025

Uh oh!

rzarzynski commented Nov 4, 2025

Uh oh!

JonBailey1993 commented Nov 4, 2025

Uh oh!

JonBailey1993 commented Nov 14, 2025

Uh oh!

JonBailey1993 commented Nov 14, 2025

Uh oh!

JonBailey1993 commented Nov 14, 2025

Uh oh!

ljflores commented Dec 2, 2025

Uh oh!

github-actions bot commented Feb 1, 2026

Uh oh!

JonBailey1993 commented Feb 2, 2026

Uh oh!

JonBailey1993 commented Feb 5, 2026

Uh oh!

Uh oh!

github-actions bot commented Feb 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

ljflores left a comment •

edited

Loading