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 <john.spray@redhat.com>

diff --git a/src/mgr/DaemonState.cc b/src/mgr/DaemonState.cc
index 8da321747357593f66cc5d1bab27a1211beae70d..a7b8f572e1614b3f0a0b420f7685ccdeccb5ecaa 100644 (file)
--- 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<MgrSession*>(
+        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 7dbdeb657cda06c53562cea63a7bf092144ebb2f..846ce5dd8d9fcbe6e6580eecb55f2c7146c44ed2 100644 (file)
--- a/src/mgr/DaemonState.h
+++ b/src/mgr/DaemonState.h
@@ -74,18 +74,11 @@ class DaemonPerfCounters
 
   std::map<std::string, PerfCounterInstance> 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<std::string> 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 72afd7a676728f0066c65ebd5b71413ba52e612e..c52e2e177761ac979eeea3791d9c69aa05c7f764 100644 (file)
--- 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<std::string> declared_types;
+
   MgrSession(CephContext *cct) : RefCountedObject(cct, 0) {}
   ~MgrSession() override {}
 };

diff --git a/src/mgr/PyModules.cc b/src/mgr/PyModules.cc
index 0fdf21654ab455125f4cf82a5dc1ad7e26aa2952..003a0b9bee5328301ed4a06e993c84b5a65c3893 100644 (file)
--- 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);