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

[python] Thread pool not closed or handled on client. #6392

Closed
tuxbotix opened this issue Aug 28, 2017 · 6 comments
Closed

[python] Thread pool not closed or handled on client. #6392

tuxbotix opened this issue Aug 28, 2017 · 6 comments

Comments

@tuxbotix
Copy link
Contributor

tuxbotix commented Aug 28, 2017

Description

This is a potential issue I found on python client after testing a very badly implemented app which create an API client once in a few seconds!! 😛

The client (api_client.py) creates a ThreadPool on initialization, but it is not closed or cleared. From what I found on little-available documentation on threadPool, it has to be closed.

Suggestions:
Add close() and join() for the "destructor". Once the destructor start to close and join the threads, the system showed no problem after many hours of operations throughout few days.

ref: https://www.codementor.io/lance/simple-parallelism-in-python-du107klle

I can do a PR if this is acceptable.

def __del__(self):
  self.pool.close()
  self.pool.join()
Swagger-codegen version

Spec: 2.0, Codegen: 2.3

Command line used for generation

python client. Nothing special!

swagger-codegen -l python

Steps to reproduce

Try to create client once a second or such small interval (speed up to see results quicker xD), the system will run out of threads. (bad implementations can easily show this.!)

@wing328
Copy link
Contributor

wing328 commented Aug 28, 2017

@tuxbotix please submit a PR so that we can review the fix more easily.

(Note: the latest master (2.3.0) has the Python API client refactored)

@rousku
Copy link

rousku commented Feb 8, 2018

This fix introduced another bug:
kubernetes-client/python#411

@tuxbotix
Copy link
Contributor Author

tuxbotix commented Feb 8, 2018

@rousku Interesting situation, I introduced this fix to avoid running out of threads itself when running many clients.
Anyway as I said in my pull request, do we have a better way to do this? perhaps using something other than threadpools which is barely documented !?

@wing328
Copy link
Contributor

wing328 commented Feb 10, 2018

cc @taxpon (2017/07) @frol (2017/07) @mbohlool (2017/07) @cbornet (2017/09) @kenjones-cisco (2017/11)

@cbornet
Copy link
Contributor

cbornet commented Feb 17, 2018

Using a concurrent.futures.ThreadPoolExecutor would probably be better

@cbornet
Copy link
Contributor

cbornet commented Feb 17, 2018

Creating the pool outside of the class so that the same pool is used for all Apiclient instances would also do it I guess.

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

No branches or pull requests

4 participants