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.

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

mds: misc multimds fixes #12274

Merged
merged 8 commits into from Jan 9, 2017

Conversation

Projects
None yet
3 participants
@ukernel
Member

ukernel commented Dec 2, 2016

No description provided.

@gregsfortytwo

Some quick questions.

for (auto p : dfls) {
if (mds->get_nodeid() != p->get_dir_auth().first)
witnesses.insert(p->get_dir_auth().first);
}

This comment has been minimized.

@gregsfortytwo

gregsfortytwo Dec 6, 2016

Member

If we don't make other replicas a witness here, at which point do they find out the inode has been unlinked?

This comment has been minimized.

@ukernel

ukernel Dec 6, 2016

Member

Server::_unlink_local_finish send dentry unlink message to the rest replicas

@@ -2049,6 +2055,10 @@ void Locker::resume_stale_caps(Session *session)
if (cap->is_stale()) {
dout(10) << " clearing stale flag on " << *in << dendl;
cap->clear_stale();
if (in->state_test(CInode::STATE_EXPORTINGCAPS))
continue;

This comment has been minimized.

@gregsfortytwo

gregsfortytwo Dec 6, 2016

Member

This is subtle; can you explain in the commit message? I think this is assuming we're the recipient and can just toss out the stale caps now, right?

@ghost

This comment has been minimized.

ghost commented Dec 6, 2016

jenkins test this please (asok http://tracker.ceph.com/issues/15249)

ukernel added some commits Nov 30, 2016

mds: use projected linkage in StrayManager::migrate_stray()
Its caller StrayManager::eval_remote_stray uses projected linkage.

Signed-off-by: Yan, Zheng <zyan@redhat.com>
mds: fix indent in StrayManager::eval_remote_stray
Signed-off-by: Yan, Zheng <zyan@redhat.com>
mds: only send slave rmdir prepare to auth mds of subtree
This avoid trigger the assertion of path traverse return value
in Server::handle_slave_rmdir_prep.  (Other witness mds may release
the inode before receiving the slave rmdir prepare)

Signed-off-by: Yan, Zheng <zyan@redhat.com>
mds: don't revoke/resume stale caps while exporting caps
The revoke/resume cycle increases capability's sequence, which
confuses clients. The caps get removed if exporting succeeds.
We only need to revoke/resume stale caps after exporting fails.

Signed-off-by: Yan, Zheng <zyan@redhat.com>
mds: don't kill request that has started slave operations
Rollback slave operations is tricky. Make MDcache::request_kill
ignores request that has started slave operations.

Signed-off-by: Yan, Zheng <zyan@redhat.com>
@gregsfortytwo

This comment has been minimized.

Member

gregsfortytwo commented Dec 9, 2016

@ukernel, you appear to have pushed a bunch of new and unrelated bug fix commits on top of an already-reviewed PR? It'll help if you don't apply new ones once a PR has been looked at so it's clear what has and hasn't been reviewed.

@gregsfortytwo

There are several things I don't understand here.

srcdnl->get_inode()->filelock.remove_dirty();
srcdnl->get_inode()->nestlock.remove_dirty();
*/

This comment has been minimized.

@gregsfortytwo

gregsfortytwo Dec 9, 2016

Member

Why do you want to leave this in comments instead of removing the code?

cache->adjust_subtree_auth(dir, mds->get_nodeid());
cache->try_subtree_merge(dir); // NOTE: this may journal subtree_map as side effect
dir->unfreeze_tree();

This comment has been minimized.

@gregsfortytwo

gregsfortytwo Dec 9, 2016

Member

What's up with these ordering changes? If they're necessary for correctness we need to at least document that, if not enforce it with code state.
(Seriously, this change implies an ordering that allows unfreeze before or after adjusting auth and merging, but doesn't work in both orders.)

@@ -89,7 +89,6 @@ class Migrator {
set<mds_rank_t> warning_ack_waiting;
set<mds_rank_t> notify_ack_waiting;
map<inodeno_t,map<client_t,Capability::Import> > peer_imported;
list<MDSInternalContextBase*> waiting_for_finish;

This comment has been minimized.

@gregsfortytwo

gregsfortytwo Dec 9, 2016

Member

"mds: remove useless code in Migrator"
With commits like that it's best to say exactly why it's useless. In this case, I gather nobody ever put anything on waiting_for_finish or called add_export_finish_waiter(), but the commit should say so.

bufferlist::iterator blp = client_map.begin();
::decode(cm, blp);
mds->server->prepare_force_open_sessions(cm, seqm);
mds->server->finish_force_open_sessions(cm, seqm);
mds->sessionmap.open_sessions(cm);

This comment has been minimized.

@gregsfortytwo

gregsfortytwo Dec 9, 2016

Member

I don't understand why this change is being made at all. I think maybe the commit means you want to switch to doing this, but I'm not sure why. It looks like you're just tossing out a bunch of asserts and other safety/metadata code.

This comment has been minimized.

@gregsfortytwo

gregsfortytwo Dec 12, 2016

Member

Okay, makes sense that trying to use the same code on both import and replay wouldn't work.

ukernel added some commits Dec 6, 2016

mds: properly wake up scatter_nudge waiters after clearing scatter dirty
LogSegment::try_to_expire() calls Locker::scatter_nudge() for dirty
scatter locks. If lock's parent is non-auth, Locker::scatter_nudge()
waits on the stable bit of corresponding lock. There are some cases
that scatter locks get marked clean without involving lock state
transition. We need to wake up scatter_nudge waiters in these cases.

Also remove the code that clear dirty scatter locks after inode gets
imported. I can't see why we should do that.

Signed-off-by: Yan, Zheng <zyan@redhat.com>
mds: fix sessionmap version increasement when replaying ESessions
SessionMap::open_sessions() is used by ESessions::replay() to open
sessions that were opened by Server::prepare_force_open_sessions().

Server::prepare_force_open_sessions() increases session map's version
for each opened sessions. But SessionMap::open_sessions() only increases
session map's version by one.

Signed-off-by: Yan, Zheng <zyan@redhat.com>
mds: don't use Server::finish_force_open_sessions during log replay
Server::finish_force_open_sessions() send session open message to
clients. It's wrong to use it during log replay.

Signed-off-by: Yan, Zheng <zyan@redhat.com>

@ukernel ukernel assigned gregsfortytwo and unassigned ukernel Dec 9, 2016

@ukernel

This comment has been minimized.

Member

ukernel commented Dec 9, 2016

updated

@gregsfortytwo

This comment has been minimized.

Member

gregsfortytwo commented Dec 12, 2016

Reviewed-by: Greg Farnum gfarnum@redhat.com

@gregsfortytwo gregsfortytwo removed their assignment Dec 12, 2016

@liewegas liewegas changed the title from Wip multimds misc to mds: misc multimds fixes Dec 29, 2016

@jcsp jcsp merged commit 30cfc81 into ceph:master Jan 9, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

@ukernel ukernel deleted the ukernel:wip-multimds-misc branch Jan 10, 2017

@ukernel ukernel restored the ukernel:wip-multimds-misc branch Jan 11, 2017

@ukernel ukernel deleted the ukernel:wip-multimds-misc branch Jan 12, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment