From 3a4d63ee67765010a8e53af5a89aef4f49fafd56 Mon Sep 17 00:00:00 2001 From: Douglas Fuller Date: Mon, 27 Jun 2016 12:30:11 -0700 Subject: [PATCH] mds: Kill C_SaferCond in evict_sessions() MDSRankDispatcher::evict_sessions waits on a C_SaferCond for kill_session to complete on each of its victims. Change the command handling flow to pass command messages all the way down to MDSRankDispatcher. Extract the MDSDaemon's reply path into a static function callable from a new context in the MDSRankDispatcher. See: http://tracker.ceph.com/issues/16288 Signed-off-by: Douglas Fuller --- src/mds/MDSDaemon.cc | 59 +++++++++++++++++++++++++++----------------- src/mds/MDSDaemon.h | 8 ++++-- src/mds/MDSRank.cc | 49 ++++++++++++++++++++++++------------ src/mds/MDSRank.h | 13 +++++----- 4 files changed, 82 insertions(+), 47 deletions(-) diff --git a/src/mds/MDSDaemon.cc b/src/mds/MDSDaemon.cc index ef9ade80fc877..ad163372b1c17 100644 --- a/src/mds/MDSDaemon.cc +++ b/src/mds/MDSDaemon.cc @@ -587,6 +587,33 @@ void MDSDaemon::tick() } } +void MDSDaemon::send_command_reply(MCommand *m, MDSRank *mds_rank, + int r, bufferlist outbl, + const std::string& outs) +{ + Session *session = static_cast(m->get_connection()->get_priv()); + assert(session != NULL); + // If someone is using a closed session for sending commands (e.g. + // the ceph CLI) then we should feel free to clean up this connection + // as soon as we've sent them a response. + const bool live_session = mds_rank && + mds_rank->sessionmap.get_session(session->info.inst.name) != nullptr + && session->get_state_seq() > 0; + + if (!live_session) { + // This session only existed to issue commands, so terminate it + // as soon as we can. + assert(session->is_closed()); + session->connection->mark_disposable(); + session->put(); + } + + MCommandReply *reply = new MCommandReply(r, outs); + reply->set_tid(m->get_tid()); + reply->set_data(outbl); + m->get_connection()->send_message(reply); +} + /* This function DOES put the passed message before returning*/ void MDSDaemon::handle_command(MCommand *m) { @@ -599,7 +626,7 @@ void MDSDaemon::handle_command(MCommand *m) std::string outs; bufferlist outbl; Context *run_after = NULL; - + bool need_reply = true; if (!session->auth_caps.allow_all()) { dout(1) << __func__ @@ -615,29 +642,13 @@ void MDSDaemon::handle_command(MCommand *m) r = -EINVAL; outs = ss.str(); } else { - r = _handle_command(cmdmap, m->get_data(), &outbl, &outs, &run_after); + r = _handle_command(cmdmap, m, &outbl, &outs, &run_after, &need_reply); } - // If someone is using a closed session for sending commands (e.g. - // the ceph CLI) then we should feel free to clean up this connection - // as soon as we've sent them a response. - const bool live_session = mds_rank && - mds_rank->sessionmap.get_session(session->info.inst.name) != nullptr - && session->get_state_seq() > 0; - - if (!live_session) { - // This session only existed to issue commands, so terminate it - // as soon as we can. - assert(session->is_closed()); - session->connection->mark_disposable(); - session->put(); + if (need_reply) { + send_command_reply(m, mds_rank, r, outbl, outs); } - MCommandReply *reply = new MCommandReply(r, outs); - reply->set_tid(m->get_tid()); - reply->set_data(outbl); - m->get_connection()->send_message(reply); - if (run_after) { run_after->complete(0); } @@ -703,10 +714,11 @@ void MDSDaemon::handle_command(MMonCommand *m) int MDSDaemon::_handle_command( const cmdmap_t &cmdmap, - bufferlist const &inbl, + MCommand *m, bufferlist *outbl, std::string *outs, - Context **run_later) + Context **run_later, + bool *need_reply) { assert(outbl != NULL); assert(outs != NULL); @@ -824,7 +836,8 @@ int MDSDaemon::_handle_command( } else { // Give MDSRank a shot at the command if (mds_rank) { - bool handled = mds_rank->handle_command(cmdmap, inbl, &r, &ds, &ss); + bool handled = mds_rank->handle_command(cmdmap, m, &r, &ds, &ss, + need_reply); if (handled) { goto out; } diff --git a/src/mds/MDSDaemon.h b/src/mds/MDSDaemon.h index 4d6296c5df2ab..53d0cf46ec24f 100644 --- a/src/mds/MDSDaemon.h +++ b/src/mds/MDSDaemon.h @@ -187,13 +187,17 @@ protected: bool handle_core_message(Message *m); // special message types + friend class C_MDS_Send_Command_Reply; + static void send_command_reply(MCommand *m, MDSRank* mds_rank, int r, + bufferlist outbl, const std::string& outs); int _handle_command_legacy(std::vector args); int _handle_command( const cmdmap_t &cmdmap, - bufferlist const &inbl, + MCommand *m, bufferlist *outbl, std::string *outs, - Context **run_later); + Context **run_later, + bool *need_reply); void handle_command(class MMonCommand *m); void handle_command(class MCommand *m); void handle_mds_map(class MMDSMap *m); diff --git a/src/mds/MDSRank.cc b/src/mds/MDSRank.cc index fb9c89465ae83..46cfc908be53b 100644 --- a/src/mds/MDSRank.cc +++ b/src/mds/MDSRank.cc @@ -17,7 +17,10 @@ #include "messages/MClientRequestForward.h" #include "messages/MMDSMap.h" +#include "messages/MCommand.h" +#include "messages/MCommandReply.h" +#include "MDSDaemon.h" #include "MDSMap.h" #include "SnapClient.h" #include "SnapServer.h" @@ -1777,15 +1780,32 @@ bool MDSRankDispatcher::handle_asok_command( return true; } +class C_MDS_Send_Command_Reply : public MDSInternalContext +{ +protected: + MCommand *m; +public: + C_MDS_Send_Command_Reply(MDSRank *_mds, MCommand *_m) : + MDSInternalContext(_mds), m(_m) { m->get(); } + void send (int r) { + bufferlist bl; + MDSDaemon::send_command_reply(m, mds, r, bl, ""); + m->put(); + } + void finish (int r) { + send(r); + } +}; + /** * This function drops the mds_lock, so don't do anything with * MDSRank after calling it (we could have gone into shutdown): just * send your result back to the calling client and finish. */ -std::vector MDSRankDispatcher::evict_sessions( - const SessionFilter &filter) +void MDSRankDispatcher::evict_sessions(const SessionFilter &filter, MCommand *m) { std::list victims; + C_MDS_Send_Command_Reply *reply = new C_MDS_Send_Command_Reply(this, m); const auto sessions = sessionmap.get_sessions(); for (const auto p : sessions) { @@ -1802,24 +1822,17 @@ std::vector MDSRankDispatcher::evict_sessions( dout(20) << __func__ << " matched " << victims.size() << " sessions" << dendl; - std::vector result; - if (victims.empty()) { - return result; + reply->send(0); + delete reply; + return; } - C_SaferCond on_safe; - C_GatherBuilder gather(g_ceph_context, &on_safe); + C_GatherBuilder gather(g_ceph_context, reply); for (const auto s : victims) { server->kill_session(s, gather.new_sub()); - result.push_back(s->info.inst.name); } gather.activate(); - mds_lock.Unlock(); - on_safe.wait(); - mds_lock.Lock(); - - return result; } void MDSRankDispatcher::dump_sessions(const SessionFilter &filter, Formatter *f) const @@ -2545,15 +2558,18 @@ MDSRankDispatcher::MDSRankDispatcher( bool MDSRankDispatcher::handle_command( const cmdmap_t &cmdmap, - bufferlist const &inbl, + MCommand *m, int *r, std::stringstream *ds, - std::stringstream *ss) + std::stringstream *ss, + bool *need_reply) { assert(r != nullptr); assert(ds != nullptr); assert(ss != nullptr); + *need_reply = true; + std::string prefix; cmd_getval(g_ceph_context, cmdmap, "prefix", prefix); @@ -2582,8 +2598,9 @@ bool MDSRankDispatcher::handle_command( return true; } - evict_sessions(filter); + evict_sessions(filter, m); + *need_reply = false; return true; } else if (prefix == "damage ls") { Formatter *f = new JSONFormatter(); diff --git a/src/mds/MDSRank.h b/src/mds/MDSRank.h index 6710274ac878e..b8ad90dd46f31 100644 --- a/src/mds/MDSRank.h +++ b/src/mds/MDSRank.h @@ -19,6 +19,8 @@ #include "common/LogClient.h" #include "common/Timer.h" +#include "messages/MCommand.h" + #include "Beacon.h" #include "DamageTable.h" #include "MDSMap.h" @@ -496,15 +498,14 @@ public: bool handle_command( const cmdmap_t &cmdmap, - bufferlist const &inbl, + MCommand *m, int *r, std::stringstream *ds, - std::stringstream *ss); + std::stringstream *ss, + bool *need_reply); - void dump_sessions( - const SessionFilter &filter, Formatter *f) const; - std::vector evict_sessions( - const SessionFilter &filter); + void dump_sessions(const SessionFilter &filter, Formatter *f) const; + void evict_sessions(const SessionFilter &filter, MCommand *m); // Call into me from MDS::ms_dispatch bool ms_dispatch(Message *m); -- 2.39.5