mgr: templatize metrics collection interface#29214
Conversation
yep -- in a days time |
50aed6b to
0bcf020
Compare
0bcf020 to
d5a9b0d
Compare
|
will do the review today! sorry for the latency. |
I'll push an update (+rebase) soon -- requesting hold on to reviews. |
d5a9b0d to
cda07bd
Compare
|
@tchaikov rebased and updated. please take a look. |
|
@tchaikov pinging for reviews |
|
@vshankar i am still reviewing it. will finish the review tomorrow. |
tchaikov
left a comment
There was a problem hiding this comment.
i am still reviewing this PR..
src/messages/MMgrConfigure.h
Outdated
| uint32_t stats_threshold = 0; | ||
|
|
||
| std::map<OSDPerfMetricQuery, OSDPerfMetricLimits> osd_perf_metric_queries; | ||
| boost::optional<std::map<OSDPerfMetricQuery, OSDPerfMetricLimits>> osd_perf_metric_queries; |
There was a problem hiding this comment.
this is not backward compatible.
There was a problem hiding this comment.
true -- I had made changes to MgrClient::_send_configure() to use boost::optional which I reverted since it wasn't backward compatible. Forgot to revert this bit.
|
|
||
| uint64_t query_id; | ||
| bool notify = false; | ||
|
|
src/osd/OSD.cc
Outdated
| }, | ||
| [this](std::map<OSDPerfMetricQuery, OSDPerfMetricReport> *reports) { | ||
| get_perf_reports(reports); | ||
| [this]() { |
There was a problem hiding this comment.
nit, could remove () if no parameters will be passed to this lambda.
@tchaikov checking back on this. |
cda07bd to
8d8f260
Compare
|
@tchaikov sorry for the late update -- been tied up with other work. rebased + comments addressed. please take a look. |
8d8f260 to
1119c38
Compare
templatize metrics collection so as to reuse quering routines. `MetricCollector` can be subclassed and along with implementing ` process_reports()` to process incoming metrics data. also, generalize metrics data in `MMgrReport` and metric query configuration in `MMgrConfigure`. Signed-off-by: Venky Shankar <vshankar@redhat.com>
1119c38 to
efcebe1
Compare
|
rebased and updated |
batrick
left a comment
There was a problem hiding this comment.
Makes sense to me. Don't know if I fully understand some of the boost parts but the idea looks right.
* refs/pull/29214/head: mgr: templatize/generalize metrics collection interface Reviewed-by: Patrick Donnelly <pdonnell@redhat.com>
|
\o/ thanks @liewegas |
[This change was initially a part of
fs topwork where MDS forwards metrics information to the manager]Metric collection for a ceph entity can inherit from
MetricCollectorclass (seeOSDPerfMetricCollector) which provides basic{add,remove}_query(templated) interface.process_reportsis a virtual function which needs to be overridden to processMetricPayloadvariant (which is a part ofMMgrReport::metric_report_message.