cephfs: implement snapdiff by ifed01 · Pull Request #43546 · ceph/ceph

ifed01 · 2021-10-14T15:54:26Z

This patch supersedes both #42517 and #43328 and implements API to access snapshot delta (aka snapdiff) via
new libcephfs API method: readdir_snapdiff.

New test cases "LibCephFS.SnapDiff*" can serve as an example of new API calls.

Signed-off-by: Igor Fedotov igor.fedotov@croit.io
Signed-off-by: Denis Barahtanov denis.barahtanov@croit.io

Checklist

References tracker ticket
Updates documentation if necessary
Includes tests for new functionality or reproducer for bug

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

ifed01 · 2021-10-15T09:39:52Z

jenkins test make check

vshankar · 2021-10-18T04:36:47Z

@ifed01 I most likely have some unpublished comments in the other pr by @denisb-croit.

I'll go through this anyway.

vshankar · 2021-10-18T04:37:22Z

jenkins test make check arm64

ifed01 · 2021-11-08T13:46:57Z

@vshankar - gentle reminder about the PR ;)

vshankar · 2021-11-12T08:56:49Z

@vshankar - gentle reminder about the PR ;)

ACK.

vshankar · 2021-11-24T04:20:44Z

jenkins test make check arm64

vshankar · 2021-12-02T04:27:42Z

jenkins test make check arm64

vshankar · 2021-12-06T09:03:02Z

@ifed01 - sorry for the delay. I have started reviewing and testing these changes starting last Friday - will let you know how things go.

BTW, I'm planning to include these changes once quincy is branched off. I hope that is ok?

ifed01 · 2021-12-06T13:45:38Z

@ifed01 - sorry for the delay. I have started reviewing and testing these changes starting last Friday - will let you know how things go.
No worries, thanks in advance.

BTW, I'm planning to include these changes once quincy is branched off. I hope that is ok?
Sure.

ifed01 · 2021-12-20T15:39:28Z

@vshankar - any updates? ;)

vshankar

@ifed01 I have mostly reviewed changes in MDS in Denis's original PR which are carry-forwarded in this PR, so, just commenting on the interface for now...

(I have tried playing/testing the original PR which worked well)

vshankar · 2022-01-04T09:29:08Z

src/include/cephfs/libcephfs.h

+ *          is empty.  This pointer should not be freed by the caller, and is only safe to
+ *          access between return and the next call to ceph_readdir or ceph_closedir.
+ */
+int ceph_readdir_snapdiff(struct ceph_mount_info* cmount,


It's looks a bit unusual to pass in the snap-id of the other snapshot against which the delta has to be built. Could this be the snapshot name?

FWIW, this interface can accept two directory stream pointers (one for each snapshot) and a path for which the entries are to be built.

@ifed01 @batrick I'm open to suggestions on how the API would look like? Basically, I would like to avoid passing in snap-ids if possible.

@vshankar - I've reworked the API to accept snapshot root(!) dir stream pointer instead of snapid.
There is still a snapid returned by the function to distinguish removed and existing entries. Probably we can get rid off it as well and replace with an explicit bool flag... Do you think we need should do that?

I think better would be to create an API for building a snapdiff stream. Something like:

snapdiff_result_t* opensnapdiff(snappath1, snappath2);

Do sanity checks like same dir inode. Save snapids.

int readdir_snapdiff(snapdiff_result_t*, snapid_t* res_snap);

What do you think?

@batrick - I refactored this part a bit, please take a look. Is that what you suggested?

ifed01 · 2022-01-04T11:55:41Z

@ifed01 I have mostly reviewed changes in MDS in Denis's original PR which are carry-forwarded in this PR, so, just commenting on the interface for now...
@vshankar - thanks for taking a look. Could you please elaborate what PR are you referring as the original one: #42517 (which has got just the "tilda" interface) or #43328 (which is a dual-API evolution). I can't see any comments from your side for the latter - is it that good or you just haven't publish the comments? ;)

vshankar · 2022-01-04T12:16:51Z

@vshankar - thanks for taking a look. Could you please elaborate what PR are you referring as the original one: #42517 (which has got just the "tilda" interface) or #43328 (which is a dual-API evolution). I can't see any comments from your side for the latter - is it that good or you just haven't publish the comments? ;)

I'm talking about Denis's PR #42517 where I had some comments that Denis took care of.

github-actions · 2022-01-05T12:58:44Z

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

ifed01 · 2022-01-10T15:38:17Z

jenkins test make check

ifed01 · 2022-01-10T15:40:39Z

@vshankar - thanks for taking a look. Could you please elaborate what PR are you referring as the original one: #42517 (which has got just the "tilda" interface) or #43328 (which is a dual-API evolution). I can't see any comments from your side for the latter - is it that good or you just haven't publish the comments? ;)

I'm talking about Denis's PR #42517 where I had some comments that Denis took care of.

It looks like those comments have been answered/resolved? Do you see anything still open for discussion there ?

ifed01 · 2023-04-10T15:55:09Z

jenkins test make check

ifed01 · 2023-04-10T16:38:55Z

jenkins test ceph windows tests

ifed01 · 2023-04-10T16:53:48Z

jenkins test ceph windows

ifed01 · 2023-04-10T17:36:58Z

jenkins test windows

Signed-off-by: Igor Fedotov <igor.fedotov@croit.io>

vshankar · 2023-04-11T03:41:18Z

jenkins retest this please

petrutlucian94 · 2023-04-11T06:22:36Z

jenkins test windows

petrutlucian94 · 2023-04-11T06:29:16Z

I'm seeing some unrelated Windows test timeouts, will look into it.

ifed01 · 2023-04-12T14:34:22Z

jenkins test windows

petrutlucian94 · 2023-04-12T15:58:42Z

Thanks for issuing a Windows recheck. For what is worth, the Windows job is currently failing due to an unrelated regression: #51043

ifed01 · 2023-04-12T16:07:43Z

Thanks for issuing a Windows recheck. For what is worth, the Windows job is currently failing due to an unrelated regression: #51043

OK, good to know. Thanks!

vshankar · 2023-04-13T04:39:47Z

Thanks for issuing a Windows recheck. For what is worth, the Windows job is currently failing due to an unrelated regression: #51043

Thx @petrutlucian94. Since the failure is unrelated, this is good to merge then :)

Comments resolved

ifed01 · 2023-04-13T09:27:28Z

Thanks for issuing a Windows recheck. For what is worth, the Windows job is currently failing due to an unrelated regression: #51043

Thx @petrutlucian94. Since the failure is unrelated, this is good to merge then :)

OK, I'm going ahead and merge.. Thanks a lot to everyone involved ;)

cephfs: implement snapdiff Reviewed-by: Venky Shankar <vshankar@redhat.com>

* use a fixture for setup_test() * invoke test_cephfs.py with pytest Signed-off-by: Casey Bodley <cbodley@redhat.com> (cherry picked from commit 0bd813f) Conflicts: src/test/pybind/test_cephfs.py missing ceph#43546 missing ceph#46905

github-actions bot added build/ops cephfs Ceph File System common tests labels Oct 14, 2021

ifed01 mentioned this pull request Oct 14, 2021

[RFC,WIP] cephfs: implement snapdiff via two alternate APIs #43328

Closed

ifed01 requested a review from vshankar October 14, 2021 15:57

ghost mentioned this pull request Oct 16, 2021

cephfs: implement snapdiff via fake .snap subfolder [RFC] #42517

Closed

vshankar self-assigned this Dec 2, 2021

ifed01 force-pushed the wip-ifed-snapdiff3-good branch from 9a11e6e to 7bad085 Compare December 20, 2021 17:13

vshankar reviewed Jan 4, 2022

View reviewed changes

github-actions bot added the needs-rebase label Jan 5, 2022

ifed01 force-pushed the wip-ifed-snapdiff3-good branch from 7bad085 to 8e38b9c Compare January 10, 2022 12:52

github-actions bot removed the needs-rebase label Jan 10, 2022

ifed01 force-pushed the wip-ifed-snapdiff3-good branch from 8e38b9c to b317848 Compare January 10, 2022 13:15

vshankar requested a review from batrick January 12, 2022 05:05

test/libcephfs: add cases for last dir testing.

3e88c6b

Signed-off-by: Igor Fedotov <igor.fedotov@croit.io>

ifed01 force-pushed the wip-ifed-snapdiff3-good branch from 3661d13 to 3e88c6b Compare April 10, 2023 19:43

vshankar added ready-to-merge and removed wip-vshankar-testing2 labels Apr 11, 2023

ifed01 requested review from batrick and petrutlucian94 April 13, 2023 09:23

ifed01 merged commit 3db83ad into ceph:main Apr 13, 2023

ifed01 deleted the wip-ifed-snapdiff3-good branch April 13, 2023 14:35

yuvalif pushed a commit to yuvalif/ceph that referenced this pull request Apr 30, 2023

Merge pull request ceph#43546 from ifed01/wip-ifed-snapdiff3-good

15292ef

cephfs: implement snapdiff Reviewed-by: Venky Shankar <vshankar@redhat.com>

cbodley mentioned this pull request Jul 20, 2023

reef: test/pybind: replace nose with pytest #52566

Merged

ifed01 mentioned this pull request Aug 30, 2023

reef: cephfs: implement snapdiff #53229

Merged

14 tasks

Conversation

ifed01 commented Oct 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

ifed01 commented Oct 15, 2021

Uh oh!

vshankar commented Oct 18, 2021

Uh oh!

vshankar commented Oct 18, 2021

Uh oh!

ifed01 commented Nov 8, 2021

Uh oh!

vshankar commented Nov 12, 2021

Uh oh!

vshankar commented Nov 24, 2021

Uh oh!

vshankar commented Dec 2, 2021

Uh oh!

vshankar commented Dec 6, 2021

Uh oh!

ifed01 commented Dec 6, 2021

Uh oh!

ifed01 commented Dec 20, 2021

Uh oh!

vshankar left a comment

Choose a reason for hiding this comment

Uh oh!

vshankar Jan 4, 2022

Choose a reason for hiding this comment

Uh oh!

vshankar Jan 4, 2022

Choose a reason for hiding this comment

Uh oh!

ifed01 Jan 10, 2022

Choose a reason for hiding this comment

Uh oh!

batrick Feb 17, 2022

Choose a reason for hiding this comment

Uh oh!

ifed01 May 26, 2022

Choose a reason for hiding this comment

Uh oh!

ifed01 commented Jan 4, 2022

Uh oh!

vshankar commented Jan 4, 2022

Uh oh!

github-actions bot commented Jan 5, 2022

Uh oh!

ifed01 commented Jan 10, 2022

Uh oh!

ifed01 commented Jan 10, 2022

Uh oh!

ifed01 commented Apr 10, 2023

Uh oh!

ifed01 commented Apr 10, 2023

Uh oh!

ifed01 commented Apr 10, 2023

Uh oh!

ifed01 commented Apr 10, 2023

Uh oh!

vshankar commented Apr 11, 2023

Uh oh!

petrutlucian94 commented Apr 11, 2023

Uh oh!

petrutlucian94 commented Apr 11, 2023

Uh oh!

ifed01 commented Apr 12, 2023

Uh oh!

petrutlucian94 commented Apr 12, 2023

Uh oh!

ifed01 commented Apr 12, 2023

Uh oh!

vshankar commented Apr 13, 2023

Uh oh!

ifed01 commented Apr 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

ifed01 commented Oct 14, 2021 •

edited

Loading

ifed01 commented Apr 13, 2023 •

edited

Loading