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.

Sign up for GitHub

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

Jump to bottom

Allow preemptable tasks schedule without checking proportion #2967

Closed
wants to merge 1 commit into from
Closed

Allow preemptable tasks schedule without checking proportion #2967

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions pkg/scheduler/plugins/predicates/proportional.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ import (
// checkNodeResourceIsProportional checks if a gpu:cpu:memory is Proportional
func checkNodeResourceIsProportional(task *api.TaskInfo, node *api.NodeInfo, proportional map[v1.ResourceName]baseResource) (*api.Status, error) {
status := &api.Status{}

if task.Preemptable {
Copy link
Member

Choose a reason for hiding this comment

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

The task here is the role of the preemptor, and task.Preemptable indicates whether the current task is allowed to be preempted.
Sorry, I don't quite understand the purpose of the judgment here.

Copy link
Author

Choose a reason for hiding this comment

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

Main idea of this PR, that preemptable tasks can skip this proportional check, because they can be preempted, so there are no problem to schedule them. In system which I maintain in my organisation should be available ability to schedule preemptible tasks bypassing proportional plugin.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for your response, I see what you mean.

However, the proportional logic is designed to ensure that resources are available in the allocate scene, and is not bound to preempt. In reality, many users will not configure preempt actions.

I am worried that the modification here will affect the existing proportional function. So I have reservations about the current pr, and I can refer to the ideas of other contributors in the community.

Copy link
Author

Choose a reason for hiding this comment

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

Maybe we can add an optional parameter to proportional plugin configuration, which by default will save current behavior but allow to enable changes made by this pr?

Copy link
Member

Choose a reason for hiding this comment

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

It sounds more reasonable to add a config. And what's more, job's queue should support Reclaimable so that the job in this queue can be reclaimed.

status.Code = api.Success
return status, nil
}

for resourceName := range proportional {
if value, found := task.Resreq.ScalarResources[resourceName]; found && value > 0 {
status.Code = api.Success
Expand Down
Loading