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

Make priority score info clearer #27237

Merged
merged 1 commit into from
Jun 17, 2016
Merged

Make priority score info clearer #27237

merged 1 commit into from
Jun 17, 2016

Conversation

xiangpengzhao
Copy link
Contributor

@xiangpengzhao xiangpengzhao commented Jun 11, 2016 •
edited
Loading

When I trace the scheduler workflow, the log info makes me a bit confused:
Taint Toleration Priority Score info is lacking.
The values of Absolute and Requested resources are in the reverse order.
The values of resources have no type and unit.

This PR tries to make the log info clearer.

before:

I0609 15:18:17.978739   32560 node_affinity.go:92] mongo -> vm: NodeAffinityPriority, Score: (0)
I0609 15:18:17.978756   32560 priorities.go:69] mongo -> vm: Least Requested Priority, Absolute/Requested: (100, 209715200) / (4000, 8372678656) Score: (9, 9)
I0609 15:18:17.978896   32560 priorities.go:262] mongo -> vm: Balanced Resource Allocation, Absolute/Requested: (100, 209715200) / (4000, 8372678656) Score: (9)
I0609 15:18:17.978971   32560 selector_spreading.go:233] mongo -> vm: SelectorSpreadPriority, Score: (10)
I0609 15:18:17.979043   32560 generic_scheduler.go:301] Host vm Score 38

after:

I0611 06:58:23.132306   28814 taint_toleration.go:108] mongo -> vm: Taint Toleration Priority, Score: (10)
I0611 06:58:23.132326   28814 priorities.go:69] mongo -> vm: Least Requested Priority, Absolute/Requested(CPU:millicores, memory:bytes): (4000, 8372678656) / (100, 209715200) Score: (9, 9)
I0611 06:58:23.132367   28814 node_affinity.go:92] mongo -> vm: NodeAffinityPriority, Score: (0)
I0611 06:58:23.132400   28814 priorities.go:262] mongo -> vm: Balanced Resource Allocation, Absolute/Requested(CPU:millicores, memory:bytes): (4000, 8372678656) / (100, 209715200) Score: (9)
I0611 06:58:23.132544   28814 selector_spreading.go:233] mongo -> vm: SelectorSpreadPriority, Score: (10)
I0611 06:58:23.132567   28814 generic_scheduler.go:301] Host vm Score 38

@k8s-bot
Copy link

k8s-bot commented Jun 11, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

2 similar comments
@k8s-bot
Copy link

k8s-bot commented Jun 11, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

@k8s-bot
Copy link

k8s-bot commented Jun 11, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

@k8s-github-robot k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note-label-needed labels Jun 11, 2016
davidopp
davidopp reviewed Jun 11, 2016
View reviewed changes
plugin/pkg/scheduler/algorithm/priorities/priorities.go
@@ -61,10 +61,10 @@ func calculateResourceOccupancy(pod *api.Pod, node api.Node, nodeInfo *scheduler
cpuScore := calculateScore(totalMilliCPU, capacityMilliCPU, node.Name)
memoryScore := calculateScore(totalMemory, capacityMemory, node.Name)
glog.V(10).Infof(
"%v -> %v: Least Requested Priority, Absolute/Requested: (%d, %d) / (%d, %d) Score: (%d, %d)",
"%v -> %v: Least Requested Priority, Absolute/Requested(CPU:millicores, memory:bytes): (%d, %d) / (%d, %d) Score: (%d, %d)",
Copy link
Member

Choose a reason for hiding this comment

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

This would be even clearer as

"%v -> %v: Least Requested Priority, capacity %d millicores %d memory bytes, total request %d millicores %d memory bytes, score %d CPU %d memory", pod.Name, node.Name, capacityMilliCPU, capacityMemory, totalMilliCPU, totalMemory, cpuScore, memoryScore

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks David! I will modify it. One more thing, do we need to log cpuScore and memoryScore separately? Actually, we use the average value of them as the Least Requested Priority score. I thought about this but not sure to modify or not. @davidopp

Copy link
Member

Choose a reason for hiding this comment

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

Logging them separately (like it is now) is probably better for debugging.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. That's reasonable, I will modify as you wrote above. BTW, could you please also review #26520 , #26774? Thanks @davidopp 😄

@xiangpengzhao
Copy link
Contributor Author

PTAL, @davidopp

@davidopp
Copy link
Member

ok to test

@davidopp
Copy link
Member

This looks fine but please squash the commits into a single commit.

@davidopp davidopp added release-note-none Denotes a PR that doesn't merit a release note. ok-to-merge and removed release-note-label-needed labels Jun 12, 2016
@xiangpengzhao
Copy link
Contributor Author

I'm not familar to squash way, so I need your help. What should I do now? Thank you so much! @davidopp

@davidopp
Copy link
Member

https://ariejan.net/2011/07/05/git-squash-your-latests-commits-into-one/

@xiangpengzhao xiangpengzhao force-pushed the fix_priorityscoreinfo branch from 50877e4 to f1d98ba Compare June 12, 2016 06:23
@xiangpengzhao
Copy link
Contributor Author

squashed. skill +1 😄 Thanks! @davidopp

@davidopp
Copy link
Member

LGTM

@davidopp davidopp added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 12, 2016
@xiangpengzhao xiangpengzhao changed the title Make priority score info more clear Make priority score info clearer Jun 12, 2016
@k8s-bot
Copy link

k8s-bot commented Jun 14, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

1 similar comment
@k8s-bot
Copy link

k8s-bot commented Jun 15, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

@davidopp
Copy link
Member

ok to test

@davidopp davidopp added this to the v1.3 milestone Jun 16, 2016
@k8s-bot
Copy link

k8s-bot commented Jun 16, 2016

GCE e2e build/test passed for commit f1d98ba.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit f830a2c into kubernetes:master Jun 17, 2016
@xiangpengzhao xiangpengzhao deleted the fix_priorityscoreinfo branch June 17, 2016 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@davidopp davidopp

Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
v1.3
Development

Successfully merging this pull request may close these issues.
6 participants
@xiangpengzhao @k8s-bot @davidopp @k8s-github-robot @googlebot @wojtek-t