Skip to content
This repository has been archived by the owner on May 25, 2023. It is now read-only.

Replace NodeOrder with BestNode #699

Closed
k82cn opened this issue Apr 2, 2019 · 8 comments · Fixed by #721
Closed

Replace NodeOrder with BestNode #699

k82cn opened this issue Apr 2, 2019 · 8 comments · Fixed by #721
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Milestone

Comments

@k82cn
Copy link
Contributor

k82cn commented Apr 2, 2019

Is this a BUG REPORT or FEATURE REQUEST?:

/kind feature

Description:

Currently, we're using similar way with upstream to sort nodes for a task to get the best node; but the sort action is not necessary as we just want to get the best node after predicates. It's better to get best node directly instead of sort, the time complexity will be reduced from O(nlogn) to O(n).

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 2, 2019
@hex108
Copy link
Contributor

hex108 commented Apr 2, 2019

If the best node doest not fits, we need pick next one, so will it be better to return a PriorityQueue?

@k82cn
Copy link
Contributor Author

k82cn commented Apr 2, 2019

why best node does not fit after predicates?

@hex108
Copy link
Contributor

hex108 commented Apr 3, 2019

@k82cn If I understand correctly, the predicates does not contain resource predicates, so there is some resource compare logic in https://github.com/kubernetes-sigs/kube-batch/blob/master/pkg/scheduler/actions/allocate/allocate.go#L168. Need we enhance the prediates?

Another situation: administrator perhaps does not enable predicate plugin.

@k82cn
Copy link
Contributor Author

k82cn commented Apr 3, 2019

Need we enhance the prediates?

yes, refer to #694 .

Another situation: administrator perhaps does not enable predicate plugin.

Yes, but scheduler can not do anything to enhance that even we have a ordered list :)

@hex108
Copy link
Contributor

hex108 commented Apr 3, 2019

Need we enhance the prediates?

yes, refer to #694 .

Thanks for it!

Another situation: administrator perhaps does not enable predicate plugin.

Yes, but scheduler can not do anything to enhance that even we have a ordered list :)

There is resource predicate in https://github.com/kubernetes-sigs/kube-batch/blob/master/pkg/scheduler/actions/allocate/allocate.go#L168. If we fix #694, we could delete those code. However after we add resource predicate in predicates, if there are no nodes satisfying the task, we will try to allocate for next job and we could not pipeline task(it works like reserving resource), big tasks might starve.

@k82cn
Copy link
Contributor Author

k82cn commented Apr 3, 2019

If we fix #694, we could delete those code.

In resource predicates, we need to check both idel & releasing to avoid duplicated preemption; the logic is necessary here as the action maybe different when using resources.

@k82cn k82cn added this to the v0.5 milestone Apr 3, 2019
@hex108
Copy link
Contributor

hex108 commented Apr 4, 2019

/assign

@k82cn
Copy link
Contributor Author

k82cn commented Apr 7, 2019

btw, we only need this in allocate action.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants