mds: wait unlink to finish to avoid conflict when creating same dentries #46331

lxbsz · 2022-05-19T02:38:44Z

If the previous unlink request has been delayed due to some reasons,
and the new creating for the same dentry may fail or new open will
succeeds but new contents wrote to it will be lost.

The kernel client will make sure before the unlink getting the first
reply it won't send the followed create requests for the same dentry.
Here we need to make sure that before the first reply has been sent
out the dentry must be marked as unlinking.

Fixes: https://tracker.ceph.com/issues/55332
Signed-off-by: Xiubo Li xiubli@redhat.com

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. "pacific"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

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

mchangir

just a few nits

src/mds/MDCache.h

src/mds/Server.cc

joscollin · 2022-06-09T12:33:14Z

jenkins test make check

src/msg/Message.h

src/mds/MDCache.cc

src/mds/MDCache.h

src/mds/MDCache.cc

src/mds/Server.cc

src/mds/MDCache.cc

vshankar · 2022-06-29T05:10:36Z

@lxbsz Is this ready for another round of review?

lxbsz · 2022-06-29T06:59:30Z

@lxbsz Is this ready for another round of review?

Yeah, I have already fixed the issues as the comments mentioned.

vshankar · 2022-08-11T09:38:33Z

@lxbsz I'm seeing fsstresss job failures in teuthology - https://pulpito.ceph.com/vshankar-2022-08-10_04:06:00-fs-wip-vshankar-testing-20220805-190751-testing-default-smithi/6965167/

This is the only MDS related PR with significant changes in the branch. Could you check if the above failure (stalled job) is related to this change?

vshankar · 2022-08-11T09:47:25Z

Another instance - https://pulpito.ceph.com/vshankar-2022-08-10_04:06:00-fs-wip-vshankar-testing-20220805-190751-testing-default-smithi/6965181/

The create call seems to be blocked for ~3hrs

2022-08-10T05:06:07.461 INFO:tasks.workunit.client.1.smithi130.stdout:8/727: dread d16/d17/d3f/d4f/f7b [0,4194304] 0
2022-08-10T05:06:07.464 INFO:tasks.workunit.client.1.smithi130.stdout:8/728: write d16/d6f/d71/d68/d86/fbc [986545,101947] 0
2022-08-10T05:06:07.467 INFO:tasks.workunit.client.1.smithi130.stdout:8/729: creat d16/d6f/d71/d68/d73/ff6 x:0 0 0
2022-08-10T08:03:58.198 DEBUG:teuthology.orchestra.run:got remote process result: 124
2022-08-10T08:04:00.533 DEBUG:teuthology.orchestra.run:got remote process result: 124
2022-08-10T08:04:58.241 DEBUG:teuthology.orchestra.run:timed out waiting; will kill: <Greenlet at 0x7f586ece3448: copy_file_to(<paramiko.ChannelFile from <paramiko.Channel 85 (E, <Logger tasks.workunit.client.1.smithi130.stderr (, None, False)>
2022-08-10T08:05:00.535 DEBUG:teuthology.orchestra.run:timed out waiting; will kill: <Greenlet at 0x7f586ece3e48: copy_file_to(<paramiko.ChannelFile from <paramiko.Channel 88 (E, <Logger tasks.workunit.client.0.smithi061.stderr (, None, False)>
2022-08-10T08:05:58.244 DEBUG:teuthology.orchestra.run:timed out waiting; will kill: <Greenlet at 0x7f586ece3648: copy_file_to(<paramiko.ChannelFile from <paramiko.Channel 85 (E, <Logger tasks.workunit.client.1.smithi130.stdout (, None, False)>
2022-08-10T08:05:58.246 INFO:tasks.workunit:Stopping ['suites/fsstress.sh'] on client.1...

lxbsz · 2022-08-11T12:15:52Z

Another instance - https://pulpito.ceph.com/vshankar-2022-08-10_04:06:00-fs-wip-vshankar-testing-20220805-190751-testing-default-smithi/6965181/

The create call seems to be blocked for ~3hrs

2022-08-10T05:06:07.461 INFO:tasks.workunit.client.1.smithi130.stdout:8/727: dread d16/d17/d3f/d4f/f7b [0,4194304] 0
2022-08-10T05:06:07.464 INFO:tasks.workunit.client.1.smithi130.stdout:8/728: write d16/d6f/d71/d68/d86/fbc [986545,101947] 0
2022-08-10T05:06:07.467 INFO:tasks.workunit.client.1.smithi130.stdout:8/729: creat d16/d6f/d71/d68/d73/ff6 x:0 0 0
2022-08-10T08:03:58.198 DEBUG:teuthology.orchestra.run:got remote process result: 124
2022-08-10T08:04:00.533 DEBUG:teuthology.orchestra.run:got remote process result: 124
2022-08-10T08:04:58.241 DEBUG:teuthology.orchestra.run:timed out waiting; will kill: <Greenlet at 0x7f586ece3448: copy_file_to(<paramiko.ChannelFile from <paramiko.Channel 85 (E, <Logger tasks.workunit.client.1.smithi130.stderr (, None, False)>
2022-08-10T08:05:00.535 DEBUG:teuthology.orchestra.run:timed out waiting; will kill: <Greenlet at 0x7f586ece3e48: copy_file_to(<paramiko.ChannelFile from <paramiko.Channel 88 (E, <Logger tasks.workunit.client.0.smithi061.stderr (, None, False)>
2022-08-10T08:05:58.244 DEBUG:teuthology.orchestra.run:timed out waiting; will kill: <Greenlet at 0x7f586ece3648: copy_file_to(<paramiko.ChannelFile from <paramiko.Channel 85 (E, <Logger tasks.workunit.client.1.smithi130.stdout (, None, False)>
2022-08-10T08:05:58.246 INFO:tasks.workunit:Stopping ['suites/fsstress.sh'] on client.1...

Sure, will check it.

Signed-off-by: Xiubo Li <xiubli@redhat.com>

lxbsz · 2022-08-12T09:39:44Z

@vshankar Found the root cause and have pushed one tmp commit 20c8646. Please review and if it's okay I will squash them.
Locally I can reproduce it and after this fix the test passed, and I am still running the teuthology test.

I'm dropping this change from my branch for now. Will pick it up in my next run.

Sure, I will run some of the teuthology tests those failed in your last run. But I am still hitting the Shaman building failure as usual before. Not sure when that could be fixed.

lxbsz · 2022-08-15T02:03:26Z

fsstress teuthology test passed.

vshankar · 2022-08-25T04:26:41Z

@vshankar Found the root cause and have pushed one tmp commit 20c8646. Please review and if it's okay I will squash them.

Locally I can reproduce it and after this fix the test passed, and I am still running the teuthology test.

Checked the commit and its seems to do the right thing.

vshankar · 2022-09-05T08:10:19Z

Running this through fs suite: https://pulpito.ceph.com/vshankar-2022-09-05_05:44:13-fs-wip-vshankar-testing-20220901-100101-testing-default-smithi/

vshankar · 2022-09-14T09:31:54Z

@lxbsz Could you please take a look at ffsb failure - https://pulpito.ceph.com/vshankar-2022-09-13_09:03:58-fs-wip-vshankar-testing-20220909-120801-testing-default-smithi/7030431/

Excerpt from the log:

022-09-13T09:39:27.816 INFO:tasks.workunit.client.0.smithi087.stdout:checking for main in -lm... yes
2022-09-13T09:39:27.970 INFO:tasks.workunit.client.0.smithi087.stdout:checking for main in -lpthread... yes
2022-09-13T09:39:27.976 INFO:tasks.workunit.client.0.smithi087.stdout:checking for ANSI C header files... (cached) yes
2022-09-13T09:39:28.082 INFO:tasks.workunit.client.0.smithi087.stdout:checking for sys/wait.h that is POSIX.1 compatible... yes
2022-09-13T09:39:31.321 DEBUG:teuthology.orchestra.run.smithi087:> sudo logrotate /etc/logrotate.d/ceph-test.conf
2022-09-13T09:39:31.328 DEBUG:teuthology.orchestra.run.smithi132:> sudo logrotate /etc/logrotate.d/ceph-test.conf

The test times out and no activity is seen post 2022-09-13T09:39:28.082. This is similar to the failure I reported with my last run.

lxbsz · 2022-09-14T10:21:27Z

@lxbsz Could you please take a look at ffsb failure - https://pulpito.ceph.com/vshankar-2022-09-13_09:03:58-fs-wip-vshankar-testing-20220909-120801-testing-default-smithi/7030431/

Excerpt from the log:
022-09-13T09:39:27.816 INFO:tasks.workunit.client.0.smithi087.stdout:checking for main in -lm... yes
2022-09-13T09:39:27.970 INFO:tasks.workunit.client.0.smithi087.stdout:checking for main in -lpthread... yes
2022-09-13T09:39:27.976 INFO:tasks.workunit.client.0.smithi087.stdout:checking for ANSI C header files... (cached) yes
2022-09-13T09:39:28.082 INFO:tasks.workunit.client.0.smithi087.stdout:checking for sys/wait.h that is POSIX.1 compatible... yes
2022-09-13T09:39:31.321 DEBUG:teuthology.orchestra.run.smithi087:> sudo logrotate /etc/logrotate.d/ceph-test.conf
2022-09-13T09:39:31.328 DEBUG:teuthology.orchestra.run.smithi132:> sudo logrotate /etc/logrotate.d/ceph-test.conf
The test times out and no activity is seen post 2022-09-13T09:39:28.082. This is similar to the failure I reported with my last run.

@vshankar I think it should be caused by another PR #40787.

2022-09-13T09:39:28.118+0000 7f7923fff700 20 client.4815 _lookup have 0x10000000002.head["conftest.err"] from mds.-1 ttl 0.000000 seq 0
2022-09-13T09:39:28.118+0000 7f7923fff700  1 client.4815 _lookup dir 0x10000000002.head(faked_ino=0 nref=39 ll_ref=788 cap_refs={} open={} mode=40775 size=0/0 nlink=1 btime=2022-09-13T09:39:00.428798+0000 mtime=2022-09-13T09:39:28.106629+0000 ctime=2022-09-13T09:39:28.106629+0000 caps=pAsLsXsFsx(0=pAsLsXsFsx) parents=0x10000000001.head["ffsb"] 0x7f794400b430) rename is on the way, will wait for dn 'conftest.err'
2022-09-13T09:39:28.909+0000 7f794b7f6700 20 client.4815 tick

Not this one.

vshankar · 2022-09-14T10:27:02Z

@lxbsz Could you please take a look at ffsb failure - https://pulpito.ceph.com/vshankar-2022-09-13_09:03:58-fs-wip-vshankar-testing-20220909-120801-testing-default-smithi/7030431/
Excerpt from the log:
022-09-13T09:39:27.816 INFO:tasks.workunit.client.0.smithi087.stdout:checking for main in -lm... yes
2022-09-13T09:39:27.970 INFO:tasks.workunit.client.0.smithi087.stdout:checking for main in -lpthread... yes
2022-09-13T09:39:27.976 INFO:tasks.workunit.client.0.smithi087.stdout:checking for ANSI C header files... (cached) yes
2022-09-13T09:39:28.082 INFO:tasks.workunit.client.0.smithi087.stdout:checking for sys/wait.h that is POSIX.1 compatible... yes
2022-09-13T09:39:31.321 DEBUG:teuthology.orchestra.run.smithi087:> sudo logrotate /etc/logrotate.d/ceph-test.conf
2022-09-13T09:39:31.328 DEBUG:teuthology.orchestra.run.smithi132:> sudo logrotate /etc/logrotate.d/ceph-test.conf
The test times out and no activity is seen post 2022-09-13T09:39:28.082. This is similar to the failure I reported with my last run.
@vshankar I think it should be caused by another PR #40787.

I commented on that PR -- there are timeouts seen in other tests too..

2022-09-13T09:39:28.118+0000 7f7923fff700 20 client.4815 _lookup have 0x10000000002.head["conftest.err"] from mds.-1 ttl 0.000000 seq 0
2022-09-13T09:39:28.118+0000 7f7923fff700  1 client.4815 _lookup dir 0x10000000002.head(faked_ino=0 nref=39 ll_ref=788 cap_refs={} open={} mode=40775 size=0/0 nlink=1 btime=2022-09-13T09:39:00.428798+0000 mtime=2022-09-13T09:39:28.106629+0000 ctime=2022-09-13T09:39:28.106629+0000 caps=pAsLsXsFsx(0=pAsLsXsFsx) parents=0x10000000001.head["ffsb"] 0x7f794400b430) rename is on the way, will wait for dn 'conftest.err'
2022-09-13T09:39:28.909+0000 7f794b7f6700 20 client.4815 tick

Not this one.

vshankar · 2022-09-14T10:27:53Z

@lxbsz It's probably a good idea to drop #40787 from my branch and test this change. WDYT?

lxbsz · 2022-09-14T11:03:08Z

@lxbsz It's probably a good idea to drop #40787 from my branch and test this change. WDYT?

Sounds good. I will fix that PR and run the test first after that.

vshankar · 2022-09-15T09:27:23Z

@rishabh-d-dave Please include this in your next run.

rishabh-d-dave · 2022-10-10T18:43:05Z

jenkins test make check arm64

rishabh-d-dave · 2022-10-10T18:43:17Z

jenkins test windows

rishabh-d-dave

https://tracker.ceph.com/projects/cephfs/wiki/Main#10-Oct-2022

rishabh-d-dave · 2022-10-10T18:44:29Z

This PR is ready for merge. Waiting for the CI jobs to finish first.

rishabh-d-dave · 2022-10-10T18:59:58Z

Error look irrelevant - https://jenkins.ceph.com/job/ceph-pull-requests-arm64/33426/

rishabh-d-dave · 2022-10-10T19:00:06Z

jenkins test make check arm64

lxbsz · 2022-10-11T00:31:58Z

jenkins test make check arm64

This test is optional , so we can just ignore this IMO.

rishabh-d-dave · 2022-10-11T07:15:46Z

#46331 (review)

https://tracker.ceph.com/projects/cephfs/wiki/Main#10-Oct-2022

Since the QA testing was fine, merging this PR now.

lxbsz requested a review from a team May 19, 2022 02:38

github-actions bot added cephfs Ceph File System core labels May 19, 2022

lxbsz removed the core label May 19, 2022

lxbsz mentioned this pull request May 23, 2022

mds: do not assert early on when issuing client leases #46210

Merged

14 tasks

lxbsz force-pushed the wip-55332 branch from 09e4141 to b6d6a3f Compare May 23, 2022 07:25

github-actions bot added the core label May 23, 2022

mchangir reviewed May 24, 2022

View reviewed changes

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

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

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

vshankar requested a review from a team May 25, 2022 01:44

djgalloway changed the base branch from master to main May 25, 2022 19:57

lxbsz force-pushed the wip-55332 branch from b6d6a3f to b640f94 Compare May 26, 2022 04:36

vshankar self-assigned this Jun 16, 2022

vshankar reviewed Jun 23, 2022

View reviewed changes

lxbsz force-pushed the wip-55332 branch from b640f94 to aa966cc Compare June 24, 2022 01:12

lxbsz requested a review from a team as a code owner June 24, 2022 01:12

lxbsz force-pushed the wip-55332 branch from aa966cc to 4fe2264 Compare June 24, 2022 05:30

lxbsz requested review from vshankar and mchangir June 29, 2022 07:00

vshankar added the wip-vshankar-testing1 label Jul 5, 2022

vshankar added wip-vshankar-testing and removed wip-vshankar-testing1 labels Jul 14, 2022

lxbsz force-pushed the wip-55332 branch from 4fe2264 to 20c8646 Compare August 12, 2022 00:46

github-actions bot added build/ops tests labels Aug 12, 2022

mds: clear the STATE_UNLINKING state when the unlink fails

417f247

Signed-off-by: Xiubo Li <xiubli@redhat.com>

lxbsz force-pushed the wip-55332 branch from 20c8646 to 417f247 Compare August 12, 2022 09:37

lxbsz added the wip-lxb-testing label Aug 12, 2022

lxbsz removed the wip-lxb-testing label Aug 23, 2022

vshankar added the wip-vshankar-testing label Aug 25, 2022

vshankar added wip-rishabh-testing Rishabh's testing label and removed wip-vshankar-testing labels Sep 15, 2022

rishabh-d-dave approved these changes Oct 10, 2022

View reviewed changes

rishabh-d-dave merged commit 7d781fb into ceph:main Oct 11, 2022

This was referenced Oct 12, 2022

quincy: mds: wait unlink to finish to avoid conflict when creating same dentries #48452

Merged

pacific: mds: wait unlink to finish to avoid conflict when creating same dentries #48453

Merged

orange30 mentioned this pull request Jan 6, 2023

mds：when want_auth is true, path_traverse should ensure the return Inode is auth. #49652

Closed

14 tasks

lxbsz mentioned this pull request Sep 12, 2023

mds: fix deadlock between unlinking and linkmerge #52199

Merged

14 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mds: wait unlink to finish to avoid conflict when creating same dentries #46331

mds: wait unlink to finish to avoid conflict when creating same dentries #46331

lxbsz commented May 19, 2022 •

edited by joscollin

mchangir left a comment •

edited

joscollin commented Jun 9, 2022

vshankar commented Jun 29, 2022

lxbsz commented Jun 29, 2022

vshankar commented Aug 11, 2022

vshankar commented Aug 11, 2022

lxbsz commented Aug 11, 2022

lxbsz commented Aug 12, 2022

lxbsz commented Aug 15, 2022 •

edited

vshankar commented Aug 25, 2022

vshankar commented Sep 5, 2022

vshankar commented Sep 14, 2022

lxbsz commented Sep 14, 2022

vshankar commented Sep 14, 2022

vshankar commented Sep 14, 2022

lxbsz commented Sep 14, 2022

vshankar commented Sep 15, 2022

rishabh-d-dave commented Oct 10, 2022

rishabh-d-dave commented Oct 10, 2022

rishabh-d-dave left a comment

rishabh-d-dave commented Oct 10, 2022

rishabh-d-dave commented Oct 10, 2022

rishabh-d-dave commented Oct 10, 2022

lxbsz commented Oct 11, 2022

rishabh-d-dave commented Oct 11, 2022

mds: wait unlink to finish to avoid conflict when creating same dentries #46331

mds: wait unlink to finish to avoid conflict when creating same dentries #46331

Conversation

lxbsz commented May 19, 2022 • edited by joscollin

Contribution Guidelines

Checklist

mchangir left a comment • edited

Choose a reason for hiding this comment

joscollin commented Jun 9, 2022

vshankar commented Jun 29, 2022

lxbsz commented Jun 29, 2022

vshankar commented Aug 11, 2022

vshankar commented Aug 11, 2022

lxbsz commented Aug 11, 2022

lxbsz commented Aug 12, 2022

lxbsz commented Aug 15, 2022 • edited

vshankar commented Aug 25, 2022

vshankar commented Sep 5, 2022

vshankar commented Sep 14, 2022

lxbsz commented Sep 14, 2022

vshankar commented Sep 14, 2022

vshankar commented Sep 14, 2022

lxbsz commented Sep 14, 2022

vshankar commented Sep 15, 2022

rishabh-d-dave commented Oct 10, 2022

rishabh-d-dave commented Oct 10, 2022

rishabh-d-dave left a comment

Choose a reason for hiding this comment

rishabh-d-dave commented Oct 10, 2022

rishabh-d-dave commented Oct 10, 2022

rishabh-d-dave commented Oct 10, 2022

lxbsz commented Oct 11, 2022

rishabh-d-dave commented Oct 11, 2022

lxbsz commented May 19, 2022 •

edited by joscollin

mchangir left a comment •

edited

lxbsz commented Aug 15, 2022 •

edited