On Thu, 21 Mar 2013, Yan, Zheng wrote: > On 03/21/2013 05:19 AM, Greg Farnum wrote: > > On Sunday, March 17, 2013 at 7:51 AM, Yan, Zheng wrote: > >> From: "Yan, Zheng" <zheng.z.yan@xxxxxxxxx> > >> > >> Replicated objects need to be added into the cache immediately > >> > >> Signed-off-by: Yan, Zheng <zheng.z.yan@xxxxxxxxx> > > Why do we need to add them right away? Shouldn't we have a journaled replica if we need it? > > -Greg > > The issue I encountered is lock action message received, but replicated objects wasn't in the > cache because slave request was delayed. This makes sense to me; the add_replica_*() methods that create and push replicas of cache objects to other nodes need to always be applied immediately, or else the cache coherency falls apart. There are similar games played between the client and mds with the caps protocol, although in that case IIRC there are certain limited circumstances where we can delay processing the message. For mds->mds traffic, I don't think that's possible, unless *all* potentially dependent traffic is also delayed to preserve ordering and so forth. [That said, I didn't review the actual patch :)] sage > > Thanks > Yan, Zheng > > > > > > Software Engineer #42 @ http://inktank.com | http://ceph.com > >> --- > >> src/mds/MDCache.cc | 12 ++++++++++++ > >> src/mds/MDCache.h | 2 +- > >> src/mds/MDS.cc | 6 +++--- > >> src/mds/Server.cc | 55 +++++++++++++++++++++++++++++++++++++++--------------- > >> 4 files changed, 56 insertions(+), 19 deletions(-) > >> > >> diff --git a/src/mds/MDCache.cc b/src/mds/MDCache.cc > >> index 0f6b842..b668842 100644 > >> --- a/src/mds/MDCache.cc > >> +++ b/src/mds/MDCache.cc > >> @@ -7722,6 +7722,18 @@ void MDCache::_find_ino_dir(inodeno_t ino, Context *fin, bufferlist& bl, int r) > >> > >> /* ---------------------------- */ > >> > >> +int MDCache::get_num_client_requests() > >> +{ > >> + int count = 0; > >> + for (hash_map<metareqid_t, MDRequest*>::iterator p = active_requests.begin(); > >> + p != active_requests.end(); > >> + ++p) { > >> + if (p->second->reqid.name.is_client() && !p->second->is_slave()) > >> + count++; > >> + } > >> + return count; > >> +} > >> + > >> /* This function takes over the reference to the passed Message */ > >> MDRequest *MDCache::request_start(MClientRequest *req) > >> { > >> diff --git a/src/mds/MDCache.h b/src/mds/MDCache.h > >> index a9f05c6..4634121 100644 > >> --- a/src/mds/MDCache.h > >> +++ b/src/mds/MDCache.h > >> @@ -240,7 +240,7 @@ protected: > >> hash_map<metareqid_t, MDRequest*> active_requests; > >> > >> public: > >> - int get_num_active_requests() { return active_requests.size(); } > >> + int get_num_client_requests(); > >> > >> MDRequest* request_start(MClientRequest *req); > >> MDRequest* request_start_slave(metareqid_t rid, __u32 attempt, int by); > >> diff --git a/src/mds/MDS.cc b/src/mds/MDS.cc > >> index b91dcbd..e99eecc 100644 > >> --- a/src/mds/MDS.cc > >> +++ b/src/mds/MDS.cc > >> @@ -1900,9 +1900,9 @@ bool MDS::_dispatch(Message *m) > >> mdcache->is_open() && > >> replay_queue.empty() && > >> want_state == MDSMap::STATE_CLIENTREPLAY) { > >> - dout(10) << " still have " << mdcache->get_num_active_requests() > >> - << " active replay requests" << dendl; > >> - if (mdcache->get_num_active_requests() == 0) > >> + int num_requests = mdcache->get_num_client_requests(); > >> + dout(10) << " still have " << num_requests << " active replay requests" << dendl; > >> + if (num_requests == 0) > >> clientreplay_done(); > >> } > >> > >> diff --git a/src/mds/Server.cc b/src/mds/Server.cc > >> index 4c4c86b..8e89e4c 100644 > >> --- a/src/mds/Server.cc > >> +++ b/src/mds/Server.cc > >> @@ -107,10 +107,8 @@ void Server::dispatch(Message *m) > >> (m->get_type() == CEPH_MSG_CLIENT_REQUEST && > >> (static_cast<MClientRequest*>(m))->is_replay()))) { > >> // replaying! > >> - } else if (mds->is_clientreplay() && m->get_type() == MSG_MDS_SLAVE_REQUEST && > >> - ((static_cast<MMDSSlaveRequest*>(m))->is_reply() || > >> - !mds->mdsmap->is_active(m->get_source().num()))) { > >> - // slave reply or the master is also in the clientreplay stage > >> + } else if (m->get_type() == MSG_MDS_SLAVE_REQUEST) { > >> + // handle_slave_request() will wait if necessary > >> } else { > >> dout(3) << "not active yet, waiting" << dendl; > >> mds->wait_for_active(new C_MDS_RetryMessage(mds, m)); > >> @@ -1291,6 +1289,13 @@ void Server::handle_slave_request(MMDSSlaveRequest *m) > >> if (m->is_reply()) > >> return handle_slave_request_reply(m); > >> > >> + CDentry *straydn = NULL; > >> + if (m->stray.length() > 0) { > >> + straydn = mdcache->add_replica_stray(m->stray, from); > >> + assert(straydn); > >> + m->stray.clear(); > >> + } > >> + > >> // am i a new slave? > >> MDRequest *mdr = NULL; > >> if (mdcache->have_request(m->get_reqid())) { > >> @@ -1326,9 +1331,26 @@ void Server::handle_slave_request(MMDSSlaveRequest *m) > >> m->put(); > >> return; > >> } > >> - mdr = mdcache->request_start_slave(m->get_reqid(), m->get_attempt(), m->get_source().num()); > >> + mdr = mdcache->request_start_slave(m->get_reqid(), m->get_attempt(), from); > >> } > >> assert(mdr->slave_request == 0); // only one at a time, please! > >> + > >> + if (straydn) { > >> + mdr->pin(straydn); > >> + mdr->straydn = straydn; > >> + } > >> + > >> + if (!mds->is_clientreplay() && !mds->is_active() && !mds->is_stopping()) { > >> + dout(3) << "not clientreplay|active yet, waiting" << dendl; > >> + mds->wait_for_replay(new C_MDS_RetryMessage(mds, m)); > >> + return; > >> + } else if (mds->is_clientreplay() && !mds->mdsmap->is_clientreplay(from) && > >> + mdr->locks.empty()) { > >> + dout(3) << "not active yet, waiting" << dendl; > >> + mds->wait_for_active(new C_MDS_RetryMessage(mds, m)); > >> + return; > >> + } > >> + > >> mdr->slave_request = m; > >> > >> dispatch_slave_request(mdr); > >> @@ -1339,6 +1361,12 @@ void Server::handle_slave_request_reply(MMDSSlaveRequest *m) > >> { > >> int from = m->get_source().num(); > >> > >> + if (!mds->is_clientreplay() && !mds->is_active() && !mds->is_stopping()) { > >> + dout(3) << "not clientreplay|active yet, waiting" << dendl; > >> + mds->wait_for_replay(new C_MDS_RetryMessage(mds, m)); > >> + return; > >> + } > >> + > >> if (m->get_op() == MMDSSlaveRequest::OP_COMMITTED) { > >> metareqid_t r = m->get_reqid(); > >> mds->mdcache->committed_master_slave(r, from); > >> @@ -5138,10 +5166,8 @@ void Server::handle_slave_rmdir_prep(MDRequest *mdr) > >> dout(10) << " dn " << *dn << dendl; > >> mdr->pin(dn); > >> > >> - assert(mdr->slave_request->stray.length() > 0); > >> - CDentry *straydn = mdcache->add_replica_stray(mdr->slave_request->stray, mdr->slave_to_mds); > >> - assert(straydn); > >> - mdr->pin(straydn); > >> + assert(mdr->straydn); > >> + CDentry *straydn = mdr->straydn; > >> dout(10) << " straydn " << *straydn << dendl; > >> > >> mdr->now = mdr->slave_request->now; > >> @@ -5208,6 +5234,7 @@ void Server::_logged_slave_rmdir(MDRequest *mdr, CDentry *dn, CDentry *straydn) > >> // done. > >> mdr->slave_request->put(); > >> mdr->slave_request = 0; > >> + mdr->straydn = 0; > >> } > >> > >> void Server::handle_slave_rmdir_prep_ack(MDRequest *mdr, MMDSSlaveRequest *ack) > >> @@ -6460,15 +6487,12 @@ void Server::handle_slave_rename_prep(MDRequest *mdr) > >> // stray? > >> bool linkmerge = (srcdnl->get_inode() == destdnl->get_inode() && > >> (srcdnl->is_primary() || destdnl->is_primary())); > >> - CDentry *straydn = 0; > >> - if (destdnl->is_primary() && !linkmerge) { > >> - assert(mdr->slave_request->stray.length() > 0); > >> - straydn = mdcache->add_replica_stray(mdr->slave_request->stray, mdr->slave_to_mds); > >> + CDentry *straydn = mdr->straydn; > >> + if (destdnl->is_primary() && !linkmerge) > >> assert(straydn); > >> - mdr->pin(straydn); > >> - } > >> > >> mdr->now = mdr->slave_request->now; > >> + mdr->more()->srcdn_auth_mds = srcdn->authority().first; > >> > >> // set up commit waiter (early, to clean up any freezing etc we do) > >> if (!mdr->more()->slave_commit) > >> @@ -6651,6 +6675,7 @@ void Server::_logged_slave_rename(MDRequest *mdr, > >> // done. > >> mdr->slave_request->put(); > >> mdr->slave_request = 0; > >> + mdr->straydn = 0; > >> } > >> > >> void Server::_commit_slave_rename(MDRequest *mdr, int r, > >> -- > >> 1.7.11.7 > > > > > > > > -- > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
