Skip to content
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

mgr/prometheus: add missing 'deep' state to PG_STATES in ceph-mgr prometheus plugin #18890

Merged
merged 2 commits into from
Nov 23, 2017

Conversation

pjjw
Copy link
Contributor

@pjjw pjjw commented Nov 12, 2017

without this the endpoint throws 500s when any PG is doing a deep scrub

@pjjw pjjw force-pushed the pjjw/prom-deep-scrub-state-missing branch from 69b48fc to 12d4c6e Compare November 12, 2017 00:58
@liewegas liewegas changed the title add missing 'deep' state to PG_STATES in ceph-mgr prometheus plugin mgr/prometheus: add missing 'deep' state to PG_STATES in ceph-mgr prometheus plugin Nov 12, 2017
@liewegas
Copy link
Member

Can we make the module not throw 500s if there is an unrecognized pg state?

@jcsp
Copy link
Contributor

jcsp commented Nov 13, 2017

@pjjw I've created a ticket to track this http://tracker.ceph.com/issues/22116, please could you add a Fixes: line to your commit message that references it.

@jan--f
Copy link
Contributor

jan--f commented Nov 13, 2017

Could you also add a "Deep" section to doc/rados/operations/pg-states.rst? That is where I extracted the initial list of PG States. Maybe something like

Deep
    Ceph is checking the placement group data for inconsistencies.

@jan--f
Copy link
Contributor

jan--f commented Nov 13, 2017

Can we make the module not throw 500s if there is an unrecognized pg state?

Agreed. #18903

@tchaikov
Copy link
Contributor

@jcsp shall we backport the fixes of prometheus to luminous?

@jan--f
Copy link
Contributor

jan--f commented Nov 15, 2017

@tchaikov I have this on my plate. Is it urgent? Otherwise I'd like to give #18847 a test run and extend the prometheus module accordingly.

@tchaikov
Copy link
Contributor

@jan--f probably not that urgent. i just want to understand what level of stableness we want to support in the LTS' mgr plugin.

@jcsp
Copy link
Contributor

jcsp commented Nov 15, 2017

@pjjw please can you update the commit message to add the line "Fixes: http://tracker.ceph.com/issues/22116", so that this is easier to track when backporting etc.

@jcsp
Copy link
Contributor

jcsp commented Nov 15, 2017

@tchaikov yes, we should backport, but no special urgency

@pjjw pjjw force-pushed the pjjw/prom-deep-scrub-state-missing branch 2 times, most recently from f1842d0 to 07d533d Compare November 16, 2017 23:17
@pjjw
Copy link
Contributor Author

pjjw commented Nov 16, 2017

Sorry for radio silence, this is usually weekend work for me

@jan--f I took a stab at describing the 'deep' state, and also added some detail to the 'scrubbing' state (based on my limited understanding) - let me know if that doc change looks good.

@pjjw pjjw force-pushed the pjjw/prom-deep-scrub-state-missing branch 3 times, most recently from 600949f to ea4252e Compare November 16, 2017 23:43
…metheus plugin

without this the endpoint throws 500s when any PG is doing a deep scrub.

Signed-off-by: Peter Woodman <peter@shortbus.org>
Fixes: http://tracker.ceph.com/issues/22116
…ing' state to differentiate from 'deep' state

Signed-off-by: Peter Woodman <peter@shortbus.org>
@pjjw pjjw force-pushed the pjjw/prom-deep-scrub-state-missing branch from ea4252e to dff64a2 Compare November 18, 2017 00:38
@pjjw
Copy link
Contributor Author

pjjw commented Nov 20, 2017

anything else i need to do to get this merged? slightly unclear from the contributing doc if I need to send a git patch over email.

@jcsp
Copy link
Contributor

jcsp commented Nov 20, 2017

@pjjw you're all set, this is just waiting until someone picks it up and runs it through a test suite

@tchaikov
Copy link
Contributor

adding needs-qa so it'd be easier for someone to notice this PR. =)

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

Successfully merging this pull request may close these issues.

5 participants