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/dashboard: monitoring:Implement BlueStore onode hit/miss counters into the dashboard #44294

Merged
merged 1 commit into from Jan 11, 2022

Conversation

aaSharma14
Copy link
Contributor

@aaSharma14 aaSharma14 commented Dec 13, 2021

Provide the details pulled from Bluestore stats in order to display the onode hit/miss counters

Depends on #44334
Fixes: https://tracker.ceph.com/issues/53577
Signed-off-by: Aashish Sharma aasharma@redhat.com

Screenshot from 2022-01-05 13-20-05
Screenshot from 2022-01-05 13-20-05(1)

Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

@avanthakkar
Copy link
Contributor

Some unit-test would be nice to have @aaSharma14

@github-actions github-actions bot added this to In progress in Dashboard Dec 15, 2021
@aaSharma14
Copy link
Contributor Author

Some unit-test would be nice to have @aaSharma14

Done. Thanks @avanthakkar

@pereman2
Copy link
Contributor

jenkins test make check

Copy link
Contributor

@pereman2 pereman2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

Dashboard automation moved this from In progress to Reviewer approved Dec 15, 2021
@aaSharma14
Copy link
Contributor Author

jenkins test make check

@epuertat
Copy link
Member

@neha-ojha @ljflores @markhpc folks, what do you think about this one?

@alfonsomthd
Copy link
Contributor

jenkins test dashboard

@aaSharma14
Copy link
Contributor Author

jenkins test dashboard

Copy link
Contributor

@ljflores ljflores left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

I want to mention that there is another PR open right now (#44334) that shortens the names of the BlueStore onode hit/miss counters when printed. I don't think that would affect this PR, but linking it just in case.

@aaSharma14
Copy link
Contributor Author

Looks great!

I want to mention that there is another PR open right now (#44334) that shortens the names of the BlueStore onode hit/miss counters when printed. I don't think that would affect this PR, but linking it just in case.

Thanks @ljflores , Yes, in this case we need to change the metrics name here in this PR as well. We can merge this one after #44334 gets merged.

@aaSharma14 aaSharma14 changed the title mgr/dashboard: monitoring:Implement BlueStore onode hit/miss counters into the dashboard [AFTER #44334 ] mgr/dashboard: monitoring:Implement BlueStore onode hit/miss counters into the dashboard Dec 20, 2021
@ljflores
Copy link
Contributor

@aaSharma14 the branch needs to be updated

@aaSharma14 aaSharma14 changed the title [AFTER #44334 ] mgr/dashboard: monitoring:Implement BlueStore onode hit/miss counters into the dashboard mgr/dashboard: monitoring:Implement BlueStore onode hit/miss counters into the dashboard Dec 21, 2021
@aaSharma14
Copy link
Contributor Author

jenkins test dashboard cephadm

@aaSharma14 aaSharma14 force-pushed the feature-bluestore-onode branch 3 times, most recently from 59ab38b to 917649d Compare December 22, 2021 12:54
@ljflores
Copy link
Contributor

@epuertat sounds good

Copy link
Member

@neha-ojha neha-ojha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great!

monitoring/grafana/dashboards/osd-device-details.json Outdated Show resolved Hide resolved
monitoring/grafana/dashboards/osd-device-details.json Outdated Show resolved Hide resolved
@epuertat
Copy link
Member

Now I check the time-series chart again, being both series monotonic counters, I realize that if the hit/miss ratio is uneven (let's say 90% hit ratio, or 90% miss ratio), over time the smallest metric will be almost zero, and a temporal chart won't be very informative.

A possibility would be to display the hit-miss ratio over time. There's this Grafana Stat Panel widget that we don't use, and allows us printing a percentage (e.g.: hit ratio), a trend in the background, and also a threshold-driven color.

image

And coming back to the pie chart, we could replace it with the built-in gauge panel (which is great for displaying a binary dataset). One issue with pie-charts (vs. doughnut charts or gauges), is that for conveying information they rely on areas, and areas are squared values, so they actually display information^2. Gauges also allow defining thresholds, so we can visually tell users whether a 20% hit rate is actually a good (green) or bad (red) value.

image

So, summarizing my point:

  • Can we have 1 new chart instead of 2?
  • Can that be a gauge (if we don't mind temporal trends) or a Stat Panel otherwise?

@aaSharma14 aaSharma14 moved this from Reviewer approved to Review in progress in Dashboard Jan 4, 2022
@alfonsomthd alfonsomthd moved this from Review in progress to In progress in Dashboard Jan 4, 2022
@aaSharma14
Copy link
Contributor Author

Now I check the time-series chart again, being both series monotonic counters, I realize that if the hit/miss ratio is uneven (let's say 90% hit ratio, or 90% miss ratio), over time the smallest metric will be almost zero, and a temporal chart won't be very informative.

A possibility would be to display the hit-miss ratio over time. There's this Grafana Stat Panel widget that we don't use, and allows us printing a percentage (e.g.: hit ratio), a trend in the background, and also a threshold-driven color.

image

And coming back to the pie chart, we could replace it with the built-in gauge panel (which is great for displaying a binary dataset). One issue with pie-charts (vs. doughnut charts or gauges), is that for conveying information they rely on areas, and areas are squared values, so they actually display information^2. Gauges also allow defining thresholds, so we can visually tell users whether a 20% hit rate is actually a good (green) or bad (red) value.

image

So, summarizing my point:

  • Can we have 1 new chart instead of 2?
  • Can that be a gauge (if we don't mind temporal trends) or a Stat Panel otherwise?

Thank you for the suggestion @epuertat . I am currently implementing a gauge panel for this one. In any case what is the ideal onode Hits ratio ? so we can display a warning or error scenario if the ratio goes below that value. Eg. I have kept it 50% as of now, so if the ratio goes below 50%, it would show that in red color and otherwise green. @ljflores @neha-ojha Do you guys have some insight about this?

Screenshot from 2022-01-04 15-31-39

@ljflores
Copy link
Contributor

ljflores commented Jan 4, 2022

In any case what is the ideal onode Hits ratio ?

@aaSharma14 @epuertat Just talked with @aclamk about this. He said he uses the threshold of 75%, where above that is fine, and below that is something to investigate.

This is looking great!

Edit: I switched the "above" and "below" scenarios. Apologies, the comment is correct now.

@aaSharma14
Copy link
Contributor Author

In any case what is the ideal onode Hits ratio ?

@aaSharma14 @epuertat Just talked with @aclamk about this. He said he uses the threshold of 75%, where above that is fine, and below that is something to investigate.

This is looking great!

Edit: I switched the "above" and "below" scenarios. Apologies, the comment is correct now.

Thankyou @ljflores for the information, I have updated the PR accordingly.

… into the dashboard

Provide the details pulled from Bluestore stats in order to display the onode hit/miss counters

Fixes: https://tracker.ceph.com/issues/53577
Signed-off-by: Aashish Sharma <aasharma@redhat.com>
@aaSharma14 aaSharma14 moved this from In progress to Review in progress in Dashboard Jan 5, 2022
@aaSharma14
Copy link
Contributor Author

jenkins test dashboard

@aaSharma14
Copy link
Contributor Author

jenkins test make check

@aaSharma14
Copy link
Contributor Author

jenkins test api

@aaSharma14
Copy link
Contributor Author

jenkins test make check

@epuertat epuertat moved this from Review in progress to Ready-to-merge in Dashboard Jan 10, 2022
Dashboard automation moved this from Ready-to-merge to Reviewer approved Jan 10, 2022
@alfonsomthd alfonsomthd moved this from Reviewer approved to Ready-to-merge in Dashboard Jan 10, 2022
@epuertat epuertat merged commit 978d582 into ceph:master Jan 11, 2022
10 checks passed
Dashboard automation moved this from Ready-to-merge to Done Jan 11, 2022
pereman2 pushed a commit to rhcs-dashboard/ceph that referenced this pull request Jan 17, 2022
…node

mgr/dashboard: monitoring:Implement BlueStore onode hit/miss counters into the dashboard

Reviewed-by: Aashish Sharma <aasharma@redhat.com>
Reviewed-by: Alfonso Martínez <almartin@redhat.com>
Reviewed-by: Avan Thakkar <athakkar@redhat.com>
Reviewed-by: Ernesto Puerta <epuertat@redhat.com>
Reviewed-by: Laura Flores <lflores@redhat.com>
Reviewed-by: neha-ojha <NOT@FOUND>
Reviewed-by: Pere Diaz Bou <pdiazbou@redhat.com>
Signed-off-by: Pere Diaz Bou <pdiazbou@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Dashboard
  
Done
7 participants