[python] Avoid creating unused ThreadPools #1387
Conversation
Instead, create ApiClient.pool on first request for .pool property. avoids spawning n-cpus threads (the default for ThreadPool) at instantiation of every ApiClient
|
|
||
| def __init__(self, configuration=None, header_name=None, header_value=None, | ||
| cookie=None): | ||
| cookie=None, pool_threads=1): |
There was a problem hiding this comment.
It looks like a breaking change. I suggest leaving =None and still using default number of threads. It shouldn't be a problem because the pool will be created on the first request.
There was a problem hiding this comment.
Thanks for the feedback. I'll set pool_thread=None
cc @minrk
There was a problem hiding this comment.
FWIW, this is not a breaking change, though it is a change in the default performance of concurrent async requests from the same client object. Tying concurrent IO to CPU count doesn't make a lot of sense to me, especially in Python. I'd argue that a fixed default pool size (if not 1, maybe 4 or 10) would be more logical than spawning 64 threads on a big machine, and would result in more consistent behavior.
There was a problem hiding this comment.
@minrk I agree with you. This default may not work well on a huge server but it's default, it may be optimal in general, this calculation may be changed in the future releases of Python. Someone can base on this behavior and it'd be a breaking change for him/her.
There was a problem hiding this comment.
What about using pool_thread=None for current master to make it backward-compatible and using pool_threads=1 in 4.0.x (a major release with breaking changes scheduled to be released next month)?
|
|
||
| @property | ||
| def pool(self): | ||
| """Create thread pool on first request |
|
There's also an issue in the Kubernetes client (python) related to this fix: kubernetes-client/python#411 |
* Avoid creating unused ThreadPools Instead, create ApiClient.pool on first request for .pool property. avoids spawning n-cpus threads (the default for ThreadPool) at instantiation of every ApiClient * update doc * set pool_thread to None
…enapi-generator.
* related
* kubernetes-client/gen#93
* swagger-api/swagger-codegen#8061
* OpenAPITools/openapi-generator#1387
* Drop hack for empty reponses
Bump versions:
* sdks to 1.0.79
* core to 1.0.74
Instead, create ApiClient.pool on first request for .pool property.
avoids spawning n-cpus threads (the default for ThreadPool) at instantiation of every ApiClient
PR checklist
./bin/to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.shand./bin/security/{LANG}-petstore.shif updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in.\bin\windows\.master,3.4.x,4.0.x. Default:master.Description of the PR
As discussed with @minrk, here is same PR by cherry-picking the commits.
cc @taxpon (2017/07) @frol (2017/07) @mbohlool (2017/07) @cbornet (2017/09) @kenjones-cisco (2017/11) @tomplus (2018/10)