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

implement rama policy framework with pid controller #105

Merged

Conversation

sun-yuliang
Copy link
Collaborator

@sun-yuliang sun-yuliang commented Jun 13, 2023

What type of PR is this?

Features

What this PR does / why we need it:

Implement the framework of a fine tuned policy "rama" for dedicated numa exclusive region

Which issue(s) this PR fixes:

Special notes for your reviewer:

@sun-yuliang sun-yuliang added enhancement New feature or request workflow/draft draft: no need to review labels Jun 13, 2023
@sun-yuliang sun-yuliang self-assigned this Jun 13, 2023
@sun-yuliang sun-yuliang changed the title wip: implement rama policy with pid controller WIP: implement rama policy with pid controller Jun 13, 2023
@sun-yuliang sun-yuliang force-pushed the syl/implement-rama-policy branch from cd6d8dd to 9ecb9a7 Compare June 14, 2023 13:07
@codecov
Copy link

codecov bot commented Jun 14, 2023

Codecov Report

Patch coverage: 52.02% and project coverage change: +0.22 🎉

Comparison is base (21d86d9) 51.30% compared to head (e4d334a) 51.52%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #105      +/-   ##
==========================================
+ Coverage   51.30%   51.52%   +0.22%     
==========================================
  Files         318      377      +59     
  Lines       32418    36288    +3870     
==========================================
+ Hits        16632    18698    +2066     
- Misses      13840    15438    +1598     
- Partials     1946     2152     +206     
Flag Coverage Δ
unittest 51.52% <52.02%> (+0.22%) ⬆️

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

Impacted Files Coverage Δ
cmd/katalyst-agent/app/agent/eviction.go 0.00% <0.00%> (ø)
...gent/app/options/dynamic/adminqos/adminqos_base.go 54.54% <ø> (ø)
pkg/agent/evictionmanager/endpoint/endpoint.go 0.00% <ø> (ø)
pkg/agent/qrm-plugins/advisorsvc/advisor_svc.pb.go 10.34% <ø> (ø)
...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 43.73% <ø> (+5.38%) ⬆️
...ins/memory/dynamicpolicy/state/state_checkpoint.go 48.05% <0.00%> (ø)
...g/agent/qrm-plugins/network/staticpolicy/policy.go 52.97% <ø> (ø)
pkg/agent/qrm-plugins/network/staticpolicy/util.go 62.14% <ø> (ø)
... and 191 more

... and 15 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.

@sun-yuliang sun-yuliang force-pushed the syl/implement-rama-policy branch 5 times, most recently from aea7ddc to d5d2c54 Compare June 16, 2023 07:12
@waynepeking348
Copy link
Collaborator

there still exists some comments, we need to discuss before this pr can be merged

@@ -17,16 +17,14 @@ limitations under the License.
package headroompolicy
Copy link
Collaborator

Choose a reason for hiding this comment

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

if this policy is deprecated, which one should we use? will there be a corresponding rama headroom policy?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There may be a policy based on machine learning to estimate NUMA headroom for dedicated numa exclusive region in the future.

@sun-yuliang sun-yuliang force-pushed the syl/implement-rama-policy branch 2 times, most recently from 6d9df75 to 7d1ace5 Compare June 19, 2023 07:05
@sun-yuliang sun-yuliang changed the title WIP: implement rama policy with pid controller implement rama policy framework with pid controller Jun 19, 2023
@sun-yuliang sun-yuliang requested a review from cheney-lin June 19, 2023 07:06
@sun-yuliang sun-yuliang added workflow/merge-ready merge-ready: code is ready and can be merged workflow/need-review review: test succeeded, need to review and removed workflow/draft draft: no need to review workflow/merge-ready merge-ready: code is ready and can be merged labels Jun 19, 2023
@sun-yuliang sun-yuliang force-pushed the syl/implement-rama-policy branch from 7d1ace5 to 80aa3fd Compare June 19, 2023 07:12
@sun-yuliang sun-yuliang force-pushed the syl/implement-rama-policy branch from 80aa3fd to e4d334a Compare June 19, 2023 07:14
@sun-yuliang sun-yuliang 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 Jun 19, 2023
@waynepeking348 waynepeking348 merged commit 9875066 into kubewharf:main Jun 19, 2023
luomingmeng pushed a commit to luomingmeng/katalyst-core that referenced this pull request Oct 11, 2024
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