mds: drop locks and retry when lock set changes by batrick · Pull Request #52522 · ceph/ceph

batrick · 2023-07-18T14:04:44Z

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

An optimization was added to avoid an unnecessary gather on the inode filelock when the client can safely get the file size without also getting issued the requested caps. However, if a retry of getattr is necessary, this conditional inclusion of the inode filelock can cause lock-order violations resulting in deadlock. So, if we've already acquired some of the inode's locks then we must drop locks and retry. Fixes: https://tracker.ceph.com/issues/62052 Fixes: c822b3e Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

batrick · 2023-07-19T16:12:32Z

Pulled debugging commits out into: #52547

src/mds/Server.cc

batrick · 2023-08-03T13:00:58Z

jenkins test make check arm64

batrick · 2023-08-03T13:01:16Z

jenkins test windows

vshankar · 2023-08-09T12:24:36Z

jenkins test make check

vshankar · 2023-08-14T05:49:38Z

Test runs in ~2h: https://shaman.ceph.com/builds/ceph/wip-vshankar-testing-20230814.052859/

vshankar · 2023-08-14T05:49:54Z

jenkins test make check

vshankar · 2023-08-14T10:50:26Z

Test runs in ~2h: https://shaman.ceph.com/builds/ceph/wip-vshankar-testing-20230814.052859/

Build did not go well.

vshankar · 2023-08-16T11:20:38Z

https://pulpito.ceph.com/vshankar-2023-08-16_10:55:44-fs-wip-vshankar-testing-20230816.054905-testing-default-smithi/

batrick · 2023-08-16T13:36:44Z

jenkins test make check arm64

vshankar · 2023-08-17T05:44:36Z

https://pulpito.ceph.com/vshankar-2023-08-16_10:55:44-fs-wip-vshankar-testing-20230816.054905-testing-default-smithi/

The run was messy - lot many failures due to POOL_APP_NOT_ENABLED warnings (26/52 failed jobs). Some fs related test failure which needs to be looked at are

https://pulpito.ceph.com/vshankar-2023-08-16_10:55:44-fs-wip-vshankar-testing-20230816.054905-testing-default-smithi/7369749 ("test_filesystem_sync_stuck_for_around_5s")
https://pulpito.ceph.com/vshankar-2023-08-16_10:55:44-fs-wip-vshankar-testing-20230816.054905-testing-default-smithi/7369756 ("test_volume_rm")
https://pulpito.ceph.com/vshankar-2023-08-16_10:55:44-fs-wip-vshankar-testing-20230816.054905-testing-default-smithi/7369694 ("test_snapshot_remove") -- this should have been fixed by mds: remove calculating caps after adding revokes back #52176, but it reappears.

batrick · 2023-08-17T13:29:13Z

https://pulpito.ceph.com/vshankar-2023-08-16_10:55:44-fs-wip-vshankar-testing-20230816.054905-testing-default-smithi/

The run was messy - lot many failures due to POOL_APP_NOT_ENABLED warnings (26/52 failed jobs). Some fs related test failure which needs to be looked at are

#53037

* https://pulpito.ceph.com/vshankar-2023-08-16_10:55:44-fs-wip-vshankar-testing-20230816.054905-testing-default-smithi/7369749 ("test_filesystem_sync_stuck_for_around_5s")

* https://pulpito.ceph.com/vshankar-2023-08-16_10:55:44-fs-wip-vshankar-testing-20230816.054905-testing-default-smithi/7369756 ("test_volume_rm")

* https://pulpito.ceph.com/vshankar-2023-08-16_10:55:44-fs-wip-vshankar-testing-20230816.054905-testing-default-smithi/7369694 ("test_snapshot_remove") -- this should have been fixed by [mds: remove calculating caps after adding revokes back #52176](https://github.com/ceph/ceph/pull/52176), but it reappears.

Are you asking me to look at those more closely or ... ?

vshankar · 2023-08-17T13:33:31Z

https://pulpito.ceph.com/vshankar-2023-08-16_10:55:44-fs-wip-vshankar-testing-20230816.054905-testing-default-smithi/

The run was messy - lot many failures due to POOL_APP_NOT_ENABLED warnings (26/52 failed jobs). Some fs related test failure which needs to be looked at are

#53037
* https://pulpito.ceph.com/vshankar-2023-08-16_10:55:44-fs-wip-vshankar-testing-20230816.054905-testing-default-smithi/7369749 ("test_filesystem_sync_stuck_for_around_5s")

* https://pulpito.ceph.com/vshankar-2023-08-16_10:55:44-fs-wip-vshankar-testing-20230816.054905-testing-default-smithi/7369756 ("test_volume_rm")

* https://pulpito.ceph.com/vshankar-2023-08-16_10:55:44-fs-wip-vshankar-testing-20230816.054905-testing-default-smithi/7369694 ("test_snapshot_remove") -- this should have been fixed by [mds: remove calculating caps after adding revokes back #52176](https://github.com/ceph/ceph/pull/52176), but it reappears.
Are you asking me to look at those more closely or ... ?

I'll have a look maybe tomorrow or next week, so I just updated here with the details so that I don't loose them. Feel free to peek and debug if time permits.

vshankar · 2023-08-28T15:45:37Z

https://pulpito.ceph.com/vshankar-2023-08-16_10:55:44-fs-wip-vshankar-testing-20230816.054905-testing-default-smithi/

The run was messy - lot many failures due to POOL_APP_NOT_ENABLED warnings (26/52 failed jobs). Some fs related test failure which needs to be looked at are

#53037
* https://pulpito.ceph.com/vshankar-2023-08-16_10:55:44-fs-wip-vshankar-testing-20230816.054905-testing-default-smithi/7369749 ("test_filesystem_sync_stuck_for_around_5s")

* https://pulpito.ceph.com/vshankar-2023-08-16_10:55:44-fs-wip-vshankar-testing-20230816.054905-testing-default-smithi/7369756 ("test_volume_rm")

* https://pulpito.ceph.com/vshankar-2023-08-16_10:55:44-fs-wip-vshankar-testing-20230816.054905-testing-default-smithi/7369694 ("test_snapshot_remove") -- this should have been fixed by [mds: remove calculating caps after adding revokes back #52176](https://github.com/ceph/ceph/pull/52176), but it reappears.
Are you asking me to look at those more closely or ... ?
I'll have a look maybe tomorrow or next week, so I just updated here with the details so that I don't loose them. Feel free to peek and debug if time permits.

@batrick - I couldn't get to reviewing this last Friday. Would you be able to look at the failures (which seem a bit high as compared to other fs suite runs and this is the only PR in the branch)?

batrick · 2023-08-29T01:06:54Z

I will try to have a look tomorrow @vshankar

batrick · 2023-08-30T23:43:52Z

https://pulpito.ceph.com/vshankar-2023-08-16_10:55:44-fs-wip-vshankar-testing-20230816.054905-testing-default-smithi/

The run was messy - lot many failures due to POOL_APP_NOT_ENABLED warnings (26/52 failed jobs). Some fs related test failure which needs to be looked at are
* https://pulpito.ceph.com/vshankar-2023-08-16_10:55:44-fs-wip-vshankar-testing-20230816.054905-testing-default-smithi/7369749 ("test_filesystem_sync_stuck_for_around_5s")

Infrastructure noise I think.

* https://pulpito.ceph.com/vshankar-2023-08-16_10:55:44-fs-wip-vshankar-testing-20230816.054905-testing-default-smithi/7369756 ("test_volume_rm")

https://tracker.ceph.com/issues/62648

* https://pulpito.ceph.com/vshankar-2023-08-16_10:55:44-fs-wip-vshankar-testing-20230816.054905-testing-default-smithi/7369694 ("test_snapshot_remove") -- this should have been fixed by [mds: remove calculating caps after adding revokes back #52176](https://github.com/ceph/ceph/pull/52176), but it reappears.

https://tracker.ceph.com/issues/62580

I think it's a testing kernel bug.

Do you think this is good to merge otherwise?

* refs/pull/52522/head: mds: drop locks and retry when lock set changes

vshankar · 2023-08-31T05:17:28Z

https://tracker.ceph.com/issues/62580

I think it's a testing kernel bug.

ACK. Spoke to @lxbsz too regarding this.

lxbsz · 2023-08-31T05:32:45Z

https://tracker.ceph.com/issues/62580
I think it's a testing kernel bug.

ACK. Spoke to @lxbsz too regarding this.

Venky, it's a known issue and I have fixed it but not backport to Pacific yet.

lxbsz · 2023-08-31T05:33:07Z

jenkins test make check arm64

vshankar · 2023-08-31T06:40:35Z

https://pulpito.ceph.com/vshankar-2023-08-16_10:55:44-fs-wip-vshankar-testing-20230816.054905-testing-default-smithi/

The run was messy - lot many failures due to POOL_APP_NOT_ENABLED warnings (26/52 failed jobs). Some fs related test failure which needs to be looked at are
* https://pulpito.ceph.com/vshankar-2023-08-16_10:55:44-fs-wip-vshankar-testing-20230816.054905-testing-default-smithi/7369749 ("test_filesystem_sync_stuck_for_around_5s")
Infrastructure noise I think.

yeh - ConnectionError: Failed to reconnect to smithi193

lxbsz · 2023-08-31T08:34:13Z

https://tracker.ceph.com/issues/62580
I think it's a testing kernel bug.

ACK. Spoke to @lxbsz too regarding this.

Venky, it's a known issue and I have fixed it but not backport to Pacific yet.

Sorry, It's just a similar issue, not the exactly the same one, but I don't think it's related to this PR. I will update the PR later for my finding.

lxbsz · 2023-08-31T09:42:11Z

https://tracker.ceph.com/issues/62580
I think it's a testing kernel bug.

ACK. Spoke to @lxbsz too regarding this.

Venky, it's a known issue and I have fixed it but not backport to Pacific yet.

Sorry, It's just a similar issue, not the exactly the same one, but I don't think it's related to this PR. I will update the PR later for my finding.

Fixed it in MDS side #53238.

vshankar · 2023-08-31T11:53:26Z

Merging this in the interest of time - will prepare run wiki asap.

batrick added bug-fix cephfs Ceph File System needs-review labels Jul 18, 2023

batrick force-pushed the i62052 branch 2 times, most recently from 5d7949c to 6df48d8 Compare July 19, 2023 16:06

batrick force-pushed the i62052 branch from 6df48d8 to b5719ac Compare July 19, 2023 16:12

batrick requested a review from a team July 19, 2023 19:01

lxbsz reviewed Jul 20, 2023

View reviewed changes

src/mds/Server.cc Show resolved Hide resolved

lxbsz mentioned this pull request Aug 2, 2023

mds: add protection from clients without fscrypt support #45073

Closed

vshankar added the wip-vshankar-testing3 label Aug 9, 2023

batrick added the wip-pdonnell-testing label Aug 30, 2023

batrick added a commit to batrick/ceph that referenced this pull request Aug 30, 2023

Merge PR ceph#52522 into wip-batrick-testing-20230830.165602

b3b86a8

* refs/pull/52522/head: mds: drop locks and retry when lock set changes

vshankar approved these changes Aug 31, 2023

View reviewed changes

vshankar merged commit e1adcae into ceph:main Aug 31, 2023

vshankar removed the wip-vshankar-testing3 label Aug 31, 2023

batrick deleted the i62052 branch August 31, 2023 12:32

This was referenced Aug 31, 2023

reef: mds: drop locks and retry when lock set changes #53241

Merged

quincy: mds: drop locks and retry when lock set changes #53242

Merged

pacific: mds: drop locks and retry when lock set changes #53243

Merged

lxbsz mentioned this pull request Apr 12, 2024

mds: adjust the locks order #56805

Closed

14 tasks

Conversation

batrick commented Jul 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution Guidelines

Checklist

Uh oh!

batrick commented Jul 19, 2023

Uh oh!

Uh oh!

batrick commented Aug 3, 2023

Uh oh!

batrick commented Aug 3, 2023

Uh oh!

vshankar commented Aug 9, 2023

Uh oh!

vshankar commented Aug 14, 2023

Uh oh!

vshankar commented Aug 14, 2023

Uh oh!

vshankar commented Aug 14, 2023

Uh oh!

vshankar commented Aug 16, 2023

Uh oh!

batrick commented Aug 16, 2023

Uh oh!

vshankar commented Aug 17, 2023

Uh oh!

batrick commented Aug 17, 2023

Uh oh!

vshankar commented Aug 17, 2023

Uh oh!

vshankar commented Aug 28, 2023

Uh oh!

batrick commented Aug 29, 2023

Uh oh!

batrick commented Aug 30, 2023

Uh oh!

vshankar commented Aug 31, 2023

Uh oh!

lxbsz commented Aug 31, 2023

Uh oh!

lxbsz commented Aug 31, 2023

Uh oh!

vshankar commented Aug 31, 2023

Uh oh!

lxbsz commented Aug 31, 2023

Uh oh!

lxbsz commented Aug 31, 2023

Uh oh!

vshankar commented Aug 31, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

batrick commented Jul 18, 2023 •

edited

Loading