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>
(cherry picked from commit dc415f1ae09a308bd448614934a4c168eb9cf07b)

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 f7da497a58a8746d1cbdf147953a8e4e87eb117c..fb1ce7400459d0cf651b90ec77de4bbfe00e9893 100644 (file)
--- a/src/mgr/PyModules.cc
+++ b/src/mgr/PyModules.cc
@@ -771,7 +771,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);