os/bluestore: Fix problem with deferred writes replay by aclamk · Pull Request #60753 · ceph/ceph

aclamk · 2024-11-15T15:24:43Z

Fixes known problems with deferred write replay.

Based on unittest for deferred writes #60732

Fixes: https://tracker.ceph.com/issues/68060

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 retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test dashboard cephadm
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox
jenkins test windows
jenkins test rook e2e

src/os/bluestore/BlueStore.h

src/test/objectstore/test_deferred.cc

ifed01 · 2024-11-20T14:45:36Z

src/os/bluestore/BlueStore.cc

-  if (debug_deferred_replay_end) debug_deferred_replay_end();
-  if (fake_ch) {
-    new_coll_map.clear();
+  bdev->aio_submit(&ioctx);


Are we sure that it's safe enough to try to accumulate all the deferred writes in a single IO/DB txc batch? Couldn't it go beyond some bounds?

It is limited by amount of deferred writes pending. It is limited by amount of deferred writes BlueStore queued before crash.
Plus ones that are stale.

I will take a look.

Added batching of bdev ops.

src/os/bluestore/BlueStore.cc

src/os/bluestore/BlueStore.h

src/os/bluestore/BlueStore.cc

github-actions · 2024-12-06T21:20:52Z

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

Added debug hooks for: - init_alloc - deferred start/stop/operation This created framework for specific unit tests. These functions are as name suggests debug level, only for unittests. Signed-off-by: Adam Kupczyk <akupczyk@ibm.com>

Two possible cases of deferred write related corruption are replicated. 1. The case when L entries remain. 2. The case when deferred overwrites newly created BlueFS files Provides unittest for: https://tracker.ceph.com/issues/68060 Signed-off-by: Adam Kupczyk <akupczyk@ibm.com>

Modify _deferred_replay to execute it directly, without involving BlueStore state machine. In result, kv-sync thread is not necessary. RocksDB L entries (deferred writes) are removed directly. Fixes: https://tracker.ceph.com/issues/68060, part responsible for stale L entries. Signed-off-by: Adam Kupczyk <akupczyk@ibm.com>

aclamk · 2025-01-23T12:28:37Z

jenkins test make check

aclamk · 2025-01-23T12:28:52Z

jenkins test make check arm64

ifed01 · 2025-01-24T04:35:55Z

src/test/objectstore/test_deferred.cc

+    };
+  }

  // small deferred writes over object


The comment isn't correct now, is it?

Doch, it checks out.
16x IO by 8 bytes and then one 64K write.

I meant we don't overwrite the previous object any more

src/os/bluestore/BlueStore.cc

ifed01 · 2025-01-24T05:08:00Z

src/os/bluestore/BlueStore.cc

+          }
+        }
+      } else {
+        delete deferred_txn;


Looks like deferred_txn is leaking if not getting here.
Previously TransContext cared about that...

In fact we don't need it to be allocated from heap any more.

No longer allocated.

src/os/bluestore/BlueStore.cc

Modify _deferred_replay to separate: - applying IO to the disk - DB transaction to remove keys Changed _open_db_and_around. It now calls _deferred_replay. Adapted callers, including fsck. Fixed: https://tracker.ceph.com/issues/68060, original report. Signed-off-by: Adam Kupczyk <akupczyk@ibm.com>

aclamk · 2025-02-07T17:27:49Z

jenkins test make check arm64

aclamk · 2025-02-11T11:14:35Z

jenkins test make check

aclamk · 2025-02-12T11:35:27Z

jenkins test make check

aclamk · 2025-02-25T13:38:42Z

jenkins test make check arm64

Add code to print times; this is for make check arm64. Signed-off-by: Adam Kupczyk <akupczyk@ibm.com>

Signed-off-by: Adam Kupczyk <akupczyk@ibm.com>

aclamk · 2025-03-04T11:22:42Z

jenkins test make check arm64

aclamk · 2025-03-04T14:43:38Z

jenkins test make check arm64

aclamk · 2025-03-04T15:47:27Z

jenkins test make check arm64

ronen-fr · 2025-03-05T16:08:37Z

src/os/bluestore/BlueStore.cc

+  bool read_only,
+  bool to_repair,
+  bool apply_deferred,
+  bool remove_deferred)


Not a must for this specific PR, but can we create types for these 4 boolean values? having a function accept 4 boolean params is an invitation for problems...

ronen-fr · 2025-03-10T08:47:50Z

Please note - this issue is affecting multiple CI runs, and is a real nuisance...

Matan-B · 2025-03-11T08:48:04Z

Could this possibly break Crimson w/ Bluestore? See dead jobs, all due to unable to monut Bluestore:
https://pulpito.ceph.com/matan-2025-03-10_15:16:52-crimson-rados-wip-matanb-backfill-scan-distro-crimson-smithi/

ERROR 2025-03-10 15:39:32,855 [shard 0:main] none - bluestore(/var/lib/ceph/osd/ceph-0) _verify_csum bad crc32c/0x1000 checksum at blob offset 0x0, got 0x7c7a2766, expected 0x2ce10c3d, device location [0x432000~1000], logical extent 0x0~1000, object #-1:7b3f43c4:::osd_superblock:0#
ERROR 2025-03-10 15:39:32,855 [shard 0:main] none - bluestore(/var/lib/ceph/osd/ceph-0) _verify_csum bad crc32c/0x1000 checksum at blob offset 0x0, got 0x7c7a2766, expected 0x2ce10c3d, device location [0x432000~1000], logical extent 0x0~1000, object #-1:7b3f43c4:::osd_superblock:0#
ERROR 2025-03-10 15:39:32,855 [shard 0:main] none - bluestore(/var/lib/ceph/osd/ceph-0) _verify_csum bad crc32c/0x1000 checksum at blob offset 0x0, got 0x7c7a2766, expected 0x2ce10c3d, device location [0x432000~1000], logical extent 0x0~1000, object #-1:7b3f43c4:::osd_superblock:0#
ERROR 2025-03-10 15:39:32,855 [shard 0:main] none - bluestore(/var/lib/ceph/osd/ceph-0) _verify_csum bad crc32c/0x1000 checksum at blob offset 0x0, got 0x7c7a2766, expected 0x2ce10c3d, device location [0x432000~1000], logical extent 0x0~1000, object #-1:7b3f43c4:::osd_superblock:0#

aclamk requested a review from a team as a code owner November 15, 2024 15:24

github-actions bot added bluestore core tests labels Nov 15, 2024

aclamk force-pushed the wip-aclamk-more-deferred-overwrite-fix branch from 602b98c to deef599 Compare November 15, 2024 16:26

ifed01 reviewed Nov 20, 2024

View reviewed changes

github-actions bot added the needs-rebase label Dec 6, 2024

aclamk added 3 commits December 18, 2024 09:36

os/bluestore: Add debug hooks

cbfc24d

Added debug hooks for: - init_alloc - deferred start/stop/operation This created framework for specific unit tests. These functions are as name suggests debug level, only for unittests. Signed-off-by: Adam Kupczyk <akupczyk@ibm.com>

aclamk force-pushed the wip-aclamk-more-deferred-overwrite-fix branch from deef599 to 3e49cd4 Compare December 18, 2024 11:53

github-actions bot removed the needs-rebase label Dec 18, 2024

ifed01 reviewed Jan 24, 2025

View reviewed changes

aclamk force-pushed the wip-aclamk-more-deferred-overwrite-fix branch from 3e49cd4 to f2b33bc Compare January 24, 2025 11:39

ifed01 approved these changes Jan 24, 2025

View reviewed changes

aclamk added 3 commits March 1, 2025 09:32

test/unittest_deferred: debug code

fff7139

Add code to print times; this is for make check arm64. Signed-off-by: Adam Kupczyk <akupczyk@ibm.com>

more debug code

e1e8406

Signed-off-by: Adam Kupczyk <akupczyk@ibm.com>

fixup

5935a0f

Signed-off-by: Adam Kupczyk <akupczyk@ibm.com>

ronen-fr reviewed Mar 5, 2025

View reviewed changes

aclamk merged commit 9ef9c12 into ceph:main Mar 10, 2025
11 checks passed

Matan-B mentioned this pull request Mar 11, 2025

Revert "os/bluestore: Fix problem with deferred writes replay" #62214

Merged

14 tasks

aclamk mentioned this pull request Mar 20, 2025

os/bluestore: Revival of _deferred_replay fix #62412

Closed

14 tasks

Conversation

aclamk commented Nov 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution Guidelines

Checklist

Uh oh!

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!

github-actions bot commented Dec 6, 2024

Uh oh!

aclamk commented Jan 23, 2025

Uh oh!

aclamk commented Jan 23, 2025

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!

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!

aclamk commented Feb 7, 2025

Uh oh!

aclamk commented Feb 11, 2025

Uh oh!

aclamk commented Feb 12, 2025

Uh oh!

aclamk commented Feb 25, 2025

Uh oh!

aclamk commented Mar 4, 2025

Uh oh!

aclamk commented Mar 4, 2025

Uh oh!

aclamk commented Mar 4, 2025

Uh oh!

ronen-fr Mar 5, 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 Mar 10, 2025

Uh oh!

Uh oh!

Matan-B commented Mar 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aclamk commented Nov 15, 2024 •

edited

Loading

ronen-fr Mar 5, 2025 •

edited

Loading