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: Pool overall performance shows multiple entries of same pool in pool overview #45335

Merged
merged 1 commit into from Mar 30, 2022

Conversation

aaSharma14
Copy link
Contributor

This PR intends to fix this issue

Fixes:https://tracker.ceph.com/issues/54513
Signed-off-by: Aashish Sharma aasharma@redhat.com

Before:
pool_overall_performance

After:
Screenshot from 2022-03-10 17-27-19

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
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
  • jenkins test windows

Copy link
Member

@epuertat epuertat left a comment

Choose a reason for hiding this comment

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

Thanks @aaSharma14!

I checked the dashboards before the grafonnet refactoring and found other Grafana dashboards that might require instant to be true:

# grep instant *
hosts-overview.json:          "instant": true,
hosts-overview.json:          "instant": true,
hosts-overview.json:          "instant": true,
hosts-overview.json:          "instant": true,
hosts-overview.json:          "instant": true,
hosts-overview.json:          "instant": true,
osds-overview.json:          "instant": true,
osds-overview.json:          "instant": true,
osds-overview.json:          "instant": true,
pool-overview.json:          "instant": true,
pool-overview.json:          "instant": true,
pool-overview.json:          "instant": true,
rbd-overview.json:          "instant": true,
rbd-overview.json:          "instant": true,
rbd-overview.json:          "instant": true

monitoring/ceph-mixin/dashboards/utils.libsonnet Outdated Show resolved Hide resolved
monitoring/ceph-mixin/dashboards/cephfs.libsonnet Outdated Show resolved Hide resolved
Dashboard automation moved this from In progress to Review in progress Mar 10, 2022
@aaSharma14 aaSharma14 force-pushed the fix-54513-master branch 4 times, most recently from 2be8f2e to 22c7069 Compare March 16, 2022 06:57
Copy link
Contributor

@avanthakkar avanthakkar left a comment

Choose a reason for hiding this comment

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

LGTM!

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.

Tested locally and looking fine

@aaSharma14
Copy link
Contributor Author

jenkins test dashboard

@aaSharma14
Copy link
Contributor Author

jenkins test make check arm64

1 similar comment
@aaSharma14
Copy link
Contributor Author

jenkins test make check arm64

@aaSharma14
Copy link
Contributor Author

jenkins test dashboard

3 similar comments
@pereman2
Copy link
Contributor

jenkins test dashboard

@aaSharma14
Copy link
Contributor Author

jenkins test dashboard

@epuertat
Copy link
Member

jenkins test dashboard

@epuertat epuertat moved this from Review in progress to Ready-to-merge in Dashboard Mar 21, 2022
@aaSharma14
Copy link
Contributor Author

jenkins test dashboard

Copy link
Member

@epuertat epuertat left a comment

Choose a reason for hiding this comment

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

@aaSharma14 I see some "time_series" missing over there... Are you sure those are harmless?

monitoring/ceph-mixin/dashboards_out/cephfs-overview.json Outdated Show resolved Hide resolved
monitoring/ceph-mixin/dashboards_out/hosts-overview.json Outdated Show resolved Hide resolved
monitoring/ceph-mixin/dashboards_out/pool-overview.json Outdated Show resolved Hide resolved
monitoring/ceph-mixin/dashboards/rgw.libsonnet Outdated Show resolved Hide resolved
@aaSharma14
Copy link
Contributor Author

jenkins test dashboard

@epuertat epuertat removed the DNM label Mar 25, 2022
Dashboard automation moved this from Ready-to-merge to Review in progress Mar 28, 2022
…e pool in pool overview

This PR intends to fix this issue

Fixes:https://tracker.ceph.com/issues/54513
Signed-off-by: Aashish Sharma <aasharma@redhat.com>
@aaSharma14
Copy link
Contributor Author

jenkins test dashboard

Dashboard automation moved this from Review in progress to Reviewer approved Mar 30, 2022
@epuertat epuertat merged commit 043f795 into ceph:master Mar 30, 2022
12 of 13 checks passed
Dashboard automation moved this from Reviewer approved to Done Mar 30, 2022
@epuertat epuertat deleted the fix-54513-master branch March 30, 2022 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Dashboard
  
Done
5 participants