From de093c61ac9734fd3e9f06ff1760adf1e63ef40d Mon Sep 17 00:00:00 2001 From: Xiubo Li Date: Thu, 24 Sep 2020 14:27:34 +0800 Subject: [PATCH] client: add command_lock support The Client::command_table member could get rid of the client_lock. And this could make the client have a higher concurrency. Fixes: https://tracker.ceph.com/issues/46620 Signed-off-by: Xiubo Li --- src/client/Client.cc | 102 ++++++++++++++++++++++++------------------- src/client/Client.h | 3 ++ 2 files changed, 61 insertions(+), 44 deletions(-) diff --git a/src/client/Client.cc b/src/client/Client.cc index 4f929a39e349a..1102778c1c283 100644 --- a/src/client/Client.cc +++ b/src/client/Client.cc @@ -2717,49 +2717,58 @@ void Client::handle_fs_map_user(const MConstRef& m) signal_cond_list(waiting_for_fsmap); } -void Client::handle_mds_map(const MConstRef& m) +// Cancel all the commands for missing or laggy GIDs +void Client::cancel_commands(const MDSMap& newmap) { - mds_gid_t old_inc, new_inc; - - std::scoped_lock cl(client_lock); - if (m->get_epoch() <= mdsmap->get_epoch()) { - ldout(cct, 1) << __func__ << " epoch " << m->get_epoch() - << " is identical to or older than our " - << mdsmap->get_epoch() << dendl; - return; - } + std::vector cancel_ops; - ldout(cct, 1) << __func__ << " epoch " << m->get_epoch() << dendl; - - std::unique_ptr oldmap(new MDSMap); - - oldmap->decode(m->get_encoded()); - - // Cancel any commands for missing or laggy GIDs - std::list cancel_ops; + std::scoped_lock cmd_lock(command_lock); auto &commands = command_table.get_commands(); - for (const auto &i : commands) { - auto &op = i.second; + for (const auto &[tid, op] : commands) { const mds_gid_t op_mds_gid = op.mds_gid; - if (oldmap->is_dne_gid(op_mds_gid) || oldmap->is_laggy_gid(op_mds_gid)) { - ldout(cct, 1) << __func__ << ": cancelling command op " << i.first << dendl; - cancel_ops.push_back(i.first); + if (newmap.is_dne_gid(op_mds_gid) || newmap.is_laggy_gid(op_mds_gid)) { + ldout(cct, 1) << __func__ << ": cancelling command op " << tid << dendl; + cancel_ops.push_back(tid); if (op.outs) { std::ostringstream ss; ss << "MDS " << op_mds_gid << " went away"; *(op.outs) = ss.str(); } + /* + * No need to make the con->mark_down under + * client_lock here, because the con will + * has its own lock. + */ op.con->mark_down(); - if (op.on_finish) { + if (op.on_finish) op.on_finish->complete(-ETIMEDOUT); - } } } for (const auto &tid : cancel_ops) command_table.erase(tid); +} - oldmap.swap(mdsmap); +void Client::handle_mds_map(const MConstRef& m) +{ + mds_gid_t old_inc, new_inc; + + std::unique_lock cl(client_lock); + if (m->get_epoch() <= mdsmap->get_epoch()) { + ldout(cct, 1) << __func__ << " epoch " << m->get_epoch() + << " is identical to or older than our " + << mdsmap->get_epoch() << dendl; + return; + } + + cl.unlock(); + ldout(cct, 1) << __func__ << " epoch " << m->get_epoch() << dendl; + std::unique_ptr _mdsmap(new MDSMap); + _mdsmap->decode(m->get_encoded()); + cancel_commands(*_mdsmap.get()); + cl.lock(); + + _mdsmap.swap(mdsmap); // reset session for (auto p = mds_sessions.begin(); p != mds_sessions.end(); ) { @@ -2767,12 +2776,12 @@ void Client::handle_mds_map(const MConstRef& m) MetaSession *session = &p->second; ++p; - int oldstate = oldmap->get_state(mds); + int oldstate = _mdsmap->get_state(mds); int newstate = mdsmap->get_state(mds); if (!mdsmap->is_up(mds)) { session->con->mark_down(); } else if (mdsmap->get_addrs(mds) != session->addrs) { - old_inc = oldmap->get_incarnation(mds); + old_inc = _mdsmap->get_incarnation(mds); new_inc = mdsmap->get_incarnation(mds); if (old_inc != new_inc) { ldout(cct, 1) << "mds incarnation changed from " @@ -5900,7 +5909,7 @@ int Client::mds_command( if (!iref_reader.is_state_satisfied()) return -ENOTCONN; - std::scoped_lock lock(client_lock); + std::unique_lock cl(client_lock); int r; r = authenticate(); @@ -5948,23 +5957,28 @@ int Client::mds_command( // Open a connection to the target MDS ConnectionRef conn = messenger->connect_to_mds(info.get_addrs()); - // Generate MDSCommandOp state - auto &op = command_table.start_command(); + cl.unlock(); + { + std::scoped_lock cmd_lock(command_lock); + // Generate MDSCommandOp state + auto &op = command_table.start_command(); - op.on_finish = gather.new_sub(); - op.cmd = cmd; - op.outbl = outbl; - op.outs = outs; - op.inbl = inbl; - op.mds_gid = target_gid; - op.con = conn; + op.on_finish = gather.new_sub(); + op.cmd = cmd; + op.outbl = outbl; + op.outs = outs; + op.inbl = inbl; + op.mds_gid = target_gid; + op.con = conn; - ldout(cct, 4) << __func__ << ": new command op to " << target_gid - << " tid=" << op.tid << cmd << dendl; + ldout(cct, 4) << __func__ << ": new command op to " << target_gid + << " tid=" << op.tid << cmd << dendl; - // Construct and send MCommand - auto m = op.get_message(monclient->get_fsid()); - conn->send_message2(std::move(m)); + // Construct and send MCommand + MessageRef m = op.get_message(monclient->get_fsid()); + conn->send_message2(std::move(m)); + } + cl.lock(); } gather.activate(); @@ -5977,7 +5991,7 @@ void Client::handle_command_reply(const MConstRef& m) ldout(cct, 10) << __func__ << ": tid=" << m->get_tid() << dendl; - std::scoped_lock cl(client_lock); + std::scoped_lock cmd_lock(command_lock); if (!command_table.exists(tid)) { ldout(cct, 1) << __func__ << ": unknown tid " << tid << ", dropping" << dendl; return; diff --git a/src/client/Client.h b/src/client/Client.h index 73b287cddd9dc..ccc9423ae219e 100644 --- a/src/client/Client.h +++ b/src/client/Client.h @@ -616,6 +616,7 @@ public: virtual void shutdown(); // messaging + void cancel_commands(const MDSMap& newmap); void handle_mds_map(const MConstRef& m); void handle_fs_map(const MConstRef& m); void handle_fs_map_user(const MConstRef& m); @@ -1357,6 +1358,8 @@ private: std::unique_ptr fsmap; std::unique_ptr fsmap_user; + // This mutex only protects command_table + ceph::mutex command_lock = ceph::make_mutex("Client::command_lock"); // MDS command state CommandTable command_table; -- 2.39.5