From 2ebab2f19cdd44f4c568eb96388fc4296f0f1814 Mon Sep 17 00:00:00 2001 From: John Spray Date: Mon, 25 Sep 2017 11:14:57 -0400 Subject: [PATCH] mgr: respect perf counter prio_adjust in MgrClient This awkwardly involves re-ordering some definitions in perf_counters.h in order to refer to the prio names defined in PerfCountersBuilder. Signed-off-by: John Spray (cherry picked from commit 88163749b572ffd2bfe0850136fad5dbed2a9180) --- src/common/perf_counters.cc | 7 +- src/common/perf_counters.h | 165 ++++++++++++++++++++---------------- src/mgr/MgrClient.cc | 40 ++++++--- 3 files changed, 121 insertions(+), 91 deletions(-) diff --git a/src/common/perf_counters.cc b/src/common/perf_counters.cc index 34c6d5e294286..048c93a64334a 100644 --- a/src/common/perf_counters.cc +++ b/src/common/perf_counters.cc @@ -53,7 +53,7 @@ void PerfCountersCollection::add(class PerfCounters *l) path += "."; path += data.name; - by_path[path] = &data; + by_path[path] = {&data, l}; } } @@ -396,10 +396,7 @@ void PerfCounters::dump_formatted_generic(Formatter *f, bool schema, } else { f->dump_string("nick", ""); } - int p = std::max(std::min(d->prio + prio_adjust, - (int)PerfCountersBuilder::PRIO_CRITICAL), - 0); - f->dump_int("priority", p); + f->dump_int("priority", get_adjusted_priority(d->prio)); f->close_section(); } else { if (d->type & PERFCOUNTER_LONGRUNAVG) { diff --git a/src/common/perf_counters.h b/src/common/perf_counters.h index 10cd0450e5230..846e407ad2f0d 100644 --- a/src/common/perf_counters.h +++ b/src/common/perf_counters.h @@ -42,6 +42,80 @@ enum perfcounter_type_d : uint8_t }; +/* Class for constructing a PerfCounters object. + * + * This class performs some validation that the parameters we have supplied are + * correct in create_perf_counters(). + * + * In the future, we will probably get rid of the first/last arguments, since + * PerfCountersBuilder can deduce them itself. + */ +class PerfCountersBuilder +{ +public: + PerfCountersBuilder(CephContext *cct, const std::string &name, + int first, int last); + ~PerfCountersBuilder(); + + // prio values: higher is better, and higher values get included in + // 'ceph daemonperf' (and similar) results. + // Use of priorities enables us to add large numbers of counters + // internally without necessarily overwhelming consumers. + enum { + PRIO_CRITICAL = 10, + // 'interesting' is the default threshold for `daemonperf` output + PRIO_INTERESTING = 8, + // `useful` is the default threshold for transmission to ceph-mgr + // and inclusion in prometheus/influxdb plugin output + PRIO_USEFUL = 5, + PRIO_UNINTERESTING = 2, + PRIO_DEBUGONLY = 0, + }; + void add_u64(int key, const char *name, + const char *description=NULL, const char *nick = NULL, + int prio=0); + void add_u64_counter(int key, const char *name, + const char *description=NULL, + const char *nick = NULL, + int prio=0); + void add_u64_avg(int key, const char *name, + const char *description=NULL, + const char *nick = NULL, + int prio=0); + void add_time(int key, const char *name, + const char *description=NULL, + const char *nick = NULL, + int prio=0); + void add_time_avg(int key, const char *name, + const char *description=NULL, + const char *nick = NULL, + int prio=0); + void add_u64_counter_histogram( + int key, const char* name, + PerfHistogramCommon::axis_config_d x_axis_config, + PerfHistogramCommon::axis_config_d y_axis_config, + const char *description=NULL, + const char* nick = NULL, + int prio=0); + + void set_prio_default(int prio_) + { + prio_default = prio_; + } + + PerfCounters* create_perf_counters(); +private: + PerfCountersBuilder(const PerfCountersBuilder &rhs); + PerfCountersBuilder& operator=(const PerfCountersBuilder &rhs); + void add_impl(int idx, const char *name, + const char *description, const char *nick, int prio, int ty, + unique_ptr> histogram = nullptr); + + PerfCounters *m_perf_counters; + + int prio_default = 0; +}; + /* * A PerfCounters object is usually associated with a single subsystem. * It contains counters which we modify to track performance and throughput @@ -179,6 +253,12 @@ public: prio_adjust = p; } + int get_adjusted_priority(int p) const { + return std::max(std::min(p + prio_adjust, + (int)PerfCountersBuilder::PRIO_CRITICAL), + 0); + } + private: PerfCounters(CephContext *cct, const std::string &name, int lower_bound, int upper_bound); @@ -240,8 +320,17 @@ public: dump_formatted_generic(f, schema, true, logger, counter); } + // A reference to a perf_counter_data_any_d, with an accompanying + // pointer to the enclosing PerfCounters, in order that the consumer + // can see the prio_adjust + class PerfCounterRef + { + public: + PerfCounters::perf_counter_data_any_d *data; + PerfCounters *perf_counters; + }; typedef std::map CounterMap; + PerfCounterRef> CounterMap; void with_counters(std::function) const; @@ -257,83 +346,11 @@ private: perf_counters_set_t m_loggers; - std::map by_path; + CounterMap by_path; friend class PerfCountersCollectionTest; }; -/* Class for constructing a PerfCounters object. - * - * This class performs some validation that the parameters we have supplied are - * correct in create_perf_counters(). - * - * In the future, we will probably get rid of the first/last arguments, since - * PerfCountersBuilder can deduce them itself. - */ -class PerfCountersBuilder -{ -public: - PerfCountersBuilder(CephContext *cct, const std::string &name, - int first, int last); - ~PerfCountersBuilder(); - // prio values: higher is better, and higher values get included in - // 'ceph daemonperf' (and similar) results. - // Use of priorities enables us to add large numbers of counters - // internally without necessarily overwhelming consumers. - enum { - PRIO_CRITICAL = 10, - // 'interesting' is the default threshold for `daemonperf` output - PRIO_INTERESTING = 8, - // `useful` is the default threshold for transmission to ceph-mgr - // and inclusion in prometheus/influxdb plugin output - PRIO_USEFUL = 5, - PRIO_UNINTERESTING = 2, - PRIO_DEBUGONLY = 0, - }; - void add_u64(int key, const char *name, - const char *description=NULL, const char *nick = NULL, - int prio=0); - void add_u64_counter(int key, const char *name, - const char *description=NULL, - const char *nick = NULL, - int prio=0); - void add_u64_avg(int key, const char *name, - const char *description=NULL, - const char *nick = NULL, - int prio=0); - void add_time(int key, const char *name, - const char *description=NULL, - const char *nick = NULL, - int prio=0); - void add_time_avg(int key, const char *name, - const char *description=NULL, - const char *nick = NULL, - int prio=0); - void add_u64_counter_histogram( - int key, const char* name, - PerfHistogramCommon::axis_config_d x_axis_config, - PerfHistogramCommon::axis_config_d y_axis_config, - const char *description=NULL, - const char* nick = NULL, - int prio=0); - - void set_prio_default(int prio_) - { - prio_default = prio_; - } - - PerfCounters* create_perf_counters(); -private: - PerfCountersBuilder(const PerfCountersBuilder &rhs); - PerfCountersBuilder& operator=(const PerfCountersBuilder &rhs); - void add_impl(int idx, const char *name, - const char *description, const char *nick, int prio, int ty, - unique_ptr> histogram = nullptr); - - PerfCounters *m_perf_counters; - - int prio_default = 0; -}; #endif diff --git a/src/mgr/MgrClient.cc b/src/mgr/MgrClient.cc index dd35e27141720..0e4b01931ef2e 100644 --- a/src/mgr/MgrClient.cc +++ b/src/mgr/MgrClient.cc @@ -227,27 +227,43 @@ void MgrClient::send_report() pcc->with_counters([this, report]( const PerfCountersCollection::CounterMap &by_path) { + // Helper for checking whether a counter should be included auto include_counter = [this]( - const PerfCounters::perf_counter_data_any_d &ctr) + const PerfCounters::perf_counter_data_any_d &ctr, + const PerfCounters &perf_counters) { - return ctr.prio >= (int)stats_threshold; + return perf_counters.get_adjusted_priority(ctr.prio) >= (int)stats_threshold; + }; + + // Helper for cases where we want to forget a counter + auto undeclare = [report, this](const std::string &path) + { + report->undeclare_types.push_back(path); + ldout(cct,20) << " undeclare " << path << dendl; + session->declared.erase(path); }; ENCODE_START(1, 1, report->packed); + + // Find counters that no longer exist, and undeclare them for (auto p = session->declared.begin(); p != session->declared.end(); ) { - if (by_path.count(*p) == 0 || !include_counter(*(by_path.at(*p)))) { - report->undeclare_types.push_back(*p); - ldout(cct,20) << __func__ << " undeclare " << *p << dendl; - p = session->declared.erase(p); - } else { - ++p; + const auto &path = *(p++); + if (by_path.count(path) == 0) { + undeclare(path); } } + for (const auto &i : by_path) { auto& path = i.first; - auto& data = *(i.second); - - if (!include_counter(data)) { + auto& data = *(i.second.data); + auto& perf_counters = *(i.second.perf_counters); + + // Find counters that still exist, but are no longer permitted by + // stats_threshold + if (!include_counter(data, perf_counters)) { + if (session->declared.count(path)) { + undeclare(path); + } continue; } @@ -262,7 +278,7 @@ void MgrClient::send_report() type.nick = data.nick; } type.type = data.type; - type.priority = data.prio; + type.priority = perf_counters.get_adjusted_priority(data.prio); report->declare_types.push_back(std::move(type)); session->declared.insert(path); } -- 2.39.5