From: Sage Weil <sage@redhat.com>
Date: Fri, 10 Mar 2017 22:14:24 +0000 (-0500)
Subject: mgr/MgrClient: cope with disappearing perf_counters
X-Git-Tag: v12.0.2~252^2~14
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=a46690c6230b868b3a17cf6720119067891a1620;p=ceph.git

mgr/MgrClient: cope with disappearing perf_counters

Perfcounters can go away; deal with it.  This collapses
into a single loop, but it means that the mgr might
stop getting certain counters without explicit
notification.

Signed-off-by: Sage Weil <sage@redhat.com>
---

diff --git a/src/messages/MMgrReport.h b/src/messages/MMgrReport.h
index 825cb8308588..52090b256b35 100644
--- a/src/messages/MMgrReport.h
+++ b/src/messages/MMgrReport.h
@@ -55,7 +55,7 @@ WRITE_CLASS_ENCODER(PerfCounterType)
 
 class MMgrReport : public Message
 {
-  static const int HEAD_VERSION = 1;
+  static const int HEAD_VERSION = 2;
   static const int COMPAT_VERSION = 1;
 
 public:
@@ -65,6 +65,7 @@ public:
    * counter, it must inline the counter's schema here.
    */
   std::vector<PerfCounterType> declare_types;
+  std::vector<std::string> undeclare_types;
 
   // For all counters present, sorted by idx, output
   // as many bytes as are needed to represent them
@@ -82,18 +83,21 @@ public:
     ::decode(daemon_name, p);
     ::decode(declare_types, p);
     ::decode(packed, p);
+    if (header.version >= 2)
+      ::decode(undeclare_types, p);
   }
 
   void encode_payload(uint64_t features) override {
     ::encode(daemon_name, payload);
     ::encode(declare_types, payload);
     ::encode(packed, payload);
+    ::encode(undeclare_types, payload);
   }
 
   const char *get_type_name() const override { return "mgrreport"; }
   void print(ostream& out) const override {
-    out << get_type_name() << "(" << declare_types.size() << " "
-        << packed.length() << ")"; 
+    out << get_type_name() << "(+" << declare_types.size() << "-" << undeclare_types.size()
+        << " packed " << packed.length() << ")";
   }
 
   MMgrReport()
diff --git a/src/mgr/DaemonState.cc b/src/mgr/DaemonState.cc
index 132f722947b6..f83f9749de6c 100644
--- a/src/mgr/DaemonState.cc
+++ b/src/mgr/DaemonState.cc
@@ -110,6 +110,8 @@ void DaemonStateIndex::cull(entity_type_t daemon_type,
 void DaemonPerfCounters::update(MMgrReport *report)
 {
   dout(20) << "loading " << report->declare_types.size() << " new types, "
+	   << report->undeclare_types.size() << " old types, had "
+	   << types.size() << " types, got "
            << report->packed.length() << " bytes of data" << dendl;
 
   // Load any newly declared types
@@ -117,6 +119,10 @@ void DaemonPerfCounters::update(MMgrReport *report)
     types.insert(std::make_pair(t.path, t));
     declared_types.insert(t.path);
   }
+  // Remove any old types
+  for (const auto &t : report->undeclare_types) {
+    declared_types.erase(t);
+  }
 
   const auto now = ceph_clock_now();
 
diff --git a/src/mgr/MgrClient.cc b/src/mgr/MgrClient.cc
index 1defc2c9f0a3..5301253654ff 100644
--- a/src/mgr/MgrClient.cc
+++ b/src/mgr/MgrClient.cc
@@ -199,45 +199,45 @@ void MgrClient::send_report()
   pcc->with_counters([this, report](
         const PerfCountersCollection::CounterMap &by_path)
   {
-    bool const declared_all = (session->declared.size() == by_path.size());
-
-    if (!declared_all) {
-      for (const auto &i : by_path) {
-        auto path = i.first;
-        auto data = *(i.second);
-        
-        if (session->declared.count(path) == 0) {
-          PerfCounterType type;
-          type.path = path;
-          if (data.description) {
-            type.description = data.description;
-          }
-          if (data.nick) {
-            type.nick = data.nick;
-          }
-          type.type = data.type;
-          report->declare_types.push_back(std::move(type));
-          session->declared.insert(path);
-        }
+    ENCODE_START(1, 1, report->packed);
+    for (auto p = session->declared.begin(); p != session->declared.end(); ) {
+      if (by_path.count(*p) == 0) {
+	report->undeclare_types.push_back(*p);
+	ldout(cct,20) << __func__ << " undeclare " << *p << dendl;
+	p = session->declared.erase(p);
+      } else {
+	++p;
       }
     }
+    for (const auto &i : by_path) {
+      auto& path = i.first;
+      auto& data = *(i.second);
+
+      if (session->declared.count(path) == 0) {
+	ldout(cct,20) << __func__ << " declare " << path << dendl;
+	PerfCounterType type;
+	type.path = path;
+	if (data.description) {
+	  type.description = data.description;
+	}
+	if (data.nick) {
+	  type.nick = data.nick;
+	}
+	type.type = data.type;
+	report->declare_types.push_back(std::move(type));
+	session->declared.insert(path);
+      }
 
-    ldout(cct, 20) << by_path.size() << " counters, of which "
-             << report->declare_types.size() << " new" << dendl;
-
-    ENCODE_START(1, 1, report->packed);
-    for (const auto &path : session->declared) {
-      auto data = by_path.at(path);
-      ::encode(static_cast<uint64_t>(data->u64.read()),
-          report->packed);
-      if (data->type & PERFCOUNTER_LONGRUNAVG) {
-        ::encode(static_cast<uint64_t>(data->avgcount.read()),
-            report->packed);
-        ::encode(static_cast<uint64_t>(data->avgcount2.read()),
-            report->packed);
+      ::encode(static_cast<uint64_t>(data.u64.read()), report->packed);
+      if (data.type & PERFCOUNTER_LONGRUNAVG) {
+        ::encode(static_cast<uint64_t>(data.avgcount.read()), report->packed);
+        ::encode(static_cast<uint64_t>(data.avgcount2.read()), report->packed);
       }
     }
     ENCODE_FINISH(report->packed);
+
+    ldout(cct, 20) << by_path.size() << " counters, of which "
+		   << report->declare_types.size() << " new" << dendl;
   });
 
   ldout(cct, 20) << "encoded " << report->packed.length() << " bytes" << dendl;