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

Optimizing the processing flow of HandlePodAdditions and canAdmitPod … #28570

Merged
merged 1 commit into from
Jul 11, 2016
Merged

Optimizing the processing flow of HandlePodAdditions and canAdmitPod … #28570

merged 1 commit into from
Jul 11, 2016

Conversation

kevinjkj
Copy link

@kevinjkj kevinjkj commented Jul 7, 2016

Optimizing the processing flow of HandlePodAdditions and canAdmitPod methods. If the following loop body in canAdmitPod method is removed, the detection speed can be improved, and the change is very small.

otherPods := []*api.Pod{}
for _, p := range pods {
if p != pod {
otherPods = append(otherPods, p)
}

}

@k8s-bot
Copy link

k8s-bot commented Jul 7, 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 Jul 7, 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 Jul 7, 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 Jul 7, 2016
@k8s-bot
Copy link

k8s-bot commented Jul 7, 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 Jul 7, 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.

jessfraz
jessfraz reviewed Jul 7, 2016
View reviewed changes
pkg/kubelet/kubelet.go
@@ -2400,8 +2400,8 @@ func (kl *Kubelet) rejectPod(pod *api.Pod, reason, message string) {
}

// canAdmitPod determines if a pod can be admitted, and gives a reason if it
// cannot. "pod" is new pod, while "pods" include all admitted pods plus the
// new pod. The function returns a boolean value indicating whether the pod
// cannot. "pod" is new pod, while "pods" is all admitted pods
Copy link
Contributor

Choose a reason for hiding this comment

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

"are"?

@vishh vishh assigned Random-Liu and unassigned vishh Jul 7, 2016
@Random-Liu Random-Liu added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Jul 7, 2016
@kevinjkj kevinjkj force-pushed the kevinjkj-patch-3 branch from 14a36a6 to 2bf4e72 Compare July 8, 2016 02:13
@Random-Liu Random-Liu added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 11, 2016
@Random-Liu
Copy link
Member

LGTM, thanks for the cleanup!

@k8s-bot
Copy link

k8s-bot commented Jul 11, 2016

GCE e2e build/test passed for commit 2bf4e723fcea6aede83d75cb7d3b6fd156aec114.

@Random-Liu
Copy link
Member 

 ./hack/../hack/verify-gofmt.sh 9s

@kevinjkj Please run gofmt -s -w pkg/kubelet/kubelet.go and apply the cleanup suggested by gofmt.

Optimizing the processing flow of HandlePodAdditions and canAdmitPod …
09344c1 
…methods.

Signed-off-by: Kevin Wang <wang.kanghua@zte.com.cn>

change the note for the canAdmitPod method.

Signed-off-by: Kevin Wang <wang.kanghua@zte.com.cn>

gofmt kubelet.go

Signed-off-by: Kevin Wang <wang.kanghua@zte.com.cn>
@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 11, 2016
@k8s-bot
Copy link

k8s-bot commented Jul 11, 2016

GCE e2e build/test failed for commit 09344c1.

Please reference the list of currently known flakes when examining this failure. If you request a re-test, you must reference the issue describing the flake.

@249043822
Copy link
Member

@k8s-bot issue #27503

@249043822
Copy link
Member

@k8s-bot test this issue: #27503

@Random-Liu
Copy link
Member

@k8s-bot test this issue #28722.

@k8s-bot
Copy link

k8s-bot commented Jul 11, 2016

GCE e2e build/test passed for commit 09344c1.

@Random-Liu Random-Liu added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 11, 2016
@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Jul 11, 2016

GCE e2e build/test passed for commit 09344c1.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit d6e84cc into kubernetes:master Jul 11, 2016
@yujuhong yujuhong mentioned this pull request Nov 29, 2016
Closed
@dchen1107 dchen1107 mentioned this pull request Nov 29, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yujuhong yujuhong yujuhong left review comments

@Random-Liu Random-Liu Random-Liu left review comments

Assignees

@Random-Liu Random-Liu

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
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@kevinjkj @k8s-bot @Random-Liu @249043822 @k8s-github-robot @jessfraz @yujuhong @googlebot @vishh