Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

mds: preparation for async dir operation support #30972

Merged
merged 12 commits into from Nov 25, 2019
Merged

mds: preparation for async dir operation support #30972

merged 12 commits into from Nov 25, 2019

Conversation

ukernel
Copy link
Contributor

@ukernel ukernel commented Oct 17, 2019

cleanup and preparation pathes for #27866

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 crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard backend
  • jenkins test docs
  • jenkins render docs

@ukernel ukernel added the cephfs Ceph File System label Oct 17, 2019
@ukernel ukernel mentioned this pull request Oct 21, 2019
Merged
3 tasks
@jtlayton jtlayton self-requested a review October 21, 2019 11:12
@jtlayton
Copy link
Contributor

jenkins test make check

@jtlayton
Copy link
Contributor

jenkins test make check arm64

@jtlayton jtlayton requested a review from batrick October 21, 2019 11:52
jtlayton
jtlayton approved these changes Oct 29, 2019
View reviewed changes
Copy link
Contributor

@jtlayton jtlayton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked over the patches and they seem sane enough, though I'm not familiar enough with the MDS code to review this very well. I have been testing with the larger async MDS patch series, and I haven't seen any regressions with the most recent set.

I move that we go ahead and get this merged soon, so it has some time to soak and run full QA cycles.

@batrick
Copy link
Member

batrick commented Nov 5, 2019

@ukernel I'm going to run this through QA ASAP. Needs rebased though.

@ukernel
Copy link
Contributor Author

ukernel commented Nov 5, 2019

updated

@jtlayton
Copy link
Contributor

jtlayton commented Nov 8, 2019

jenkins retest this please

@ukernel
mds: change MDCache::path_traverse()'s 'onfail' argument to flags
e9f94dd 
Later pathes will introduce more flags

Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
@ukernel
mds: cleanup dentry non-readable check in MDCache::path_traverse()
abad03c 
unify non-readable checks of null dentry and non-null dentry.

Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
@ukernel
mds: explictly specify if MDCache::path_traverse() needs to check nul…
7b8e289 
…l dentry

Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
@ukernel
mds: introduce Locker::wrlock_try()
06aca48 
So MDCache::predirty_journal_parents can avoid casting MutationRef
to MDRequestImpl.

Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
@ukernel
mds: cleanup Locker::acquire_locks()
81f4e9c 
define varible 'lock' in the loop that acquires locks

Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
@ukernel
mds: define operator<(...) for MutationImpl::LockOp
41877de 
Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
@ukernel
mds: use single map to track pinned & auth_pinned objects
66035e4 
reduce memory allocation and fragment

Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
@ukernel
mds: restrict path that walks into snapdir
097eedb 
The zero-length component (stands for snapdir) must be at very
beginning of path. This allow us to easily check if a client request
want to access snapshot

Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
@ukernel
mds: cleanup Server::try_open_auth_dirfrag()
8a077e3 
Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
@ukernel
mds: cleanup code that kills session requests
a10f051 
Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
@ukernel
mds: cleanup Capability initialization
1c24cf2 
Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
@batrick
Copy link
Member

batrick commented Nov 9, 2019

http://pulpito.ceph.com/?branch=wip-pdonnell-testing-20191109.183750

@ukernel
mds: move 'traverse to auth' logic into MDCache::path_traverse
fdaacd7 
simplfy code and reduce duplication

Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
@ukernel
Copy link
Contributor Author

ukernel commented Nov 11, 2019

Commit 'mds: move 'traverse to auth' logic into MDCache::path_traverse' contains a multi-mds bug. please re-run multimds tests

@batrick
Copy link
Member

batrick commented Nov 11, 2019

pulpito.ceph.com/?branch=wip-pdonnell-testing-20191111.154849

@batrick
Copy link
Member

batrick commented Nov 12, 2019

@ukernel let me know if you found any problems in these test runs. I will take a look myself later today.

@ukernel
Copy link
Contributor Author

ukernel commented Nov 14, 2019

test runs look good. A few tests ran two slow, which causes timeout

@batrick
Copy link
Member

batrick commented Nov 20, 2019 •
edited

Doubel checking some fs failures are unrelated: http://pulpito.ceph.com/?branch=wip-pdonnell-testing-20191120.192241

@batrick
Copy link
Member

batrick commented Nov 21, 2019 •
edited

@ukernel

PTAL @ http://pulpito.ceph.com/pdonnell-2019-11-20_23:37:41-fs-wip-pdonnell-testing-20191120.192241-distro-basic-smithi/4527630/

I'm attempting to get the job to fail so that the logs are gathered.

This failure is new with your PR. Latest master nightly didn't see this: http://pulpito.ceph.com/teuthology-2019-11-19_03:15:03-fs-master-distro-basic-smithi/

@batrick
Copy link
Member

batrick commented Nov 22, 2019

Sorry, not this PR. It's also happening with another one:

http://pulpito.ceph.com/pdonnell-2019-11-21_22:49:13-fs-wip-pdonnell-testing-20191121.191852-distro-basic-smithi/4530520/

@batrick
Copy link
Member

batrick commented Nov 22, 2019

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

@batrick
Copy link
Member

batrick commented Nov 23, 2019

Zheng, I don't know how but this PR breaks the config show test:

http://pulpito.ceph.com/pdonnell-2019-11-20_23:37:41-fs-wip-pdonnell-testing-20191120.192241-distro-basic-smithi/4527596/

and another more recent build:

http://pulpito.ceph.com/pdonnell-2019-11-22_21:28:34-fs-wip-pdonnell-testing-20191122.191531-distro-basic-smithi/4534154/

compare to master:

/ceph/teuthology-archive/pdonnell-2019-11-22_22:39:39-fs-master-distro-basic-smithi/4534608/teuthology.log

mgr logs are different. It might be something about the MDS changing what it sends to the mgr but I don't see how this PR would do that.

I'm still looking into this but so far I'm pretty confused.

@batrick
Copy link
Member

batrick commented Nov 23, 2019

cache drop test is also broken but I haven't looked into that one yet: http://pulpito.ceph.com/pdonnell-2019-11-22_21:28:34-fs-wip-pdonnell-testing-20191122.191531-distro-basic-smithi/4534150/

@ukernel
Copy link
Contributor Author

ukernel commented Nov 24, 2019

I re-ran the 'config show' tests. They passed.
http://pulpito.ceph.com/zyan-2019-11-24_04:05:41-fs-wip-pdonnell-testing-20191120.192241-distro-basic-smithi/

maybe the failed test ran 'config show' command too fast, before mds reported its configs

@batrick
Copy link
Member

batrick commented Nov 25, 2019

The cache drop failures look the same as the latest nautilus run: http://pulpito.ceph.com/yuriw-2019-11-21_16:57:10-fs-wip-yuri-nautilus_14.2.5_RC1-distro-basic-smithi/

I'll review this again now and see if I can merge it.

batrick added a commit that referenced this pull request Nov 25, 2019
@batrick
Merge PR #30972 into master
d7821ce 
* refs/pull/30972/head:
	mds: move 'traverse to auth' logic into MDCache::path_traverse
	mds: cleanup Capability initialization
	mds: cleanup code that kills session requests
	mds: cleanup Server::try_open_auth_dirfrag()
	mds: restrict path that walks into snapdir
	mds: use single map to track pinned & auth_pinned objects
	mds: define operator<(...) for MutationImpl::LockOp
	mds: cleanup Locker::acquire_locks()
	mds: introduce Locker::wrlock_try()
	mds: explictly specify if MDCache::path_traverse() needs to check null dentry
	mds: cleanup dentry non-readable check in MDCache::path_traverse()
	mds: change MDCache::path_traverse()'s 'onfail' argument to flags

Reviewed-by: Patrick Donnelly <pdonnell@redhat.com>
Reviewed-by: Jeff Layton <jlayton@redhat.com>
@batrick batrick merged commit fdaacd7 into ceph:master Nov 25, 2019
@ukernel ukernel deleted the wip-mds-locking-order-prep branch November 25, 2019 03:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@batrick batrick batrick approved these changes

@jtlayton jtlayton jtlayton approved these changes

Assignees
No one assigned
Labels
cephfs Ceph File System needs-review
Projects
None yet
Milestone
No milestone
3 participants
@ukernel @jtlayton @batrick