enhancement(eviction): load eviction support dynamic threshold #129
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zzzzhhb
changed the title
zzzzhhb
force-pushed
the
load_eviction_dynamic_threshold
branch
from
e0656fd to
0c9f496
Compare
zzzzhhb
requested review from
sun-yuliang,
waynepeking348,
csfldf and
luomingmeng
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #129 +/- ##
==========================================
+ Coverage 51.04% 51.32% +0.27%
==========================================
Files 419 421 +2
Lines 40577 40641 +64
==========================================
+ Hits 20713 20859 +146
+ Misses 17604 17504 -100
- Partials 2260 2278 +18
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
zzzzhhb
changed the title
sun-yuliang
reviewed
Jul 26, 2023
pkg/agent/qrm-plugins/cpu/dynamicpolicy/cpueviction/strategy/pressure_load.go
Outdated
Show resolved
Hide resolved
sun-yuliang
previously approved these changes
Jul 26, 2023
zzzzhhb
force-pushed
the
load_eviction_dynamic_threshold
branch
2 times, most recently
from
c7fa022 to
3513f6b
Compare
| @@ -785,3 +800,803 @@ func TestGetTopEvictionPods(t *testing.T) { | |||
| }) | |||
| } | |||
| } | |||
|
|
|||
| func TestCPUPressureLoadEviction_collectMetrics(t *testing.T) { | |||
| pod1UID := "pod1" | |||
There was a problem hiding this comment.
add t.Parallel() for all tests
pkg/agent/qrm-plugins/util/util.go
Outdated
| @@ -313,6 +315,18 @@ func GetContainerAsyncWorkName(podUID, containerName, topic string) string { | |||
| return strings.Join([]string{podUID, containerName, topic}, "/") | |||
| } | |||
|
|
|||
| func GetSystemReservedCores(conf *config.Configuration, machineInfo *machine.KatalystMachineInfo, allCPUs machine.CPUSet) (machine.CPUSet, error) { | |||
There was a problem hiding this comment.
what's the difference with GetCoreNumsReservedForReclaim
There was a problem hiding this comment.
There are two main differences:
GetSystemReservedCoresreturns certain CPU IDs butGetCoreNumsReservedForReclaimonly returns the amount of cores.GetSystemReservedCoresis related to the cores reserved for system_cores qos butGetCoreNumsReservedForReclaimis about the cores reserved for reclaimed_cores qos.
I'll rename it to GetCoresReservedForSystem to make it more consistent.
| @@ -391,6 +391,19 @@ func (ns *NUMANodeState) GetFilteredDefaultCPUSet(excludeEntry, excludeWholeNUMA | |||
| return res | |||
| } | |||
|
|
|||
| // Exist returns true if the stated predicate holds true for some pods of this numa else it returns false. | |||
| func (ns *NUMANodeState) Exist(f func(ai *AllocationInfo) bool) bool { | |||
There was a problem hiding this comment.
it's too wield to naming this function as Exist
There was a problem hiding this comment.
actually it's a common function name in Java or Scala,😂 never mind, I'll rename it.
zzzzhhb
force-pushed
the
load_eviction_dynamic_threshold
branch
2 times, most recently
from
9bb8327 to
77758be
Compare
zzzzhhb
force-pushed
the
load_eviction_dynamic_threshold
branch
from
77758be to
3f11832
Compare
zzzzhhb
force-pushed
the
load_eviction_dynamic_threshold
branch
from
3f11832 to
91f1891
Compare
waynepeking348
approved these changes
Jul 31, 2023
csfldf
approved these changes
Aug 1, 2023
luomingmeng
pushed a commit
to luomingmeng/katalyst-core
that referenced
this pull request
Oct 11, 2024
…harf#129) * enhancement(eviction): evict by share pool status * enhancement(eviction): load eviction plugin use readonly state * chore(eviction): make test parallel * chore(eviction): rename some function and add some metric * enhancement(eviction): extract GetPodPoolMapFunc interface * enhancement(eviction): ignore isolation pool * refine implementation for load pressure --------- Co-authored-by: shaowei.wayne <shaowei.wayne@bytedance.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
Enhancements
What this PR does / why we need it:
this pr support using dynamic threshold in load eviction plugin.
Which issue(s) this PR fixes:
Special notes for your reviewer: