New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mon: maintain the "cluster" PerfCounters when using ceph-mgr #16249

Merged
merged 1 commit into from Jul 11, 2017

Conversation

Projects
None yet
3 participants
@gregsfortytwo
Member

gregsfortytwo commented Jul 10, 2017

Fixes: http://tracker.ceph.com/issues/20562

Signed-off-by: Greg Farnum gfarnum@redhat.com

@gregsfortytwo

This comment has been minimized.

Member

gregsfortytwo commented Jul 10, 2017

Just copied the update_logger() from PGMonitor and switched it over to use the digest instead of a pg_map.
Only odd bit is that since we managed to squash out most of the stats users, we don't have a total pg count being relayed anywhere — have to count it up from the per-pool mapping.

@liewegas liewegas added this to the luminous milestone Jul 10, 2017

unsigned active = 0, active_clean = 0, peering = 0;
for (ceph::unordered_map<int,int>::iterator p = digest.num_pg_by_state.begin();
p != digest.num_pg_by_state.end();
++p) {

This comment has been minimized.

@liewegas

liewegas Jul 10, 2017

Member

could use auto here

This comment has been minimized.

@gregsfortytwo

gregsfortytwo Jul 10, 2017

Member

Heh, copy-pasta. Will switch it for more better code. :)

@@ -136,6 +178,7 @@ version_t MgrStatMonitor::get_trim_to()
void MgrStatMonitor::on_active()
{
update_logger();

This comment has been minimized.

@liewegas

liewegas Jul 10, 2017

Member

Oh, I think this shoudl be guarded by the osdmap.require_osd_release >= CEPH_RELEASE_LUMINOUS so that we don't step on the PGMonitor stats during upgrade

This comment has been minimized.

@gregsfortytwo

gregsfortytwo Jul 10, 2017

Member

Good catch, was thinking that earlier and forgot...

This comment has been minimized.

@jecluis

jecluis Jul 11, 2017

Member

update_logger() is already guarding for the same thing. Adding the check here, while a bit more verbose, would be redundant at this point. Otherwise, maybe check it here and assert on the function?

This comment has been minimized.

@jecluis

jecluis Jul 11, 2017

Member

Well, doh. Commented on a thread about a former state of the PR, apparently. Please disregard.

mon: maintain the "cluster" PerfCounters when using ceph-mgr instead …
…of PGMonitor

Fixes: http://tracker.ceph.com/issues/20562

Signed-off-by: Greg Farnum <gfarnum@redhat.com>
@gregsfortytwo

This comment has been minimized.

Member

gregsfortytwo commented Jul 10, 2017

@liewegas, updated. (Put the luminous check in update_logger() so it gets caught by both callers.)

@liewegas liewegas merged commit eaf5d03 into ceph:master Jul 11, 2017

4 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details
make check (arm64) make check succeeded
Details

@gregsfortytwo gregsfortytwo deleted the gregsfortytwo:wip-20562-cluster-stats branch Jul 21, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment