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

Avoid unnecessary allocations in kube-proxy #65902

Merged
merged 1 commit into from Jul 10, 2018

Conversation

Projects
None yet
6 participants
@wojtek-t
Copy link
Member

wojtek-t commented Jul 6, 2018

No description provided.

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Jul 6, 2018

@wojtek-t: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@wojtek-t wojtek-t self-assigned this Jul 6, 2018

@k8s-ci-robot k8s-ci-robot requested review from caseydavenport and dcbw Jul 6, 2018

@wojtek-t wojtek-t force-pushed the wojtek-t:kube_proxy_less_allocations_2 branch 3 times, most recently from 12bc13c to 2ef4b84 Jul 7, 2018

@wojtek-t wojtek-t assigned thockin and dcbw and unassigned wojtek-t Jul 7, 2018

@wojtek-t wojtek-t changed the title [WIP] Avoid unnecessary allocations in kube-proxy Avoid unnecessary allocations in kube-proxy Jul 7, 2018

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Jul 7, 2018

@wojtek-t: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@wojtek-t

This comment has been minimized.

Copy link
Member Author

wojtek-t commented Jul 7, 2018

@thockin @dcbw - this is now ready for review

@@ -137,6 +137,11 @@ func writeLine(buf *bytes.Buffer, words ...string) {
buf.WriteString(strings.Join(words, " ") + "\n")
}

func writeBytes(buf *bytes.Buffer, bytes []byte) {

This comment has been minimized.

@itnilesh

itnilesh Jul 7, 2018

if possible move this to pkg/util so that code is not repeated

This comment has been minimized.

@itnilesh

itnilesh Jul 7, 2018

Name of function is not clear it writes bytes plus adds new line

This comment has been minimized.

@wojtek-t

wojtek-t Jul 8, 2018

Author Member

Renamed to writeBytesLine (for better consistency with writeLine above).

Re moving to pkg/util - I'm not a fan of large pkg/util, because when you need something you will not find it anyway in such case. This function is so extremely simple that because of lack of obvious place to put it, I'm leaving it here.

@@ -1356,6 +1359,11 @@ func writeLine(buf *bytes.Buffer, words ...string) {
}
}

func writeBytes(buf *bytes.Buffer, bytes []byte) {

This comment has been minimized.

@itnilesh

itnilesh Jul 7, 2018

this function is repeated again move to pkg/util DRY

This comment has been minimized.

@wojtek-t

wojtek-t Jul 8, 2018

Author Member

Re moving to pkg/util - I'm not a fan of large pkg/util, because when you need something you will not find it anyway in such case. This function is so extremely simple that because of lack of obvious place to put it, I'm leaving it here.

@wojtek-t wojtek-t force-pushed the wojtek-t:kube_proxy_less_allocations_2 branch from 2ef4b84 to 6e50f39 Jul 8, 2018

@wojtek-t

This comment has been minimized.

Copy link
Member Author

wojtek-t commented Jul 8, 2018

/retest

@thockin

This comment has been minimized.

Copy link
Member

thockin commented Jul 9, 2018

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label Jul 9, 2018

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Jul 9, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: thockin, wojtek-t

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

@k8s-github-robot

This comment has been minimized.

Copy link
Contributor

k8s-github-robot commented Jul 10, 2018

Automatic merge from submit-queue (batch tested with PRs 65902, 65781). If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-github-robot k8s-github-robot merged commit 13f9c26 into kubernetes:master Jul 10, 2018

17 checks passed

Submit Queue Queued to run github e2e tests a second time.
Details
cla/linuxfoundation wojtek-t authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Skipped
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gke Skipped
pull-kubernetes-e2e-kops-aws Job succeeded.
Details
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped
pull-kubernetes-local-e2e-containerized Job succeeded.
Details
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details

k8s-github-robot pushed a commit that referenced this pull request Aug 29, 2018

Kubernetes Submit Queue
Merge pull request #67948 from wojtek-t/use_buffers_in_kube_proxy
Automatic merge from submit-queue (batch tested with PRs 66577, 67948, 68001, 67982). If you want to cherry-pick this change to another branch, please follow the instructions here: https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md.

Reduce amount of allocations in kube-proxy

Follow up from #65902
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.