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
scheduler: calculate priority in parallel. #18413
Conversation
Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist") If this message is too spammy, please complain to ixdy. |
Labelling this PR as size/M |
Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist") If this message is too spammy, please complain to ixdy. |
Great work. If we could have the performance test, the result would have more credit. |
/cc @HardySimpson |
@xiang90 - can you please rebase this PR? |
Yes, please rebase. Assigning to @wojtek-t who seems to be offering to review it. :) |
@xiang90 - can you please rebase? |
@@ -170,40 +170,72 @@ func PrioritizeNodes(pod *api.Pod, podLister algorithm.PodLister, priorityConfig | |||
return EqualPriority(pod, podLister, nodeLister) | |||
} | |||
|
|||
var ( | |||
mu sync.Mutex |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
personally I perfer:
mu := sync.Mutex{}
@xiang90 - I also added some comments - please apply them when rebasing. |
@wojtek-t Rebased. Fixed the issues you mentioned. PTAL. |
LGTM - thanks! |
GCE e2e test build/test passed for commit 13d29440edac214d7a1b9452e89a323084b8ded2. |
LGTM - thanks! |
@k8s-bot test this Tests are more than 48 hours old. Re-running tests. |
GCE e2e test build/test passed for commit 7f4f754. |
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
GCE e2e test build/test passed for commit 7f4f754. |
@k8s-bot unit test this please |
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
GCE e2e test build/test passed for commit 7f4f754. |
@k8s-bot unit test this please |
4 similar comments
@k8s-bot unit test this please |
@k8s-bot unit test this please |
@k8s-bot unit test this please |
@k8s-bot unit test this please |
@xiang90 - can you please rebase this PR - it's now constantly failing with the following error:
I hope rebase will help... |
@k8s-bot unit test this please |
1 similar comment
@k8s-bot unit test this please |
@wojtek-t Do I still need to rebase? Seems to be fine? |
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
GCE e2e build/test failed for commit 7f4f754. |
GCE e2e test build/test passed for commit 7f4f754. |
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
GCE e2e test build/test passed for commit 7f4f754. |
Automatic merge from submit-queue |
Auto commit by PR queue bot
To improve the throughput of current scheduler, we can do
a simple optimization by calculating priorities in parallel.
This doubles the throughput of density test, which has the default
config with 3 priority funcs (the spreading one does not actually
consume any computation time). It matches the expectation.
/cc @hongchaodeng @davidopp @HaiyangDING @kevin-wangzefeng