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

Pods are preferentially scheduled to machines that meet the current session resources #2815

Merged
merged 1 commit into from
Aug 7, 2023

Conversation

wangyang0616
Copy link
Member

@wangyang0616 wangyang0616 commented Apr 26, 2023

fix: #2782

Except that the binpack plugin has this problem, I understand that other algorithm plugins may encounter similar problems, such as task-topology, nodeorder, etc.

I was wondering if this generic problem could be solved by:
When allocate scores nodes, it divides nodes into two groups. One group is machines whose idle resources meet task resource requests, and the second group is futrue idle machines that meet task resource demands.

First, score the first group of machines, and if a suitable machine can be found, schedule the task to a suitable node; if the first group does not have a machine that meets the resource request, then score the second group of machines, and then select a suitable node for scheduling.

In this way, the pod can be dispatched to the machine that meets the resource requirements in the current session first, so that the pod will not be pending for a long time. If all the machines in the current session do not meet the requirements, it can also be scheduled to wait in the machine that meets the futrue idle.

@volcano-sh-bot volcano-sh-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 26, 2023
@wangyang0616 wangyang0616 changed the title Prioritize scheduling to machines that are satisfied with current res… Pods are preferentially scheduled to machines that meet the current session resources Apr 26, 2023
@wangyang0616 wangyang0616 force-pushed the fix_node_score branch 2 times, most recently from a15a408 to 73b0116 Compare April 27, 2023 09:46
@wangyang0616
Copy link
Member Author

/priority important-soon

@volcano-sh-bot volcano-sh-bot added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label May 5, 2023
@wangyang0616
Copy link
Member Author

Close the current pr, and fix the preemption problem through #2916

/close

@volcano-sh-bot
Copy link
Contributor

@wangyang0616: Closed this PR.

In response to this:

Close the current pr, and fix the preemption problem through #2916

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@wangyang0616
Copy link
Member Author

Close the current pr, and fix the preemption problem through #2916

/close

The current pr was closed by mistake, reopen it.
/reopen

@volcano-sh-bot volcano-sh-bot reopened this Aug 1, 2023
@volcano-sh-bot
Copy link
Contributor

@wangyang0616: Reopened this PR.

In response to this:

Close the current pr, and fix the preemption problem through #2916

/close

The current pr was closed by mistake, reopen it.
/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@wangyang0616 wangyang0616 force-pushed the fix_node_score branch 2 times, most recently from c841e5e to 041dfeb Compare August 2, 2023 11:37
}
}

var node *api.NodeInfo
Copy link
Member

Choose a reason for hiding this comment

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

no need to define a new node variable.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

if bestNode != nil {
node = bestNode
} else {
klog.Errorf("task %s/%s allocate failed, bestNode is nil", task.Namespace, task.Name)
Copy link
Member

Choose a reason for hiding this comment

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

The codes between 249 - 255 are redundent.

Copy link
Member Author

Choose a reason for hiding this comment

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

The code after line 250 uses node information for resource judgment, and it is necessary to ensure that the node pointer is not empty.

}
switch {
case len(nodes) == 0:
klog.V(3).Infof("Task: %v, no matching node is found in the nodes list."+
Copy link
Member

Choose a reason for hiding this comment

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

Here log level 3 is not so good. There will be too much log.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@@ -195,28 +195,64 @@ func (alloc *Action) Execute(ssn *framework.Session) {
break
}

var candidateNodes []*api.NodeInfo
// When scheduling pods, gradient scoring is performed on all nodes that are successfully filtered.
Copy link
Member

Choose a reason for hiding this comment

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

// Candidate nodes are divided into two gradients:
// - the first gradient node: a list of free nodes that satisfy the task resource request;
// - The second gradient node: the node list whose sum of node idle resources and future idle meets the task resource request;
// Score the first gradient node first. If the first gradient node meets the requirements, ignore the second gradient node list, otherwise, score the second echelon node and select the appropriate node.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

…ources, and then consider machines that are satisfied with future resources

Signed-off-by: wangyang <[email protected]>
Copy link
Member

@william-wang william-wang left a comment

Choose a reason for hiding this comment

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

/lgtm

@volcano-sh-bot volcano-sh-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 7, 2023
@volcano-sh-bot
Copy link
Contributor

[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:

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 volcano-sh-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
3 participants