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

Disable gzip compression in core control plane components #80919

Merged
merged 2 commits into from Aug 5, 2019

Conversation

@smarterclayton
Copy link
Contributor

commented Aug 2, 2019

On local networks (such as the typical connection path between control
plane components) gzip compression increases CPU use and end to end p99
latency rather than decreasing it. Disable compression within the control
plane components like a 1.15 cluster would be configured.

Golang automatically enables transport level gzip, but local network
clients may wish to disable it for better CPU usage and lower latency
(scheduler, controller-manager). Allow DisableCompression on rest.Config to
modify the underlying transport. This impacts the transport cache, but it
is expected that most clients connecting to the same servers within a
process will have the same compression config.

/kind bug

Kubernetes client users may disable automatic compression when invoking Kubernetes APIs by setting the `DisableCompression` field on their rest.Config.  This is recommended when clients communicate primarily over high bandwidth / low latency networks where response compression does not improve end to end latency.
@smarterclayton

This comment has been minimized.

Copy link
Contributor Author

commented Aug 2, 2019

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Aug 2, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: smarterclayton

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

@wojtek-t

This comment has been minimized.

Copy link
Member

commented Aug 2, 2019

@smarterclayton - unit test failures seem legitimate; once fixed lgtm

smarterclayton added some commits Aug 2, 2019

Allow gzip compression to be disabled from rest.Config
Golang automatically enables transport level gzip, but local network
clients may wish to disable it for better CPU usage and lower latency
(scheduler, controller-manager). Allow DisableCompression on rest.Config
to modify the underlying transport. This impacts the transport cache,
but it is expected that most clients connecting to the same servers
within a process will have the same compression config.
Disable gzip compression in core control plane components
On local networks (such as the typical connection path between
control plane components) gzip compression increases CPU use and
end to end p99 latency rather than decreasing it. Disable compression
within the control plane components like a 1.15 cluster would be
configured.

@smarterclayton smarterclayton force-pushed the smarterclayton:disable_compression branch from 5184cc7 to 33521b4 Aug 2, 2019

@smarterclayton

This comment has been minimized.

Copy link
Contributor Author

commented Aug 2, 2019

/retest

@smarterclayton

This comment has been minimized.

Copy link
Contributor Author

commented Aug 2, 2019

@liggitt any concern about naming of the flag?

@wojtek-t

This comment has been minimized.

Copy link
Member

commented Aug 3, 2019

/lgtm

/hold
To let Jordan comment.

@dims

This comment has been minimized.

Copy link
Member

commented Aug 4, 2019

/assign @liggitt

@smarterclayton

This comment has been minimized.

Copy link
Contributor Author

commented Aug 5, 2019

Jordan and I briefly talked about naming, this is consistent with the golang field and if we need to we can rename pre 1.16 launch. I want to determine whether this mitigates the bulk of the latency we see in the graphs.

/hold cancel

@k8s-ci-robot k8s-ci-robot merged commit 4824f82 into kubernetes:master Aug 5, 2019

23 checks passed

cla/linuxfoundation smarterclayton authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-conformance-image-test Skipped.
pull-kubernetes-cross Skipped.
pull-kubernetes-dependencies Job succeeded.
Details
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-csi-serial Skipped.
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gce-iscsi Skipped.
pull-kubernetes-e2e-gce-iscsi-serial Skipped.
pull-kubernetes-e2e-gce-storage-slow Skipped.
pull-kubernetes-godeps Skipped.
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped.
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-node-e2e-containerd Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
pull-publishing-bot-validate Skipped.
tide In merge pool.
Details

@k8s-ci-robot k8s-ci-robot added this to the v1.16 milestone Aug 5, 2019

@liggitt

This comment has been minimized.

Copy link
Member

commented Aug 6, 2019

lgtm

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.