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(eviction): add cpu system pressure eviction plugin #518

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

WangZzzhe
Copy link
Collaborator

What type of PR is this?

Enhancements

What this PR does / why we need it:

  • add cpu system pressure eviction plugin, which collects cpu usage and load and verifies if these metrics are overused. It will be enabled in the scenarios where the node does not enable co-location.
  • add k8s native QoS sorter and pod owner sorter, which can be enabled through configuration in memory system eviction plugin.
  • MetricRing has been moved to util.metric

Which issue(s) this PR fixes:

#492
PR in katalyst-api: kubewharf/katalyst-api#74 go.mod should be updated after api PR merged

@WangZzzhe WangZzzhe added enhancement New feature or request workflow/need-review review: test succeeded, need to review labels Mar 21, 2024
@WangZzzhe WangZzzhe requested a review from caohe March 21, 2024 09:10
@WangZzzhe WangZzzhe self-assigned this Mar 21, 2024
@WangZzzhe WangZzzhe force-pushed the dev/cpu-system-eviction-plugin branch from 567b8d9 to 536deb9 Compare March 21, 2024 09:13
Copy link

codecov bot commented Mar 21, 2024

Codecov Report

Attention: Patch coverage is 71.05263% with 132 lines in your changes are missing coverage. Please review.

Project coverage is 56.26%. Comparing base (74b648c) to head (4db7acc).
Report is 11 commits behind head on main.

Files Patch % Lines
...gent/evictionmanager/plugin/cpu/system_pressure.go 78.82% 43 Missing and 22 partials ⚠️
...s/dynamic/adminqos/eviction/cpu_system_eviction.go 38.35% 45 Missing ⚠️
pkg/util/native/pod_sorter.go 57.69% 8 Missing and 3 partials ⚠️
pkg/agent/evictionmanager/plugin/memory/helper.go 0.00% 4 Missing ⚠️
pkg/util/metric/ring.go 88.23% 2 Missing and 2 partials ⚠️
...ynamicpolicy/cpueviction/strategy/pressure_load.go 75.00% 2 Missing ⚠️
...options/dynamic/adminqos/eviction/eviction_base.go 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #518      +/-   ##
==========================================
+ Coverage   56.06%   56.26%   +0.19%     
==========================================
  Files         534      538       +4     
  Lines       49807    50283     +476     
==========================================
+ Hits        27924    28290     +366     
- Misses      18325    18412      +87     
- Partials     3558     3581      +23     
Flag Coverage Δ
unittest 56.26% <71.05%> (+0.19%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@WangZzzhe WangZzzhe force-pushed the dev/cpu-system-eviction-plugin branch 4 times, most recently from bba6910 to 35b1cb3 Compare April 15, 2024 03:55
@waynepeking348
Copy link
Collaborator

  1. what's the difference between this eviction plugin and cpu-load && cpu-suppression eviction plugins?
  2. why don't we implement this eviction in qrm but in eviction-manager? will it be conflict with qrm hybrid logic?

@WangZzzhe WangZzzhe force-pushed the dev/cpu-system-eviction-plugin branch 3 times, most recently from b0748d8 to 6d07059 Compare May 14, 2024 07:57
@WangZzzhe
Copy link
Collaborator Author

  1. what's the difference between this eviction plugin and cpu-load && cpu-suppression eviction plugins?
  2. why don't we implement this eviction in qrm but in eviction-manager? will it be conflict with qrm hybrid logic?
  1. This eviction plugin is supposed to be used in non-colocation scenarios(eg. overcommit), detecting the load of node rather than resource pool and selecting evicted pods by native QoS.
  2. Qrm plugins will not be run when colocation is not applied in user's clusters, only eviction plugins will run based on metaServer.
    If colocation is applied, cpu-load and cpu-suppression will be used and this plugin should be disabled. @waynepeking348

@WangZzzhe WangZzzhe force-pushed the dev/cpu-system-eviction-plugin branch from 6d07059 to f2ba329 Compare May 15, 2024 03:05
@WangZzzhe WangZzzhe force-pushed the dev/cpu-system-eviction-plugin branch from b7c447b to f88c898 Compare May 27, 2024 12:08
@WangZzzhe WangZzzhe force-pushed the dev/cpu-system-eviction-plugin branch from b0c1e93 to 4db7acc Compare May 27, 2024 12:35
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/need-review review: test succeeded, need to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants