Support preempting BestEffort pods when the pods number of nodes reaches the upper limit #3335

Lily922 · 2024-03-04T01:36:24Z

Design：
Ignore pending BestEffort pods when checking if the Job is starving.

1. "allocate" action:
   if resources.IsEmpty() :    // task with empty requsted resource:
       skip to schedule it
   else:
       try to schedule it
   if job.IsReady():               // job.ReadyTaskNum()+job.PendingBestEffortTaskNum() >= ji.MinAvailable
       job.Commit()
      

2. "preempt" action:
   if job.Starving():            // job.WaitingTaskNum()+job.ReadyTaskNum() < job.MinAvailable
      try to preempt:
           if !task.IsEmpty() && preemptor.IsEmpty():
               ignore task
           else:
               add task to victims queue
   if job.IsPipeline():         // job.WaitingTaskNum()+job.ReadyTaskNum()+job.PendingBestEffortTaskNum() >= job.MinAvailable
       job.Commit()

4. "backfill" action:
   sortPendingOrPipelinedBestEffortPods()
   if resources.IsEmpty() :    // task with empty requsted resource:
       try to schedule it
   else:
       skip to schedule it

Pods which request none resource except "pods" number resource(BestEffort pods) will be scheduled in "backfill" action
Pods request any resource including scalar resources(unBestEffort pods) will be scheduled in "allocate" action
All pending pods can become a preemptor in "preempt" action, even if it is a BestEffort pod. If the preemptor is BestEffort pod, unBestEffort pod can never be the victim
Don't count pending BestEffort pods when checking if the job is starving, otherwise the pending BestEffort pods can never trigger preemption
Keep counting pending BestEffort pods when checking if the job is pipeline/ready, because the bug fix :Considering best-effort pods when calculating ready task number #647
Pending pods will be sorted by priority in "backfill" action, otherwise a scheduled low priority pod will be evicted by a high-priority pod immediately, it is meaningless for low-priority pods to be scheduled first.
Pipelined pods will be allocated in "backfill" action, in this case, "backfill" action can be configured in front of "preempt" action in configmap "volcano-scheduler-configmap".

lowang-bh · 2024-03-04T06:31:21Z

Hi, thanks for the contributions. Please add some UTs about your scenario。

lowang-bh · 2024-03-04T06:09:47Z

pkg/scheduler/api/job_info.go

+	for status, tasks := range ji.TaskStatusIndex {
+		if status == Pending {
+			for _, task := range tasks {
+				if task.InitResreq.IsEmpty() {


Already has a BestEffort flag, don't need to calculate it any more.

lowang-bh · 2024-03-04T06:10:59Z

pkg/scheduler/api/job_info.go

@@ -707,6 +699,20 @@ func (ji *JobInfo) WaitingTaskNum() int32 {
 	return int32(len(ji.TaskStatusIndex[Pipelined]))
 }

+func (ji *JobInfo) PendingBestEffortTaskNum() int32 {
+	count := 0
+	for status, tasks := range ji.TaskStatusIndex {


From my opinion, we can directly check ji.TaskStatusIndex[Pending] and sum up the count.

lowang-bh · 2024-03-04T06:24:18Z

pkg/scheduler/actions/backfill/backfill.go

-					job.NodesFitErrors[task.UID] = fe
-					break
-				}
+			if !task.InitResreq.IsEmpty() {


Use task.BestEffort property directly.

Lily922 · 2024-03-05T09:46:55Z

Hi, thanks for the contributions. Please add some UTs about your scenario。

done

lowang-bh · 2024-03-06T07:53:17Z

pkg/scheduler/actions/backfill/backfill_test.go

+		for _, job := range tc.jobs {
+			jobInfo := api.NewJobInfo(job.uid, job.tasks...)
+			jobInfo.Queue = "queue-1"
+			jobInfo.PodGroup = &api.PodGroup{


We can use BuildXX function in pkg/scheduler/util/test_utils.go to gen those basic node/pg/queue to clean code.
And use NewDefaultMockSchedulerCache in pkg/scheduler/cache/cache_mock.go to gen the SchedulerCache.

Monokaix · 2024-03-07T03:11:36Z

pkg/scheduler/actions/preempt/preempt.go

@@ -117,8 +117,8 @@ func (pmpt *Action) Execute(ssn *framework.Session) {
 					if !api.PreemptableStatus(task.Status) {
 						return false
 					}
-					// Ignore task with empty resource request.
-					if task.Resreq.IsEmpty() {
+					// BestEffort pod is not supported to preempt unBestEffort pod.


If a besteffort pod has a higher priority, preemption will not happen, is this expected?

If a besteffort pod has a higher priority, preemption will not happen, is this expected?

yes, besteffort pod will be scheduled in "backfill" action, unbesteffort pods will be scheduled in "allocate" action. In normal use, allocate is executed before backfill, the preempted pod will be scheduled first in the next scheduler cycle

Monokaix · 2024-03-07T03:12:06Z

pkg/scheduler/api/job_info_test.go

@@ -17,14 +17,16 @@ limitations under the License.
 package api

 import (
+	"k8s.io/apimachinery/pkg/api/resource"


Please sort imports.

Please sort imports.
done

…hes the upper limit Signed-off-by: lili <lili_9309@163.com>

wangyang0616 · 2024-03-11T06:39:15Z

/lgtm

william-wang

/approve

volcano-sh-bot · 2024-03-11T11:46:33Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: william-wang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~pkg/scheduler/OWNERS~~ [william-wang]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

volcano-sh-bot requested review from k82cn and Thor-wl March 4, 2024 01:36

volcano-sh-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 4, 2024

Lily922 force-pushed the preempt branch from 0e13771 to 22b3d9b Compare March 4, 2024 01:46

Lily922 changed the title ~~Support preempting BestEffort pods~~ upport preempting BestEffort pods when the pods number of nodes reaches the upper limit Mar 4, 2024

Lily922 changed the title ~~upport preempting BestEffort pods when the pods number of nodes reaches the upper limit~~ Support preempting BestEffort pods when the pods number of nodes reaches the upper limit Mar 4, 2024

lowang-bh reviewed Mar 4, 2024

View reviewed changes

Lily922 force-pushed the preempt branch 4 times, most recently from d08ed78 to a66a7fa Compare March 5, 2024 02:31

volcano-sh-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 5, 2024

Lily922 force-pushed the preempt branch from a66a7fa to 72ee9c8 Compare March 5, 2024 08:22

Lily922 mentioned this pull request Mar 5, 2024

[cherry-pick for release-1.9]Support preempting BestEffort pods when the pods number of nodes reaches the upper limit #3338

Merged

Lily922 force-pushed the preempt branch from 72ee9c8 to d13c1a8 Compare March 5, 2024 08:53

lowang-bh reviewed Mar 6, 2024

View reviewed changes

Monokaix reviewed Mar 7, 2024

View reviewed changes

Lily922 force-pushed the preempt branch from b279550 to c1468c9 Compare March 7, 2024 07:32

Support preempting BestEffort pods when the pods number of nodes reac…

59b54cc

…hes the upper limit Signed-off-by: lili <lili_9309@163.com>

Lily922 force-pushed the preempt branch from c1468c9 to 59b54cc Compare March 8, 2024 02:26

volcano-sh-bot assigned wangyang0616 Mar 11, 2024

volcano-sh-bot added the lgtm Indicates that a PR is ready to be merged. label Mar 11, 2024

william-wang approved these changes Mar 11, 2024

View reviewed changes

volcano-sh-bot assigned william-wang Mar 11, 2024

volcano-sh-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 11, 2024

volcano-sh-bot merged commit 0559427 into volcano-sh:master Mar 11, 2024
14 checks passed

Lily922 deleted the preempt branch April 7, 2024 07:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support preempting BestEffort pods when the pods number of nodes reaches the upper limit #3335

Support preempting BestEffort pods when the pods number of nodes reaches the upper limit #3335

Lily922 commented Mar 4, 2024 •

edited

lowang-bh commented Mar 4, 2024

lowang-bh Mar 4, 2024

Lily922 Mar 5, 2024

lowang-bh Mar 4, 2024

Lily922 Mar 5, 2024

lowang-bh Mar 4, 2024

Lily922 Mar 5, 2024

Lily922 commented Mar 5, 2024

lowang-bh Mar 6, 2024

Lily922 Mar 7, 2024

Monokaix Mar 7, 2024

Lily922 Mar 8, 2024

Monokaix Mar 7, 2024

Lily922 Mar 8, 2024

wangyang0616 commented Mar 11, 2024

william-wang left a comment

volcano-sh-bot commented Mar 11, 2024

Support preempting BestEffort pods when the pods number of nodes reaches the upper limit #3335

Support preempting BestEffort pods when the pods number of nodes reaches the upper limit #3335

Conversation

Lily922 commented Mar 4, 2024 • edited

lowang-bh commented Mar 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lily922 commented Mar 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wangyang0616 commented Mar 11, 2024

william-wang left a comment

Choose a reason for hiding this comment

volcano-sh-bot commented Mar 11, 2024

Lily922 commented Mar 4, 2024 •

edited