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

feat(kcnr): add node metric #63

Merged
merged 1 commit into from
Feb 21, 2024

Conversation

cheney-lin
Copy link
Member

What type of PR is this?

Features

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

@cheney-lin cheney-lin added enhancement New feature or request workflow/need-review review: test succeeded, need to review labels Feb 19, 2024
@cheney-lin cheney-lin force-pushed the dev/node_metric branch 4 times, most recently from aa1f9ea to 49cccfb Compare February 20, 2024 03:41
luomingmeng
luomingmeng previously approved these changes Feb 20, 2024
@cheney-lin cheney-lin added workflow/merge-ready merge-ready: code is ready and can be merged and removed workflow/need-review review: test succeeded, need to review labels Feb 20, 2024
pkg/apis/node/v1alpha1/types.go Show resolved Hide resolved
pkg/apis/node/v1alpha1/types.go Show resolved Hide resolved
ResourceUsage `json:",inline"`
}

type NUMAMetricInfo struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why we need per-numa level data

Copy link
Member Author

Choose a reason for hiding this comment

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

for numa aware scheduling

// NUMAUsage contains the real-time resource usage for each NUMA
NUMAUsage []NUMAMetricInfo `json:"numaUsage,omitempty"`

// GenericUsage contains the real-time resource usage, in format of '{"cpu": 2, "memory": "1Gi"}'
Copy link
Collaborator

Choose a reason for hiding this comment

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

since we already have NUMAUsage why we still need GenericUsage? should we restrict that only one of these two fields is valid?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not all metrics can be granularized to the NUMA level,such as network bandwidth.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok, so we should restrict that only one of these two fields is valid (for a certain resource)? if so, pls add some comments here

Signed-off-by: linzhecheng <[email protected]>
@waynepeking348 waynepeking348 merged commit dcfe569 into kubewharf:main Feb 21, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request workflow/merge-ready merge-ready: code is ready and can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants