mds/client: file blockdiff support by vshankar · Pull Request #61937 · ceph/ceph

vshankar · 2025-02-20T13:31:33Z

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

mchangir

some early comments of a partial review

src/mds/MDCache.cc

vshankar · 2025-02-24T04:41:14Z

I will push in the mirror daemon changes to and then mark this PR as ready-for-review.

vshankar · 2025-02-28T05:03:26Z

Marking this ready for review as the latest push has integration of the block diff API with the mirror daemon. The mirror daemon code needs a bit more polishing, however, I think its good to seek reviews before starting that effort.

vshankar · 2025-02-28T05:08:03Z

@joscollin The latest push includes commits from PR #61539. I'll will probably integrate those changes into an existing commit and add your credits in the commit message.

vshankar · 2025-03-03T07:46:40Z

I'm working on fixing a bug identified in testing the mirror daemon block diff integration.

vshankar · 2025-03-03T15:59:30Z

I'm working on fixing a bug identified in testing the mirror daemon block diff integration.

The issue seems to be related to snapdiff not returning deleted dentry information for a file which is a symlink in a snapshot and a regular file in another snap. Discussing with @ifed01 on slack.

vshankar · 2025-03-06T05:48:47Z

To be tested with #62126

vshankar · 2025-03-06T05:51:17Z

This PR is under test in https://tracker.ceph.com/issues/70332.

src/mds/MDCache.cc

vshankar · 2025-03-06T10:04:39Z

Also fixed up a minor test refactor -- nothing but, just rearranging the test case.

vshankar · 2025-03-06T10:05:16Z

Question to @ceph/cephfs - does this new mds operation need a feature bit?

vshankar · 2025-03-06T10:08:39Z

Question to @ceph/cephfs - does this new mds operation need a feature bit?

EDIT: The MDS would return -ENOTSUPP if it does not support the (block diff) op. The mirror daemon can handle this error and fallback to full file copy.

vshankar · 2025-03-10T09:26:42Z

This PR is under test in https://tracker.ceph.com/issues/70332.

There is a failed mirror daemon test that seems related: https://pulpito.ceph.com/vshankar-2025-03-10_05:25:01-fs:mirror-wip-vshankar-testing-20250306.055054-debug-testing-default-smithi/8178206/

Working on that.

vshankar · 2025-03-10T14:57:49Z

Full fs suite run: https://pulpito.ceph.com/vshankar-2025-03-10_04:27:16-fs-wip-vshankar-testing-20250306.055054-debug-testing-default-smithi/

Nothing stood out at first pass, but I'll go through the files in detail to see if something new broke.

vshankar · 2025-03-12T14:00:41Z

I refreshed this PR with just the mds and client changes for the new block diff operation. The mirror daemon changes will be a separate PR.

vshankar · 2025-03-13T05:19:15Z

@kotreshhr @mchangir PTAL - this change is ~~not~~ now only the mds and client side changes for the new block diff call. This should not break anything since it's an entirely new MDS operation. The thing to focus a bit more during review would be the blockdiff call to older MDSs which does not understand this (new) operation. I remember some older MDSs would ceph_abort() on seeing an operation that it does not understand, but I think we fixed those (in quincy+) to derr log and return -ENOTSUP.

vshankar · 2025-03-13T05:19:53Z

jenkins test make check

vshankar · 2025-03-13T05:19:58Z

jenkins test make check arm64

kotreshhr · 2025-03-13T08:30:29Z

jenkins test make check

kotreshhr · 2025-03-13T12:54:59Z

Review Comments:

Overall the code looks good.

I particularly tested with mds (not having blockdiff api) with the libcephfs program compiled with blockdiff api support.
I didn't see any issues and got the "Operation Not supported" error as expected. Please find the mds log snippet below.

2025-03-13T18:16:22.506+0530 7f30df0ee6c0  1 -- [v2:192.168.1.114:6810/4235873411,v1:192.168.1.114:6811/4235873411] <== client.4163 192.168.1.114:0/1873501055 13 ==== client_request
(client.4163:11 ??? #0x10000000002//snap1/fileA #0x10000000002//snap2/fileA 2025-03-13T18:16:22.508074+0530 caller_uid=1000, caller_gid=1000{}) ==== 192+0+0 (crc 0 0 0) 0x564125f6bc
00 con 0x564125fa1000
2025-03-13T18:16:22.506+0530 7f30df0ee6c0  4 mds.0.server handle_client_request client_request(client.4163:11 ??? #0x10000000002//snap1/fileA #0x10000000002//snap2/fileA 2025-03-13T
18:16:22.508074+0530 caller_uid=1000, caller_gid=1000{})
2025-03-13T18:16:22.506+0530 7f30df0ee6c0 20 mds.0.3 get_session have 0x564125f9c500 client.4163 192.168.1.114:0/1873501055 state open
2025-03-13T18:16:22.506+0530 7f30df0ee6c0 15 mds.0.server  oldest_client_tid=11
2025-03-13T18:16:22.506+0530 7f30df0ee6c0  7 mds.0.cache request_start request(client.4163:11 nref=2 cr=0x564125f6bc00)
2025-03-13T18:16:22.506+0530 7f30df0ee6c0  7 mds.0.server dispatch_client_request client_request(client.4163:11 ??? #0x10000000002//snap1/fileA #0x10000000002//snap2/fileA 2025-03-1
3T18:16:22.508074+0530 caller_uid=1000, caller_gid=1000{})
2025-03-13T18:16:22.506+0530 7f30df0ee6c0  1 mds.0.server  unknown client op 5125
2025-03-13T18:16:22.506+0530 7f30df0ee6c0  7 mds.0.server reply_client_request -95 ((95) Operation not supported) client_request(client.4163:11 ??? #0x10000000002//snap1/fileA #0x10000000002//snap2/fileA 2025-03-13T18:16:22.508074+0530 caller_uid=1000, caller_gid=1000{})
2025-03-13T18:16:22.506+0530 7f30df0ee6c0 10 mds.0.server apply_allocated_inos 0x0 / [] / 0x0
2025-03-13T18:16:22.506+0530 7f30df0ee6c0  1 mds.0.server : unknown client op
2025-03-13T18:16:22.506+0530 7f30df0ee6c0 20 mds.0.server lat 0.000027
2025-03-13T18:16:22.506+0530 7f30df0ee6c0 10 mds.0.3 send_message_client client.4163 192.168.1.114:0/1873501055 client_reply(???:11 = -95 (95) Operation not supported safe)
2025-03-13T18:16:22.506+0530 7f30df0ee6c0  1 -- [v2:192.168.1.114:6810/4235873411,v1:192.168.1.114:6811/4235873411] --> 192.168.1.114:0/1873501055 -- client_reply(???:11 = -95 (95) Operation not supported safe) -- 0x564125f27a00 con 0x564125fa1000

I am attaching the test program blockdiff.txt here for reference.

Following is the program output

kotresh:ceph$ g++ -o blockdiff ./blockdiff.cc -I./src/include/ -I./build/include -I./build -I./build/src/include -L./build/lib/ -lcephfs -lceph-common -D_FILE_OFFSET_BITS=64 -std=c++20 -Wl,-rpath,/home/kotresh/sandbox/upstream/kotresh-ceph2/ceph/build/lib

kotresh:ceph$ 
kotresh:ceph$ ./blockdiff 
CephFS mounted successfully!
 Failed to get next changed block, ret:-95

vshankar · 2025-03-13T12:58:12Z

Tests running here: https://tracker.ceph.com/issues/70394

vshankar · 2025-03-14T04:08:36Z

Review Comments:

Overall the code looks good.

I particularly tested with mds (not having blockdiff api) with the libcephfs program compiled with blockdiff api support. I didn't see any issues and got the "Operation Not supported" error as expected. Please find the mds log snippet below.

While that fine, I was talking to Greg today regarding this PR in general and it would benefit to add a feature bit (feature exchange b/w the client and the mds) that's used by the client to check if the mds supports block diff.

vshankar · 2025-03-17T10:00:03Z

The fs suite run didn't report any failures related to this change. I plan to merge this as it is an follow up with feature bit check as mentioned in #61937 (comment).

mchangir · 2025-03-17T16:30:20Z

The fs suite run didn't report any failures related to this change. I plan to merge this as it is an follow up with feature bit check as mentioned in #61937 (comment).

Any updates regarding the block size used by RADOS and the experiment that I had run to test fetching of rados block overlaps between two snaps?

kotreshhr · 2025-03-18T11:57:55Z

Rerun of failed/dead jobs as there were infra errors https://pulpito.ceph.com/khiremat-2025-03-18_09:30:13-fs-wip-vshankar-testing-20250313.072951-debug-testing-default-smithi/

vshankar · 2025-03-18T12:26:45Z

The fs suite run didn't report any failures related to this change. I plan to merge this as it is an follow up with feature bit check as mentioned in #61937 (comment).

Any updates regarding the block size used by RADOS and the experiment that I had run to test fetching of rados block overlaps between two snaps?

Err. no. I need to spend time on the case you mention (currently occupied with other stuff).

joscollin · 2025-03-19T04:55:04Z

@joscollin The latest push includes commits from PR #61539. I'll will probably integrate those changes into an existing commit and add your credits in the commit message.

@vshankar
#61539 commits are dropped from this PR?

vshankar · 2025-03-19T04:59:50Z

@joscollin The latest push includes commits from PR #61539. I'll will probably integrate those changes into an existing commit and add your credits in the commit message.

@vshankar
#61539 commits are dropped from this PR?

Yes and included in the other pr for integrating blockdiff with mirror daemon.

vshankar · 2025-03-19T05:11:10Z

@joscollin See #62250

kotreshhr

Looks good to me. Went through test results. Didn't find any relevant or new failures.

vshankar · 2025-03-19T09:41:39Z

Looks good to me. Went through test results. Didn't find any relevant or new failures.

Thanks! I will have a look at @mchangir query and if no-one has any other comments, then this change can be merged.

I will follow this up with the future check change.

vshankar · 2025-03-19T10:46:28Z

For @mchangir query on slack

I tried running rados listsnaps and here's the output of a file with 4096 As in snap1 followed by an append of 4096 Bs in snap2 followed by an append of 4096 Cs in snap3
$ ./bin/rados listsnaps --pool=cephfs.a.data 10000000000.00000000
10000000000.00000000:
cloneid	snaps	size	overlap
2	2	4096	[]
3	3	8192	[0~8192]
head	-	12288
what is the block size considered as an overlap unit ?

I guess when the Bs were written, it was written to a separate file and copied over the existing file, yes? Doing it that way calls truncate(0) which essentially results in no overlaps in between snaps. And the next operation basically appened data to this file (after a snap is taken), and that's the reason there is a [0-8k] overlap b/w snap=id 3 and head.

vshankar · 2025-03-19T14:53:02Z

@mchangir - I ran your test case and listsnaps shows the correct overlapping region.

   for ((i=0; i <4096; i++)); do echo -n 'A'; done > ./test-file
   stat test-file
   python -c 'print(hex(1099511631268))'
   mkdir .snap/snap_a
   dd ibs=4096 skip=1 if=~/local-test-file of=./test-file count=1 conv=notrunc oflag=append
   mkdir .snap/snap_b
   for ((i=0; i <$((4096*3)); i++)); do echo -n 'C'; done > ~/local-test-file2
   dd ibs=4096 skip=2 if=~/local-test-file2 of=./test-file count=1 conv=notrunc oflag=append

listsnaps:

./bin/rados -p cephfs.a.data listsnaps 10000000da4.00000000
10000000da4.00000000 (seq:220):
cloneid snaps   size    overlap
219     219     4096    [0~4096]
220     220     8192    [0~8192]
head    -       12288

mchangir · 2025-03-19T15:47:03Z

@mchangir - I ran your test case and listsnaps shows the correct overlapping region.

   for ((i=0; i <4096; i++)); do echo -n 'A'; done > ./test-file
   stat test-file
   python -c 'print(hex(1099511631268))'
   mkdir .snap/snap_a
   dd ibs=4096 skip=1 if=~/local-test-file of=./test-file count=1 conv=notrunc oflag=append
   mkdir .snap/snap_b
   for ((i=0; i <$((4096*3)); i++)); do echo -n 'C'; done > ~/local-test-file2
   dd ibs=4096 skip=2 if=~/local-test-file2 of=./test-file count=1 conv=notrunc oflag=append

listsnaps:

./bin/rados -p cephfs.a.data listsnaps 10000000da4.00000000
10000000da4.00000000 (seq:220):
cloneid snaps   size    overlap
219     219     4096    [0~4096]
220     220     8192    [0~8192]
head    -       12288

that looks much better
has anything been fixed in the rados layer ?
I'll be running some tests to test holes.

vshankar · 2025-03-19T16:45:41Z

has anything been fixed in the rados layer ?

Not that I know of.

I'll be running some tests to test holes.

Good line of thought. For holes, the object would be missing. blockdiff reports the whole extent if there is hole. See: https://github.com/ceph/ceph/pull/61937/files#diff-ea7cb1a6ba9fa08363b14dd00a86bc6b79e01673e93af84ffdfcdbd0d3f26b19R14639

This is fine, since there is no hole punch support in cephfs client (at least the user-space client).

vshankar · 2025-03-20T03:51:01Z

@mchangir @kotreshhr If there is nothing pending for this, please merge.

kotreshhr · 2025-03-20T08:41:48Z

@mchangir @kotreshhr If there is nothing pending for this, please merge.

Please find run wiki here.
https://tracker.ceph.com/projects/cephfs/wiki/QA_main_2025#wip-vshankar-testing-20250313072951-debug

vshankar added the cephfs Ceph File System label Feb 20, 2025

vshankar requested a review from a team February 20, 2025 13:31

github-actions bot added common core tests labels Feb 20, 2025

mchangir reviewed Feb 22, 2025

View reviewed changes

src/mds/MDCache.cc Show resolved Hide resolved

src/mds/MDCache.cc Show resolved Hide resolved

vshankar mentioned this pull request Feb 24, 2025

cephfs_mirror: avoid latest changes on the source fs to enable mirroring #61539

Closed

14 tasks

vshankar force-pushed the wip-file-block-diff branch from da173b1 to 6b48bbd Compare February 28, 2025 05:00

vshankar marked this pull request as ready for review February 28, 2025 05:02

vshankar requested a review from a team as a code owner February 28, 2025 05:02

vshankar mentioned this pull request Feb 28, 2025

cephfs-mirror: cephfs-mirror should be able to do operations concurrently for large directory #61245

Closed

14 tasks

vshankar requested review from a team, joscollin and sajibreadd February 28, 2025 05:07

vshankar force-pushed the wip-file-block-diff branch from 6b48bbd to 0e52abe Compare March 6, 2025 05:47

vshankar added the wip-vshankar-testing1 label Mar 6, 2025

kotreshhr reviewed Mar 6, 2025

View reviewed changes

src/mds/MDCache.cc Outdated Show resolved Hide resolved

vshankar force-pushed the wip-file-block-diff branch from 0e52abe to 5accf36 Compare March 6, 2025 10:00

vshankar force-pushed the wip-file-block-diff branch from 91e1125 to bb3be44 Compare March 12, 2025 13:59

vshankar mentioned this pull request Mar 12, 2025

cephfs-mirror: integrate blockdiff with mirror daemon #62250

Merged

14 tasks

vshankar requested review from kotreshhr and mchangir March 13, 2025 04:19

vshankar added wip-vshankar-testing4 and removed wip-vshankar-testing1 labels Mar 13, 2025

kotreshhr approved these changes Mar 19, 2025

View reviewed changes

kotreshhr merged commit 3f67d1f into ceph:main Mar 20, 2025
11 checks passed

Conversation

vshankar commented Feb 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution Guidelines

Checklist

Uh oh!

mchangir left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vshankar commented Feb 24, 2025

Uh oh!

vshankar commented Feb 28, 2025

Uh oh!

vshankar commented Feb 28, 2025

Uh oh!

vshankar commented Mar 3, 2025

Uh oh!

vshankar commented Mar 3, 2025

Uh oh!

vshankar commented Mar 6, 2025

Uh oh!

vshankar commented Mar 6, 2025

Uh oh!

Uh oh!

vshankar commented Mar 6, 2025

Uh oh!

vshankar commented Mar 6, 2025

Uh oh!

vshankar commented Mar 6, 2025

Uh oh!

vshankar commented Mar 10, 2025

Uh oh!

vshankar commented Mar 10, 2025

Uh oh!

vshankar commented Mar 12, 2025

Uh oh!

vshankar commented Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vshankar commented Mar 13, 2025

Uh oh!

vshankar commented Mar 13, 2025

Uh oh!

kotreshhr commented Mar 13, 2025

Uh oh!

kotreshhr commented Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vshankar commented Mar 13, 2025

Uh oh!

vshankar commented Mar 14, 2025

Uh oh!

vshankar commented Mar 17, 2025

Uh oh!

mchangir commented Mar 17, 2025

Uh oh!

kotreshhr commented Mar 18, 2025

Uh oh!

vshankar commented Mar 18, 2025

Uh oh!

joscollin commented Mar 19, 2025

Uh oh!

vshankar commented Mar 19, 2025

Uh oh!

vshankar commented Mar 19, 2025

Uh oh!

kotreshhr left a comment

Choose a reason for hiding this comment

Uh oh!

vshankar commented Mar 19, 2025

Uh oh!

vshankar commented Mar 19, 2025

Uh oh!

vshankar commented Mar 19, 2025

Uh oh!

mchangir commented Mar 19, 2025

Uh oh!

vshankar commented Mar 19, 2025

vshankar commented Feb 20, 2025 •

edited

Loading

vshankar commented Mar 13, 2025 •

edited

Loading

kotreshhr commented Mar 13, 2025 •

edited

Loading