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

Dynamic resizing of cheroot Threadpool #190

Closed
1 of 3 tasks
ppipada opened this issue Apr 17, 2019 · 12 comments
Closed
1 of 3 tasks

Dynamic resizing of cheroot Threadpool #190

ppipada opened this issue Apr 17, 2019 · 12 comments
Labels
enhancement Improvement good first issue This is what we believe is newcomer-friendly, feel free to contribute! help wanted Somebody help us, please! stale This thing has been ignored for too long

Comments

@ppipada
Copy link

ppipada commented Apr 17, 2019

I'm submitting a ...

  • 🐞 bug report
  • 🐣 feature request
  • ❓ question about the decisions made in the repository

Currently the Threadpool implementation in cheroot provides a grow and shrink method which does not seem to be called anywhere inside cheroot by default. this also means that the "max" value passed in ThreadPool init is not used.
This results in a static number of threads being spawned at init time with no dynamism in response to number of requests handled by the server. As a defensive measure we generally have to init a sufficiently large number of threads (with an appopriate request_queue_size configuration).

🐣 Describe the solution you'd like
ThreadPool should handle the the min and max num threads values.
The grow and shrink decisions can be taken at request start time or can be done as a busy loop. Given that cheroot is a excellent production ready, pure python WSGI server, dynamic load handling should come baked in to it rather than an external package.

πŸ“‹ Describe alternatives you've considered
There is a cherrypy plugin that seems to be written explicitly for doing this: https://pypi.org/project/cherrypy-dynpool/

  • This assumes cherrypy usage and does not consider Cheroot as standalone WSGI server. I would like to use cheroot as a WSGI server independent of what WSGI APP it serves (E.g: Cheroot + Flask)
  • This is an external plugin (relying on one more package "dynpool") for building dynamism into a WSGI Server.

πŸ“‹ Additional context
Cherrypy has a open issue that mentions this as a root cause:
cherrypy/cherrypy#1120

@webknjaz webknjaz added enhancement Improvement good first issue This is what we believe is newcomer-friendly, feel free to contribute! help wanted Somebody help us, please! labels Apr 25, 2019
@webknjaz
Copy link
Member

Hi, I think we're not opposed to this but somebody has to write the code and submit a PR. You are welcome to do so :)

@webknjaz
Copy link
Member

This may be related to the refactoring @the-allanc is doing now.

@the-allanc
Copy link
Contributor

This may be related to the refactoring @the-allanc is doing now.

It's not really. I think the question is whether we bring in cherrypy_dynpool into the cheroot codebase.

@webknjaz
Copy link
Member

@the-allanc what I meant is that your change with having a manager entity should make it may easier..

@stale

This comment has been minimized.

@stale stale bot added the stale This thing has been ignored for too long label Jul 22, 2019
@webknjaz webknjaz removed the stale This thing has been ignored for too long label Jul 22, 2019
@stale

This comment has been minimized.

@stale stale bot added the stale This thing has been ignored for too long label Sep 20, 2019
@webknjaz webknjaz removed the stale This thing has been ignored for too long label Sep 22, 2019
@stale

This comment has been minimized.

@stale stale bot added the stale This thing has been ignored for too long label Nov 21, 2019
@webknjaz webknjaz removed the stale This thing has been ignored for too long label Nov 21, 2019
@stale

This comment has been minimized.

@stale stale bot added the stale This thing has been ignored for too long label Jan 20, 2020
@webknjaz webknjaz removed the stale This thing has been ignored for too long label Jan 21, 2020
@stale

This comment has been minimized.

@stale stale bot added the stale This thing has been ignored for too long label Mar 21, 2020
@webknjaz webknjaz removed the stale This thing has been ignored for too long label Mar 22, 2020
@stale

This comment has been minimized.

@stale stale bot added the stale This thing has been ignored for too long label May 22, 2020
@webknjaz webknjaz removed the stale This thing has been ignored for too long label May 22, 2020
@stale

This comment has been minimized.

@stale stale bot added the stale This thing has been ignored for too long label Jul 25, 2020
@webknjaz webknjaz removed the stale This thing has been ignored for too long label Jul 25, 2020
@stale
Copy link

stale bot commented Oct 4, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale This thing has been ignored for too long label Oct 4, 2020
@stale stale bot closed this as completed Oct 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement good first issue This is what we believe is newcomer-friendly, feel free to contribute! help wanted Somebody help us, please! stale This thing has been ignored for too long
Projects
None yet
Development

No branches or pull requests

3 participants