From dc415f1ae09a308bd448614934a4c168eb9cf07b Mon Sep 17 00:00:00 2001 From: John Spray Date: Sat, 23 Sep 2017 11:55:55 -0400 Subject: [PATCH] mgr: store declared_types in MgrSession Because we don't (yet) properly prevent multiple sessions from daemons reporting the same name (e.g. rgws), storing it in the DaemonPerfCounters meant that one daemon's report was referring to another daemon's set of reported types. This should always have been a property of the session. The behaviour will still be ugly when multiple daemons are using the same name (stomping on each other's stats/statsu) but it shouldn't crash. Fixes: http://tracker.ceph.com/issues/21197 Signed-off-by: John Spray --- src/mgr/DaemonState.cc | 12 +++++++++--- src/mgr/DaemonState.h | 7 ------- src/mgr/MgrSession.h | 2 ++ src/mgr/PyModules.cc | 4 +++- 4 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/mgr/DaemonState.cc b/src/mgr/DaemonState.cc index 8da32174735..a7b8f572e16 100644 --- a/src/mgr/DaemonState.cc +++ b/src/mgr/DaemonState.cc @@ -13,6 +13,8 @@ #include "DaemonState.h" +#include "MgrSession.h" + #define dout_context g_ceph_context #define dout_subsys ceph_subsys_mgr #undef dout_prefix @@ -123,14 +125,18 @@ void DaemonPerfCounters::update(MMgrReport *report) << types.size() << " types, got " << report->packed.length() << " bytes of data" << dendl; + // Retrieve session state + MgrSessionRef session(static_cast( + report->get_connection()->get_priv())); + // Load any newly declared types for (const auto &t : report->declare_types) { types.insert(std::make_pair(t.path, t)); - declared_types.insert(t.path); + session->declared_types.insert(t.path); } // Remove any old types for (const auto &t : report->undeclare_types) { - declared_types.erase(t); + session->declared_types.erase(t); } const auto now = ceph_clock_now(); @@ -138,7 +144,7 @@ void DaemonPerfCounters::update(MMgrReport *report) // Parse packed data according to declared set of types bufferlist::iterator p = report->packed.begin(); DECODE_START(1, p); - for (const auto &t_path : declared_types) { + for (const auto &t_path : session->declared_types) { const auto &t = types.at(t_path); uint64_t val = 0; uint64_t avgcount = 0; diff --git a/src/mgr/DaemonState.h b/src/mgr/DaemonState.h index 7dbdeb657cd..846ce5dd8d9 100644 --- a/src/mgr/DaemonState.h +++ b/src/mgr/DaemonState.h @@ -74,18 +74,11 @@ class DaemonPerfCounters std::map instances; - // FIXME: this state is really local to DaemonServer, it's part - // of the protocol rather than being part of what other classes - // mgiht want to read. Maybe have a separate session object - // inside DaemonServer instead of stashing session-ish state here? - std::set declared_types; - void update(MMgrReport *report); void clear() { instances.clear(); - declared_types.clear(); } }; diff --git a/src/mgr/MgrSession.h b/src/mgr/MgrSession.h index 72afd7a6767..c52e2e17776 100644 --- a/src/mgr/MgrSession.h +++ b/src/mgr/MgrSession.h @@ -23,6 +23,8 @@ struct MgrSession : public RefCountedObject { // mon caps are suitably generic for mgr MonCap caps; + std::set declared_types; + MgrSession(CephContext *cct) : RefCountedObject(cct, 0) {} ~MgrSession() override {} }; diff --git a/src/mgr/PyModules.cc b/src/mgr/PyModules.cc index 0fdf21654ab..003a0b9bee5 100644 --- a/src/mgr/PyModules.cc +++ b/src/mgr/PyModules.cc @@ -761,7 +761,9 @@ PyObject* PyModules::get_perf_schema_python( f.open_object_section(daemon_name.str().c_str()); Mutex::Locker l(state->lock); - for (auto typestr : state->perf_counters.declared_types) { + for (const auto &ctr_inst_iter : state->perf_counters.instances) { + const auto &typestr = ctr_inst_iter.first; + f.open_object_section(typestr.c_str()); auto type = state->perf_counters.types[typestr]; f.dump_string("description", type.description); -- 2.39.5