docker-archive / docker-registry

This is **DEPRECATED**! Please go to https://github.com/docker/distribution
Apache License 2.0
2.88k stars 879 forks source link

OPS-5907: remove ParallelKey class #961

Closed chaiken-verticloud closed 9 years ago

chaiken-verticloud commented 9 years ago

This pull request removes the ParallelKey class as discussed in issue #956. Performance tests on AWS with an m3.medium docker client and different size registry servers (m3.large, m3.xlarge, c3.xlarge) indicate that:

There may be some use cases where ParallelKey helps performance, but I couldn't find them with the test configuration using the m3.medium client.

dmp42 commented 9 years ago

One simple nit, LGTM otherwise.

chaiken-verticloud commented 9 years ago

I restored the gevent.monkey.patch code, which causes the following flake8 errors:

depends/docker-registry-core/docker_registry/core/boto.py:32:1: E402 module level import not at top of file
depends/docker-registry-core/docker_registry/core/boto.py:33:1: E402 module level import not at top of file
depends/docker-registry-core/docker_registry/core/boto.py:35:1: E402 module level import not at top of file
depends/docker-registry-core/docker_registry/core/boto.py:36:1: E402 module level import not at top of file
depends/docker-registry-core/docker_registry/core/boto.py:37:1: E402 module level import not at top of file

flake8 generated similar errors before I modified the file.

I'll test the code with the restored gevent.monkey.patch (even thought I don't expect any problems from it).

chaiken-verticloud commented 9 years ago

I tested the code with the restored gevent.monkey.patch with both [AWS_SECURE=false; STORAGE_REDIRECT=true] and [AWS_SECURE=true; STORAGE_REDIRECT=false]. Both worked as expected. Performance looks unchanged.

dmp42 commented 9 years ago

In order to make flake/hacking stop complaining here, you can:

- gevent.monkey.patch_all()

+ gevent.monkey.patch_all() # noqa

LGTM otherwise - merging.

dmp42 commented 9 years ago

Fixed #956 and #954