mds: wait for unlink operation to finish by lxbsz · Pull Request #47399 · ceph/ceph

lxbsz · 2022-08-02T06:28:32Z

If the previous unlink is not finished yet but the corressponding
inode has been projected and it dentry parent has been changed to
stray. The new comming link request will fail with -EXDEV.

Just wait the previous unlink to finish and unlink the dn from
parent directory.

https://tracker.ceph.com/issues/56695
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

src/mds/CInode.h

src/mds/Server.cc

batrick

Well done tracking that down!!

vshankar · 2022-08-18T12:27:57Z

@lxbsz This change seems to be causing fsstress.sh failures here: https://pulpito.ceph.com/vshankar-2022-08-12_09:34:24-fs-wip-vshankar-testing1-20220812-072441-testing-default-smithi/ (check for fsstress.sh failures)

Without this changes, there are no such failures: https://pulpito.ceph.com/vshankar-2022-08-18_04:30:42-fs-wip-vshankar-testing1-20220818-082047-testing-default-smithi/

Please take a look.

lxbsz · 2022-08-18T12:57:12Z

@lxbsz This change seems to be causing fsstress.sh failures here: https://pulpito.ceph.com/vshankar-2022-08-12_09:34:24-fs-wip-vshankar-testing1-20220812-072441-testing-default-smithi/ (check for fsstress.sh failures)

Without this changes, there are no such failures: https://pulpito.ceph.com/vshankar-2022-08-18_04:30:42-fs-wip-vshankar-testing1-20220818-082047-testing-default-smithi/

Please take a look.

We need to check the strayin to make sure it's not nullptr when dereferencing it.

Fixed it and I will run the fsstress.sh qa tests.

lxbsz · 2022-08-19T01:25:44Z

All the fsstress.sh test passed:

fs:upgrade:featureful_client:upgraded_client
fs:upgrade:featureful_client:old_client
fs:upgrade:upgrade_without_reducing_max_mds

vshankar · 2022-08-19T04:13:17Z

need to check the strayin to make sure it's not nullptr when dereferencing it.

Will review it today.

vshankar · 2023-03-07T09:45:17Z

@lxbsz I'm seeing failures which should have been fixed by this change. See: https://pulpito.ceph.com/vshankar-2023-03-07_05:15:12-fs-wip-vshankar-testing-20230307.030510-testing-default-smithi/7195941/

2023-03-07T06:05:57.980 INFO:tasks.workunit.client.0.smithi088.stderr:+ expect 0 link fstest_6f263add3ff59b7ee938215207ed44e0 _123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123
456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_1234/_123456789_123456789_123456789_1234567
89_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_1234567
89_1234/_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_1
23456789_123456789_123456789_123456789_123456789_1234/_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_12345
6789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_12/x
2023-03-07T06:05:57.980 INFO:tasks.workunit.client.0.smithi088.stderr:+ e=0
2023-03-07T06:05:57.980 INFO:tasks.workunit.client.0.smithi088.stderr:+ shift
2023-03-07T06:05:57.980 INFO:tasks.workunit.client.0.smithi088.stderr:++ /home/ubuntu/cephtest/mnt.0/client.0/tmp/tmp/../pjd-fstest-20090130-RC/tests/link/../../fstest link fstest_6f263add3ff59b7ee938215207ed44
e0 _123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456
789_123456789_123456789_123456789_123456789_1234/_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_
123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_1234/_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_1234
56789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_1234/_123456789_123456789_123456789_123456789_123456789_123456789_12345678
9_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_12/x
2023-03-07T06:05:57.980 INFO:tasks.workunit.client.0.smithi088.stderr:++ tail -1
2023-03-07T06:05:57.981 INFO:tasks.workunit.client.0.smithi088.stderr:+ r=EXDEV
2023-03-07T06:05:57.981 INFO:tasks.workunit.client.0.smithi088.stderr:+ echo EXDEV
2023-03-07T06:05:57.981 INFO:tasks.workunit.client.0.smithi088.stderr:+ egrep '^0$'
2023-03-07T06:05:57.981 INFO:tasks.workunit.client.0.smithi088.stderr:+ '[' 1 -eq 0 ']'
2023-03-07T06:05:57.981 INFO:tasks.workunit.client.0.smithi088.stderr:+ echo 'not ok 8'
2023-03-07T06:05:57.981 INFO:tasks.workunit.client.0.smithi088.stderr:++ expr 8 + 1
2023-03-07T06:05:57.981 INFO:tasks.workunit.client.0.smithi088.stderr:+ ntest=9

lxbsz · 2023-03-07T13:44:00Z

@lxbsz I'm seeing failures which should have been fixed by this change. See: https://pulpito.ceph.com/vshankar-2023-03-07_05:15:12-fs-wip-vshankar-testing-20230307.030510-testing-default-smithi/7195941/

2023-03-07T06:05:57.980 INFO:tasks.workunit.client.0.smithi088.stderr:+ expect 0 link fstest_6f263add3ff59b7ee938215207ed44e0 _123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123
456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_1234/_123456789_123456789_123456789_1234567
89_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_1234567
89_1234/_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_1
23456789_123456789_123456789_123456789_123456789_1234/_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_12345
6789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_12/x
2023-03-07T06:05:57.980 INFO:tasks.workunit.client.0.smithi088.stderr:+ e=0
2023-03-07T06:05:57.980 INFO:tasks.workunit.client.0.smithi088.stderr:+ shift
2023-03-07T06:05:57.980 INFO:tasks.workunit.client.0.smithi088.stderr:++ /home/ubuntu/cephtest/mnt.0/client.0/tmp/tmp/../pjd-fstest-20090130-RC/tests/link/../../fstest link fstest_6f263add3ff59b7ee938215207ed44
e0 _123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456
789_123456789_123456789_123456789_123456789_1234/_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_
123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_1234/_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_1234
56789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_1234/_123456789_123456789_123456789_123456789_123456789_123456789_12345678
9_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_12/x
2023-03-07T06:05:57.980 INFO:tasks.workunit.client.0.smithi088.stderr:++ tail -1
2023-03-07T06:05:57.981 INFO:tasks.workunit.client.0.smithi088.stderr:+ r=EXDEV
2023-03-07T06:05:57.981 INFO:tasks.workunit.client.0.smithi088.stderr:+ echo EXDEV
2023-03-07T06:05:57.981 INFO:tasks.workunit.client.0.smithi088.stderr:+ egrep '^0$'
2023-03-07T06:05:57.981 INFO:tasks.workunit.client.0.smithi088.stderr:+ '[' 1 -eq 0 ']'
2023-03-07T06:05:57.981 INFO:tasks.workunit.client.0.smithi088.stderr:+ echo 'not ok 8'
2023-03-07T06:05:57.981 INFO:tasks.workunit.client.0.smithi088.stderr:++ expr 8 + 1
2023-03-07T06:05:57.981 INFO:tasks.workunit.client.0.smithi088.stderr:+ ntest=9

Okay, I will check this tomorrow. Thanks.

src/mds/Server.cc

lxbsz · 2023-03-14T05:00:49Z

We need to drop the locks before queue it into waiter list:

+ mds->locker->drop_locks(mdr.get());

I am running the test.

@vshankar Let me run the pjd tests first before you run more tests. Will ping you after it succeeds.

In the else scope the remote_dn must be non-auth. https://tracker.ceph.com/issues/56695 Signed-off-by: Xiubo Li <xiubli@redhat.com>

lxbsz · 2023-03-16T04:39:42Z

Updated it. We need to wait for the unlink to finish until the stray dentry to be unlinked from the corresponding inode, or we still could hit the same issue.

There have 3 cases:
(1), the nlink > 1 and the other links dentries are also in local mds, we need to wait the linkmerge to finish.
(2), the nlink > 1 and the other link dentries are in other mds, we need to wait the migrate to finish
(3), the nlink = 1 but the clients pass one invalidate ino, we will try to wake up the waiter after the stray dentry get purged.

I am still testing it.

If one inode has more than one link and after one of its dentries being unlinked it will be moved to stray directory. Before the linkmerge/migrate finises if a link request comes it will fail with -EXDEV. While in non-multiple link case it's also possible that the clients could pass one invalidate ino, which is still under unlinking. Just wait the linkmerge/migrate or purge to finish. https://tracker.ceph.com/issues/56695 Signed-off-by: Xiubo Li <xiubli@redhat.com>

lxbsz · 2023-03-17T08:54:22Z

@vshankar The pjd.sh test itself passed in my tests. But the qa infra seems having other irrelevant issues and took a long time and died or failed.

I have also tried to run the same tests by reverting these patches in this PR still have the same issue and took a long time, till now one has died and the others still running.

I am not sure whether my qa test commands' issue or not. Could you add this into your next test ?

vshankar · 2023-03-17T10:31:51Z

@vshankar The pjd.sh test itself passed in my tests. But the qa infra seems having other irrelevant issues and took a long time and died or failed.

I have also tried to run the same tests by reverting these patches in this PR still have the same issue and took a long time, till now one has died and the others still running.

I am not sure whether my qa test commands' issue or not. Could you add this into your next test ?

Sure. I think your change in #47399 (comment) would fix the pending issue. I'll include this change in my next run.

lxbsz · 2023-03-23T05:01:15Z

@vshankar The pjd.sh test itself passed in my tests. But the qa infra seems having other irrelevant issues and took a long time and died or failed.
I have also tried to run the same tests by reverting these patches in this PR still have the same issue and took a long time, till now one has died and the others still running.
I am not sure whether my qa test commands' issue or not. Could you add this into your next test ?

Sure. I think your change in #47399 (comment) would fix the pending issue. I'll include this change in my next run.

Today I checked the qa test failures again carefully. All the pjd.sh test passed and the client was also successfully unmounted very fast. But when tearing down the ceph cluster it was stuck when removing the osd.6 daemon. The MDS, MON, MGR and OSD.{0~5} daemons were all successfully destroyed without any issue before that.

lxbsz · 2023-03-23T05:45:08Z

destroyed

@vshankar The pjd.sh test itself passed in my tests. But the qa infra seems having other irrelevant issues and took a long time and died or failed.
I have also tried to run the same tests by reverting these patches in this PR still have the same issue and took a long time, till now one has died and the others still running.
I am not sure whether my qa test commands' issue or not. Could you add this into your next test ?

Sure. I think your change in #47399 (comment) would fix the pending issue. I'll include this change in my next run.

Today I checked the qa test failures again carefully. All the pjd.sh test passed and the client was also successfully unmounted very fast. But when tearing down the ceph cluster it was stuck when removing the osd.6 daemon. The MDS, MON, MGR and OSD.{0~5} daemons were all successfully destroyed without any issue before that.

Run the tests again twice by pulling the latest teuthology repo the tests passed:

https://pulpito.ceph.com/xiubli-2023-03-23_04:49:00-fs:workload-wip-20230316-1328-unlink-distro-default-smithi/
https://pulpito.ceph.com/xiubli-2023-03-23_04:47:23-fs:workload-wip-20230316-1328-unlink-distro-default-smithi/

vshankar · 2023-04-03T09:07:06Z

fs suite run: https://pulpito.ceph.com/?branch=wip-vshankar-testing-20230330.105356

(test run review is in progress - will update once done)

vshankar · 2023-04-05T07:38:52Z

fs suite run: https://pulpito.ceph.com/?branch=wip-vshankar-testing-20230330.105356

(test run review is in progress - will update once done)

The pjd failures are fixed - nice work @lxbsz

vshankar · 2023-04-05T07:39:03Z

jenkins test windows

vshankar · 2023-04-06T04:18:17Z

jenkins make check arm64

lxbsz · 2023-04-06T04:33:40Z

jenkins make check arm64

lxbsz · 2023-04-06T04:34:23Z

jenkins test make check arm64

vshankar · 2023-04-06T12:24:44Z

failed job rerun: https://pulpito.ceph.com/vshankar-2023-04-06_04:14:11-fs-wip-vshankar-testing-20230330.105356-testing-default-smithi/

vshankar

https://tracker.ceph.com/projects/cephfs/wiki/Main#10-Apr-2023

* refs/pull/47399/head: mds: wait the linkmerge/migrate to finish after unlink mds: remove false is_auth() check for remote_dn Reviewed-by: Ramana Raja <rraja@redhat.com> Reviewed-by: Venky Shankar <vshankar@redhat.com> Reviewed-by: Patrick Donnelly <pdonnell@redhat.com> Reviewed-by: Rishabh Dave <ridave@redhat.com>

lxbsz requested a review from a team August 2, 2022 06:28

github-actions bot added the cephfs Ceph File System label Aug 2, 2022

lxbsz force-pushed the wip-56695 branch from 0244d60 to 7fa0d91 Compare August 3, 2022 01:26

kotreshhr reviewed Aug 3, 2022

View reviewed changes

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

lxbsz force-pushed the wip-56695 branch 2 times, most recently from 32ec0c4 to aabf691 Compare August 3, 2022 04:16

ajarr reviewed Aug 3, 2022

View reviewed changes

src/mds/Server.cc Show resolved Hide resolved

lxbsz force-pushed the wip-56695 branch from aabf691 to 3873761 Compare August 4, 2022 01:07

ajarr approved these changes Aug 4, 2022

View reviewed changes

batrick approved these changes Aug 4, 2022

View reviewed changes

vshankar added the wip-vshankar-testing1 label Aug 11, 2022

vshankar changed the title ~~mds: wait the unlink to finish~~ mds: wait for unlink operation to finish Aug 11, 2022

lxbsz force-pushed the wip-56695 branch from 3873761 to 4fd0f66 Compare August 18, 2022 12:56

vshankar removed the wip-vshankar-testing1 label Aug 19, 2022

rishabh-d-dave approved these changes Nov 18, 2022

View reviewed changes

vshankar approved these changes Dec 6, 2022

View reviewed changes

vshankar added the wip-vshankar-testing1 label Dec 6, 2022

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

vshankar added wip-vshankar-backlog Backlog of CephFS PRs to pick for testing and removed wip-vshankar-testing1 labels Mar 7, 2023

lxbsz commented Mar 8, 2023

View reviewed changes

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

lxbsz force-pushed the wip-56695 branch from 4fd0f66 to 554753e Compare March 8, 2023 04:32

lxbsz requested a review from vshankar March 8, 2023 04:36

vshankar removed the wip-vshankar-backlog Backlog of CephFS PRs to pick for testing label Mar 8, 2023

vshankar added wip-vshankar-testing2 and removed wip-vshankar-backlog Backlog of CephFS PRs to pick for testing labels Mar 13, 2023

lxbsz force-pushed the wip-56695 branch from 9ef8636 to 6cb8f06 Compare March 14, 2023 04:57

mds: remove false is_auth() check for remote_dn

400dfef

In the else scope the remote_dn must be non-auth. https://tracker.ceph.com/issues/56695 Signed-off-by: Xiubo Li <xiubli@redhat.com>

lxbsz force-pushed the wip-56695 branch 3 times, most recently from 7e7f7c6 to 9506b4d Compare March 16, 2023 04:33

lxbsz force-pushed the wip-56695 branch from 9506b4d to 48f9a89 Compare March 16, 2023 11:19

vshankar approved these changes Apr 10, 2023

View reviewed changes

vshankar merged commit afb6b17 into ceph:main Apr 10, 2023

vshankar removed the wip-vshankar-testing2 label Apr 10, 2023

This was referenced Apr 11, 2023

reef: mds: wait for unlink operation to finish #50984

Merged

quincy: mds: wait for unlink operation to finish #50985

Merged

pacific: mds: wait for unlink operation to finish #50986

Merged

batrick mentioned this pull request Aug 29, 2023

mds: fix deadlock between unlinking and linkmerge #52199

Merged

14 tasks

Conversation

lxbsz commented Aug 2, 2022

Contribution Guidelines

Checklist

Uh oh!

Uh oh!

Uh oh!

batrick left a comment

Choose a reason for hiding this comment

Uh oh!

vshankar commented Aug 18, 2022

Uh oh!

lxbsz commented Aug 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lxbsz commented Aug 19, 2022

Uh oh!

vshankar commented Aug 19, 2022

Uh oh!

vshankar commented Mar 7, 2023

Uh oh!

lxbsz commented Mar 7, 2023

Uh oh!

Uh oh!

lxbsz commented Mar 14, 2023

Uh oh!

lxbsz commented Mar 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lxbsz commented Mar 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vshankar commented Mar 17, 2023 • edited by lxbsz Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lxbsz commented Mar 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lxbsz commented Mar 23, 2023

Uh oh!

vshankar commented Apr 3, 2023

Uh oh!

vshankar commented Apr 5, 2023

Uh oh!

vshankar commented Apr 5, 2023

Uh oh!

vshankar commented Apr 6, 2023

Uh oh!

lxbsz commented Apr 6, 2023

Uh oh!

lxbsz commented Apr 6, 2023

Uh oh!

vshankar commented Apr 6, 2023

Uh oh!

vshankar left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

lxbsz commented Aug 18, 2022 •

edited

Loading

lxbsz commented Mar 16, 2023 •

edited

Loading

lxbsz commented Mar 17, 2023 •

edited

Loading

vshankar commented Mar 17, 2023 •

edited by lxbsz

Loading

lxbsz commented Mar 23, 2023 •

edited

Loading