From: John Spray Date: Mon, 19 Sep 2016 19:35:28 +0000 (+0100) Subject: common: refactor CommandTable X-Git-Tag: v11.0.1~60^2~16 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=2210772ed87ca281a60549ec40b1a1abe271d6e7;p=ceph.git common: refactor CommandTable Avoid handling out raw pointers to the ops, which are in fact owned by the table. Signed-off-by: John Spray --- diff --git a/src/client/Client.cc b/src/client/Client.cc index 0da3fd9cd7cc..58bc293982e8 100644 --- a/src/client/Client.cc +++ b/src/client/Client.cc @@ -2545,21 +2545,21 @@ void Client::handle_mds_map(MMDSMap* m) // Cancel any commands for missing or laggy GIDs std::list cancel_ops; - auto commands = command_table.get_commands(); + auto &commands = command_table.get_commands(); for (const auto &i : commands) { - const MDSCommandOp *op = i.second; - const mds_gid_t op_mds_gid = op->mds_gid; + auto &op = i.second; + const mds_gid_t op_mds_gid = op.mds_gid; if (mdsmap->is_dne_gid(op_mds_gid) || mdsmap->is_laggy_gid(op_mds_gid)) { ldout(cct, 1) << __func__ << ": cancelling command op " << i.first << dendl; cancel_ops.push_back(i.first); - if (op->outs) { + if (op.outs) { std::ostringstream ss; ss << "MDS " << op_mds_gid << " went away"; - *(op->outs) = ss.str(); + *(op.outs) = ss.str(); } - op->con->mark_down(); - if (op->on_finish) { - op->on_finish->complete(-ETIMEDOUT); + op.con->mark_down(); + if (op.on_finish) { + op.on_finish->complete(-ETIMEDOUT); } } } @@ -5506,21 +5506,21 @@ int Client::mds_command( ConnectionRef conn = messenger->get_connection(inst); // Generate MDSCommandOp state - MDSCommandOp *op = command_table.start_command(); + 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; + << " tid=" << op.tid << cmd << dendl; // Construct and send MCommand - MCommand *m = op->get_message(monclient->get_fsid()); + MCommand *m = op.get_message(monclient->get_fsid()); conn->send_message(m); } gather.activate(); @@ -5534,22 +5534,22 @@ void Client::handle_command_reply(MCommandReply *m) ldout(cct, 10) << __func__ << ": tid=" << m->get_tid() << dendl; - MDSCommandOp *op = command_table.get_command(tid); - if (op == nullptr) { + if (!command_table.exists(tid)) { ldout(cct, 1) << __func__ << ": unknown tid " << tid << ", dropping" << dendl; m->put(); return; } - if (op->outbl) { - op->outbl->claim(m->get_data()); + auto &op = command_table.get_command(tid); + if (op.outbl) { + op.outbl->claim(m->get_data()); } - if (op->outs) { - *op->outs = m->rs; + if (op.outs) { + *op.outs = m->rs; } - if (op->on_finish) { - op->on_finish->complete(m->r); + if (op.on_finish) { + op.on_finish->complete(m->r); } command_table.erase(tid); diff --git a/src/common/CommandTable.h b/src/common/CommandTable.h index 6be67578eafe..6aab1b8c24b1 100644 --- a/src/common/CommandTable.h +++ b/src/common/CommandTable.h @@ -3,7 +3,7 @@ /* * Ceph - scalable distributed file system * - * Copyright (C) 2004-2006 Sage Weil + * Copyright (C) 2016 Red Hat Inc * * This is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -39,15 +39,21 @@ class CommandOp return m; } - CommandOp(const ceph_tid_t t) : tid(t) {} + CommandOp(const ceph_tid_t t) : tid(t), on_finish(nullptr), + outbl(nullptr), outs(nullptr) {} + CommandOp() : tid(0), on_finish(nullptr), outbl(nullptr), outs(nullptr) {} }; +/** + * Hold client-side state for a collection of in-flight commands + * to a remote service. + */ template class CommandTable { protected: ceph_tid_t last_tid; - std::map commands; + std::map commands; public: @@ -55,34 +61,36 @@ public: : last_tid(0) {} - T* start_command() + ~CommandTable() + { + assert(commands.empty()); + } + + T& start_command() { ceph_tid_t tid = last_tid++; - auto cmd = new T(tid); - commands[tid] = cmd; - - return cmd; + commands.insert(std::make_pair(tid, T(tid)) ); + + return commands.at(tid); } - const std::map &get_commands() const + const std::map &get_commands() const { return commands; } - T* get_command(ceph_tid_t tid) + bool exists(ceph_tid_t tid) const + { + return commands.count(tid) > 0; + } + + T& get_command(ceph_tid_t tid) { - auto result = commands.find(tid); - if (result == commands.end()) { - return nullptr; - } else { - return result->second; - } + return commands.at(tid); } void erase(ceph_tid_t tid) { - auto ptr = commands.at(tid); - delete ptr; commands.erase(tid); } }; diff --git a/src/mgr/MgrClient.cc b/src/mgr/MgrClient.cc index d0baeb4d3744..bc272d343811 100644 --- a/src/mgr/MgrClient.cc +++ b/src/mgr/MgrClient.cc @@ -109,8 +109,8 @@ bool MgrClient::handle_mgr_map(MMgrMap *m) auto commands = command_table.get_commands(); for (const auto &i : commands) { // FIXME be nicer, retarget command on new mgr? - if (i.second->on_finish != nullptr) { - i.second->on_finish->complete(-ETIMEDOUT); + if (i.second.on_finish != nullptr) { + i.second.on_finish->complete(-ETIMEDOUT); } erase_cmds.push_back(i.first); } @@ -296,15 +296,15 @@ int MgrClient::start_command(const vector& cmd, const bufferlist& inbl, assert(map.epoch > 0); - MgrCommand *op = command_table.start_command(); - op->cmd = cmd; - op->inbl = inbl; - op->outbl = outbl; - op->outs = outs; - op->on_finish = onfinish; + auto &op = command_table.start_command(); + op.cmd = cmd; + op.inbl = inbl; + op.outbl = outbl; + op.outs = outs; + op.on_finish = onfinish; // Leaving fsid argument null because it isn't used. - MCommand *m = op->get_message({}); + MCommand *m = op.get_message({}); assert(session); assert(session->con); session->con->send_message(m); @@ -319,24 +319,24 @@ bool MgrClient::handle_command_reply(MCommandReply *m) ldout(cct, 20) << *m << dendl; const auto tid = m->get_tid(); - const auto op = command_table.get_command(tid); - if (op == nullptr) { + if (!command_table.exists(tid)) { ldout(cct, 4) << "handle_command_reply tid " << m->get_tid() << " not found" << dendl; m->put(); return true; } - if (op->outbl) { - op->outbl->claim(m->get_data()); + auto &op = command_table.get_command(tid); + if (op.outbl) { + op.outbl->claim(m->get_data()); } - if (op->outs) { - *(op->outs) = m->rs; + if (op.outs) { + *(op.outs) = m->rs; } - if (op->on_finish) { - op->on_finish->complete(m->r); + if (op.on_finish) { + op.on_finish->complete(m->r); } command_table.erase(tid); diff --git a/src/mgr/MgrClient.h b/src/mgr/MgrClient.h index 705dea8c4ef3..396bb7473b0b 100644 --- a/src/mgr/MgrClient.h +++ b/src/mgr/MgrClient.h @@ -44,6 +44,7 @@ class MgrCommand : public CommandOp public: MgrCommand(ceph_tid_t t) : CommandOp(t) {} + MgrCommand() : CommandOp() {} }; class MgrClient : public Dispatcher