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

HDDS-6154. Update NodeManagerInsight with the correct node metrics #5394

Merged
merged 3 commits into from Oct 25, 2023

Conversation

VarshaRaviCV
Copy link
Contributor

@VarshaRaviCV VarshaRaviCV commented Oct 5, 2023

What changes were proposed in this pull request?

Updating the NodeManagerInsight to reflect the correct node metrics

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-6154

How was this patch tested?

Docker-compose environment with command ozone insight metrics scm.node-manager
sh-4.2$ ozone insight metrics scm.node-manager Metrics for scm.node-manager` (SCM Datanode management related information.)

Node counters

in_service healthy Nodes: 3
in_service stale Nodes: 0
in_service dead Nodes: 0
decommissioning healthy Nodes: 0
decommissioning stale Nodes: 0
decommissioning dead Nodes: 0
decommissioned healthy Nodes: 0
decommissioned stale Nodes: 0
decommissioned dead Nodes: 0
entering_maintenance healthy Nodes: 0
entering_maintenance stale Nodes: 0
entering_maintenance dead Nodes: 0
in_maintenance healthy Nodes: 0
in_maintenance stale Nodes: 0
in_maintenance dead Nodes: 0

HB processing stats

HB processed: 26
HB processing failed: 0`

@aswinshakil aswinshakil self-requested a review October 9, 2023 16:20
Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Thanks @VarshaRaviCV for working on this.

I would like to suggest creating these MetricDisplay instances in a nested loop (over operational state and health state) instead of manually enumerating all of them. This will reduce future work required for adapting to related changes. It will also take care of ordering these metrics logically (e.g. "stale" should come before "dead", "entering maint" should come before "in maint").

Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Thanks @VarshaRaviCV for updating the patch.

@adoroszlai
Copy link
Contributor

Thanks @VarshaRaviCV for updating the patch. @sodonnel please review.

Copy link
Contributor

@sodonnel sodonnel left a comment

Choose a reason for hiding this comment

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

LGTM

@sodonnel sodonnel merged commit 4fe7824 into apache:master Oct 25, 2023
32 checks passed
ibrusentsev pushed a commit to ibrusentsev/ozone that referenced this pull request Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants