From f99e0814f8329213333fed3b9e0449d877e313b5 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Tue, 2 Dec 2008 11:45:37 -0800 Subject: [PATCH] mds: avoid overlapping release attempts If the first release attempt is waiting for the log to flush, we should avoid sending any RELEASED ack until all releases have flushed. That is, only the last release will ack. Keep a counter in Capability to do this. Otherwise, we may close out a capability from under a release that is flushing, and our seq # will be meaningless later in _finish_release_cap() when we're trying to decide what to do. --- src/mds/Capability.h | 5 ++- src/mds/Locker.cc | 86 ++++++++++++++++++++++++-------------------- src/mds/Locker.h | 2 +- 3 files changed, 53 insertions(+), 40 deletions(-) diff --git a/src/mds/Capability.h b/src/mds/Capability.h index 7d8ea7e164f99..6d198612ea4c2 100644 --- a/src/mds/Capability.h +++ b/src/mds/Capability.h @@ -69,7 +69,10 @@ private: int suppress; bool stale; + public: + int releasing; // only allow a single in-progress release (it may be waiting for log to flush) + snapid_t client_follows; xlist::item session_caps_item; @@ -83,7 +86,7 @@ public: last_recv(s), last_open(0), mseq(0), - suppress(0), stale(false), + suppress(0), stale(false), releasing(0), client_follows(0), session_caps_item(this), snaprealm_caps_item(this) { } diff --git a/src/mds/Locker.cc b/src/mds/Locker.cc index 5bbfb587e6cab..252a2291469fb 100644 --- a/src/mds/Locker.cc +++ b/src/mds/Locker.cc @@ -500,26 +500,9 @@ void Locker::file_update_finish(CInode *in, Mutation *mut, bool share, int clien mut->apply(); if (releasecap) { - // before we remove the cap, make sure the release request is - // _still_ the most recent (and not racing with open() or - // something) - Capability *cap = in->get_client_cap(client); - if (!cap) { - dout(10) << " can't releasecap client" << client << ", cap has gone away?" << dendl; - delete ack; - ack = 0; - } else if (releasecap < cap->get_last_sent()) { - dout(10) << " NOT releasing cap client" << client << ", last_sent " << cap->get_last_sent() - << " > " << releasecap << dendl; - delete ack; - ack = 0; - } else { - _finish_release_cap(in, client, releasecap); - assert(ack); - } - } - - if (ack) + assert(ack); + _finish_release_cap(in, client, releasecap, ack); + } else if (ack) mds->send_message_client(ack, client); drop_locks(mut); @@ -1101,6 +1084,12 @@ void Locker::handle_client_caps(MClientCaps *m) } else if (m->get_op() == CEPH_CAP_OP_RELEASE) { dout(7) << " release request client" << client << " seq " << m->get_seq() << " on " << *in << dendl; releasecap = m->get_seq(); + /* + * if multiple release requests overlap (i.e. because the first one is waiting + * for the log to flush), wait for them all to "complete", and only ack the + * last one. to do this, keep count; see matching decrement in _finish_release_cap(). + */ + cap->releasing++; ack = new MClientCaps(CEPH_CAP_OP_RELEASED, in->inode, 0, 0, 0, 0, 0); } else if (wanted != cap->wanted()) { dout(10) << " wanted " << cap_string(cap->wanted()) @@ -1124,20 +1113,40 @@ void Locker::handle_client_caps(MClientCaps *m) delete m; } -void Locker::_finish_release_cap(CInode *in, int client, capseq_t seq) +void Locker::_finish_release_cap(CInode *in, int client, capseq_t seq, MClientCaps *ack) { dout(10) << "_finish_release_cap client" << client << " seq " << seq << " on " << *in << dendl; - in->remove_client_cap(client); - if (!in->is_auth()) - request_inode_file_caps(in); + + Capability *cap = in->get_client_cap(client); + assert(cap); - // unlinked stray? may need to purge (e.g., after all caps are released) - if (in->inode.nlink == 0 && - !in->is_any_caps() && - in->is_auth() && - in->get_parent_dn() && - in->get_parent_dn()->get_dir()->get_inode()->is_stray()) - mdcache->eval_stray(in->get_parent_dn()); + // before we remove the cap, make sure the release request is + // _still_ the most recent (and not racing with open() or + // something) + + cap->releasing--; + if (cap->releasing) { + dout(10) << " another release attempt in flight, not releasing yet" << dendl; + delete ack; + } else if (seq < cap->get_last_sent()) { + dout(10) << " NOT releasing cap client" << client << ", last_sent " << cap->get_last_sent() + << " > " << seq << dendl; + delete ack; + } else { + in->remove_client_cap(client); + if (!in->is_auth()) + request_inode_file_caps(in); + + mds->send_message_client(ack, client); + + // unlinked stray? may need to purge (e.g., after all caps are released) + if (in->inode.nlink == 0 && + !in->is_any_caps() && + in->is_auth() && + in->get_parent_dn() && + in->get_parent_dn()->get_dir()->get_inode()->is_stray()) + mdcache->eval_stray(in->get_parent_dn()); + } } void Locker::_do_cap_update(CInode *in, int had, int all_wanted, snapid_t follows, MClientCaps *m, @@ -1145,6 +1154,8 @@ void Locker::_do_cap_update(CInode *in, int had, int all_wanted, snapid_t follow { dout(10) << "_do_cap_update had " << cap_string(had) << " on " << *in << dendl; + int client = m->get_source().num(); + inode_t *latest = in->get_projected_inode(); utime_t atime = m->get_atime(); @@ -1246,15 +1257,14 @@ void Locker::_do_cap_update(CInode *in, int had, int all_wanted, snapid_t follow mdcache->journal_dirty_inode(mut, &le->metablob, in, follows); mds->mdlog->submit_entry(le, new C_Locker_FileUpdate_finish(this, in, mut, change_max, - m->get_source().num(), - ack, releasecap)); + client, ack, releasecap)); } else { // no update, ack now. - if (ack) { - mds->send_message_client(ack, m->get_source().num()); - if (releasecap) - _finish_release_cap(in, m->get_source().num(), releasecap); - } + if (releasecap) { + assert(ack); + _finish_release_cap(in, client, releasecap, ack); + } else if (ack) + mds->send_message_client(ack, client); } // reevaluate, waiters diff --git a/src/mds/Locker.h b/src/mds/Locker.h index 56fb595192301..d706db2557ea3 100644 --- a/src/mds/Locker.h +++ b/src/mds/Locker.h @@ -206,7 +206,7 @@ protected: void handle_client_caps(class MClientCaps *m); void _do_cap_update(CInode *in, int had, int wanted, snapid_t follows, MClientCaps *m, MClientCaps *ack=0, capseq_t releasecap=0); - void _finish_release_cap(CInode *in, int client, capseq_t seq); + void _finish_release_cap(CInode *in, int client, capseq_t seq, MClientCaps *ack); void request_inode_file_caps(CInode *in); -- 2.39.5