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

Add check to multiprocess supervisor #641

Open
wants to merge 3 commits into
base: master
from

Conversation

@euri10
Copy link
Contributor

@euri10 euri10 commented Apr 18, 2020

should solve #632

it adds a check to see if workers are still alive (limit_max_request will kill them) annd in that case it sets the should_exit event thread

euri10 added 2 commits Apr 12, 2020
…tdown in case of max_limit_requests hit
@euri10 euri10 requested review from rafalp and tomchristie May 4, 2020
self.shutdown()

def workers_alive(self):

This comment has been minimized.

@rafalp

rafalp May 9, 2020
Member

Is it possible for this function to return negative number, eg. when old processes haven't finished shutdown but new ones have already spawned?

This comment has been minimized.

@euri10

euri10 May 9, 2020
Author Contributor

I dont see a situation where it could happen, the max number of workers is the one when it's initialized and set to self.workers_number, after that it can only decrease,

for this number to be negative there would be the need for new workers to be spawned, but in our case it only happens once at start, if it dies it dies

@rafalp
rafalp approved these changes May 9, 2020
@@ -16,13 +16,14 @@


class Multiprocess:
def __init__(self, config, target, sockets):
def __init__(self, config, target, sockets, workers_number):

This comment has been minimized.

@tomchristie

tomchristie May 11, 2020
Member

It doesn't look like we should need workers_number here.
In startup we're using self.config.workers - can we not rely on that instead? (Or am I missing something?)

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

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.