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

nautilus: cephfs: mds: Fix duplicate client entries in eviction list #30951

Merged
merged 1 commit into from Nov 18, 2019
Merged

nautilus: cephfs: mds: Fix duplicate client entries in eviction list #30951

merged 1 commit into from Nov 18, 2019

Conversation

sidharthanup
Copy link

@sidharthanup sidharthanup commented Oct 16, 2019 •
edited

Fix duplicate client entries in list to avoid multiple client evictions.

Backport Tracker: https://tracker.ceph.com/issues/41886

Backport of: #30029

parent tracker: https://tracker.ceph.com/issues/41585

@sidharthanup sidharthanup added the cephfs Ceph File System label Oct 16, 2019
@sidharthanup sidharthanup force-pushed the mds-evict-duplicate-nautilus branch 2 times, most recently from c482440 to 22f5047 Compare October 16, 2019 01:34
@tchaikov
Copy link
Contributor

jenkins test submodules

@tchaikov tchaikov mentioned this pull request Oct 16, 2019
Merged
@sidharthanup sidharthanup added this to the nautilus milestone Oct 17, 2019
@sidharthanup sidharthanup force-pushed the mds-evict-duplicate-nautilus branch 2 times, most recently from 84f1164 to 19c4d32 Compare October 18, 2019 10:41
@sidharthanup
mds: Fix duplicate client entries in list to avoid multiple client ev…
71dd445 
…ictions

Cannot be cherry-picked from master because the container to be changed that's being used in nautilus is std::list instead of std::vector

Fixes: https://tracker.ceph.com/issues/41585
Signed-off-by: Sidharth Anupkrishnan <sanupkri@redhat.com>
(manual backport of f4afb43)
@smithfarm
Copy link
Contributor

jenkins test submodules

@smithfarm smithfarm changed the title nautilus:mds: Fix duplicate client entries in eviction list nautilus: mds: Fix duplicate client entries in eviction list Oct 22, 2019
@smithfarm smithfarm changed the title nautilus: mds: Fix duplicate client entries in eviction list nautilus: cephfs: mds: Fix duplicate client entries in eviction list Oct 22, 2019
smithfarm
smithfarm previously requested changes Oct 22, 2019
View reviewed changes
Copy link
Contributor

@smithfarm smithfarm left a comment

Choose a reason for hiding this comment

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

@sidharthanup Did this cherry-pick without any conflicts? When I try, I get:

$ git checkout -b wip-foo ceph/nautilus
Branch 'wip-foo' set up to track remote branch 'nautilus' from 'ceph'.
Switched to a new branch 'wip-foo'
$ git cherry-pick -x f4afb43
error: could not apply f4afb43f36... mds: use set to store to evict client
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
hint: and commit the result with 'git commit'

See:

@smithfarm
Copy link
Contributor

smithfarm commented Oct 22, 2019 •
edited

@sidharthanup I think it would be enough to append the following to the commit message:

Conflicts:
src/mds/Locker.cc
src/mds/Locker.h
- manual backport was necessary because ...

@smithfarm smithfarm dismissed their stale review October 22, 2019 08:07

Nevermind. Upon taking a second look, I see this is OK. Sorry for the noise!

@smithfarm smithfarm added nautilus-batch-1 nautilus point releases needs-qa labels Oct 22, 2019
@smithfarm
Copy link
Contributor

@batrick @liewegas Sage mentioned this PR in https://tracker.ceph.com/issues/42570 . . . it's still open. Is it good to go? (I guess so, since the problem in mimic was addressed by #31275)

@yuriw
Copy link
Contributor

yuriw commented Nov 13, 2019

wip-yuri7-testing-2019-11-13-1707-nautilus

@batrick
Copy link
Member

batrick commented Nov 13, 2019

@batrick @liewegas Sage mentioned this PR in https://tracker.ceph.com/issues/42570 . . . it's still open. Is it good to go? (I guess so, since the problem in mimic was addressed by #31275)

I think you're thinking of #30195

@smithfarm
Copy link
Contributor

@batrick This PR is targeting nautilus, and https://tracker.ceph.com/issues/42570 is about the mimic-master upgrade, which I suppose goes like this: mimic -> nautilus -> master?

That's why I mentioned #31275...

@batrick
Copy link
Member

batrick commented Nov 14, 2019

@batrick This PR is targeting nautilus, and https://tracker.ceph.com/issues/42570 is about the mimic-master upgrade, which I suppose goes like this: mimic -> nautilus -> master?

That's why I mentioned #31275...

I think @liewegas typo'd in https://tracker.ceph.com/issues/42570#note-1

db84d9e is in #30225

This PR has nothing to do with any of that.

@smithfarm
Copy link
Contributor

@batrick Indeed. Thanks for the correction.

@yuriw
Copy link
Contributor

yuriw commented Nov 18, 2019

@sidharthanup @smithfarm this passed test, pls merge

@yuriw yuriw merged commit a2a063e into ceph:nautilus Nov 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ajarr ajarr ajarr approved these changes

@smithfarm smithfarm smithfarm left review comments

@batrick batrick Awaiting requested review from batrick

@liewegas liewegas Awaiting requested review from liewegas

Assignees
No one assigned
Labels
cephfs Ceph File System nautilus-batch-1 nautilus point releases needs-qa wip-yuri7-testing
Projects
None yet
Milestone
nautilus
6 participants
@sidharthanup @tchaikov @smithfarm @yuriw @batrick @ajarr