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

refine coding styles for cpu&memory plugin #54

Merged

Conversation

waynepeking348
Copy link
Collaborator

What type of PR is this?

refactor

What this PR does / why we need it:

currently, qrm plugin is a little complicated and lack of abstraction for similar functionalities

this pr is for cpu and memory plugin

@waynepeking348 waynepeking348 added style workflow/need-review review: test succeeded, need to review labels May 6, 2023
@waynepeking348 waynepeking348 added this to the v0.2 milestone May 6, 2023
@codecov
Copy link

codecov bot commented May 6, 2023

Codecov Report

Patch coverage: 52.37% and project coverage change: +1.22 🎉

Comparison is base (21d86d9) 51.30% compared to head (b24b294) 52.52%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #54      +/-   ##
==========================================
+ Coverage   51.30%   52.52%   +1.22%     
==========================================
  Files         318      348      +30     
  Lines       32418    34266    +1848     
==========================================
+ Hits        16632    17999    +1367     
- Misses      13840    14137     +297     
- Partials     1946     2130     +184     
Flag Coverage Δ
unittest 52.52% <52.37%> (+1.22%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/agent/evictionmanager/endpoint/endpoint.go 0.00% <ø> (ø)
...ins/cpu/dynamicpolicy/calculator/cpu_assignment.go 77.01% <ø> (ø)
...gins/cpu/dynamicpolicy/cpueviction/cpu_eviciton.go 0.00% <0.00%> (-63.25%) ⬇️
pkg/agent/qrm-plugins/cpu/dynamicpolicy/policy.go 40.28% <ø> (+1.94%) ⬆️
...ins/memory/dynamicpolicy/state/state_checkpoint.go 48.05% <0.00%> (ø)
...ourcemanager/fetcher/kubelet/topology/interface.go 0.00% <0.00%> (ø)
.../agent/resourcemanager/reporter/cnr/cnrreporter.go 61.20% <ø> (-3.06%) ⬇️
pkg/agent/resourcemanager/reporter/converter.go 33.33% <ø> (ø)
pkg/agent/resourcemanager/reporter/manager.go 51.37% <ø> (-6.38%) ⬇️
pkg/agent/resourcemanager/reporter/reporter.go 55.55% <ø> (ø)
... and 112 more

... and 17 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@csfldf
Copy link
Collaborator

csfldf commented May 21, 2023

To be honest, I think the risk and burden of making such a large change to these codes are far more than the benefits, especially when it's already running online. But it's been done, I've tried my best to check if modifications will cause problems , but it's really hard to identify the difference between all the changes and the original codes.

@waynepeking348 waynepeking348 self-assigned this May 21, 2023
@waynepeking348 waynepeking348 force-pushed the refine_qrm_cpu_memory_plugin branch from 5d9ec8a to 227299e Compare May 23, 2023 12:47
@waynepeking348 waynepeking348 force-pushed the refine_qrm_cpu_memory_plugin branch from fafa358 to 9aa77a4 Compare May 31, 2023 04:12
@waynepeking348 waynepeking348 force-pushed the refine_qrm_cpu_memory_plugin branch 2 times, most recently from fe77380 to e65f7c9 Compare May 31, 2023 04:21
@csfldf csfldf force-pushed the refine_qrm_cpu_memory_plugin branch from e65f7c9 to b24b294 Compare May 31, 2023 06:53
@csfldf csfldf 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 May 31, 2023
@waynepeking348 waynepeking348 merged commit 03c5861 into kubewharf:main May 31, 2023
luomingmeng pushed a commit to luomingmeng/katalyst-core that referenced this pull request Oct 11, 2024
* refine coding styles for cpu&memory plugin

* fix styles and fix bugs for cpu/memory plugin according to comments

* refine(qrm): add detail comments for complex codes of qrm plugins

* refine(qrm): fix conflicts in rebase numa_exclusive feature

* fix(qrm): ensures dedicated_cores owner pool non-empty

* fix comment typo and refine numa-execlusice judgement

* remove useless functions

---------

Co-authored-by: 孙健俞 <sunjianyu@bytedance.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
style 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