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

[WIP] Configurable layer compression during push #34610

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@GaretJax
Copy link
Contributor

GaretJax commented Aug 23, 2017

Initial work to fix #1266 by making compression configurable. This PR is far from complete, but I'd like to collect feedback before investing any more time in the wrong direction. I'm far from a docker internals or go expert, so I can use guidance on all levels (testing, documentation, code style, design,...).

This PR currently just extracts the configuration of the compressor from the push internals up to the Daemon.PushImage method; additional plumbing is needed to pull it out of that method and into the API request handler.

As stated in the original issue, there are some open points:

  1. Do we want to handle just the levels which can be passed to the gzip writer (https://golang.org/pkg/compress/gzip/#pkg-constants) or do we want to support other compression algorithms altogether?

I'm leaning towards just allowing gzip levels, but without excluding a future evolution to support custom algorithms altogether. Also, I did not check what is needed on the metadata/pull level to be able to support other algorithms during pull. This works transparently as long as we stick to gzip, but I can imagine we need additional support if we want to use a different algorithm.

  1. Where should the level/algorithm be specified? Via the CLI (which then also requires changes to the API) or in the configuration? In the latter case, would it be per daemon or per registry (more appropriate)?

Would passing the compression algo/options using the HTTP headers of the push API endpoint be a viable solution? It looks like the easiest and least intrusive at the API level.

A third point is: if a layer is already uploaded to a registry with a different compression scheme, then any new pushes of that layer would be skipped, even if the compression would be different. This is not an issue per-se, but it is something to be aware of.

Initial work on pluggable layer compression
Signed-off-by: Jonathan Stoppani <jonathan@stoppani.name>
@cpuguy83

This comment has been minimized.

Copy link
Contributor

cpuguy83 commented Jan 10, 2018

ping @stevvooe

I believe we'd prefer to move to doing compression at the transport layer rather than on the actual layer themselves... but I'm not particularly familiar with this area of the code base.

@cpuguy83

This comment has been minimized.

Copy link
Contributor

cpuguy83 commented Jan 23, 2018

I'm going to go ahead and close this as it has gone stale, and likely not something we want to handle at this layer instead handling it at the transport layer.

Thanks! Feel free to comment/discuss. 😇 🙇

@cpuguy83 cpuguy83 closed this Jan 23, 2018

@GaretJax

This comment has been minimized.

Copy link
Contributor

GaretJax commented Jan 23, 2018

Thanks for the response @cpuguy83.

What does this mean with regard to the original issue? I’d very much like to eventually see a way to disable compression for pulls from a private registry; how it is implemented (or at which layer the compression happens) is not important to me.

Is there work being done in this direction that I can track (or maybe help with)? As far as I can see the original issue has also been lacking updates for a while.

@cpuguy83

This comment has been minimized.

Copy link
Contributor

cpuguy83 commented Jan 24, 2018

@GaretJax This is looking for someone interested in taking it on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment