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

Add node image information to the cache of the scheduler. #2593

Merged
merged 1 commit into from
Dec 14, 2022

Conversation

wangyang0616
Copy link
Member

Signed-off-by: wangyang wangyang289@huawei.com

fix: #2496
Verify and update based on #2543 due to contributor project adjustment. Thanks @zhifanggao

Add ImageStates information to the cache of Volcano for trial use of the imagelocality policy.

Note:
Kubernetes uses the Docker engine. When the image name is matched, the prefix name matching is incomplete. #112699

@volcano-sh-bot volcano-sh-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 6, 2022
Signed-off-by: wangyang <wangyang289@huawei.com>
@@ -19,6 +19,7 @@ package cache
import (
"context"
"fmt"
"k8s.io/kubernetes/pkg/scheduler/framework"
Copy link
Member

Choose a reason for hiding this comment

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

why we need to import this package in this file?

Copy link
Member Author

Choose a reason for hiding this comment

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

This package is referenced when imageState is used to initialize ImageStateSummary. This package is introduced by the createImageStateSummary function added to the cache file.

size: image.SizeBytes,
nodes: sets.NewString(node.Name),
}
sc.imageStates[name] = state
Copy link
Member

Choose a reason for hiding this comment

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

where we use the data from sc.imageStates[name]? Is it enough that we only store image data in nodeinfo?

Copy link
Member Author

Choose a reason for hiding this comment

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

sc.imageStates stores the image information of all nodes in the cluster. The image name is key, and the information about the node that owns the image is value.

When a node is added or deleted, the system updates the information in sc.imageStates and updates the imageStates information in nodeInfo of the current node through sc.imageStates. The scheduler scores the node based on the imageStates information in nodeInfo of each node.

Copy link
Member

Choose a reason for hiding this comment

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

imageState in nodeinfo is necessary. I want to know the necessary of sc.imagestates.

Copy link
Member Author

@wangyang0616 wangyang0616 Dec 9, 2022

Choose a reason for hiding this comment

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

sc.imageStates maintains the image distribution information of all nodes in the cluster. When a node is added or deleted, sc.imageStates is updated. If sc.imageStates does not exist, all nodeinfo needs to be traversed for recalculation each time a node is added or deleted, which affects the performance and efficiency.

In addition, sc.imageStates is properly stored in the scheduler cache. Other structures are not found to store cluster imageStates information.

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 Dec 14, 2022
@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

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. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

imagelocality.weight can not take affect when using nodeorder plugin
3 participants