BUG: Doorman manager child processing fix.

[mfendeksilverstripe]

Doorman manager child processing fix

Problem

Using asynchronous queue runner has a flaw - when the manager process terminates, it also terminates all of its child processes. This is a mistake because there is a valid use case where manager process doesn't have any more work to do (no more jobs to start) but the child processes are still executing jobs.

Solution

Fix the shell command and prevent the manager process from killing of the child processes.

Notes

• Requires 3.1.0 version or higher of asyncphp/doorman
• split from Feature batch as Feature 9 - Job manager process

Related issues

#306

[emteknetnz]

Code wise its fine, though a couple of questions

You'll need to rebase/retarget this to 4 since it has new API

Also, would it be worth simply making this PR straight to asyncphp/doorman instead?

[emteknetnz]

Is there a valid reason why the default behaviour may be good? Is it worth making this behaviour a configurable value?

[mfendeksilverstripe]

The default behaviour is generally considered a more safe option and I would still want to keep it as a default. The only reason why we can bypass this here is that this module has a very good system in place which keeps track of running jobs (job locking...).

[emteknetnz]

Ok, so you're saying that it's no worth making this configurable because the queued jobs module does such a good job?
Should we do either of the following?
a) make this configurable?
b) add your rationale for why it's ok to bypass into the inline comments?

[mfendeksilverstripe]

I had a look at it in detail and I think it's worth making it configurable. There might be some cases where you want this to be disabled.

[mfendeksilverstripe]

Rebased to 4, pushed up new changes, please review.

[mfendeksilverstripe]

@emteknetnz To answer your question:

Also, would it be worth simply making this PR straight to asyncphp/doorman instead?

I don't think this changeset belongs to the doorman module as the change by itself is probably a bit dangerous default. In the context of this module it's different as there are other mechanisms in place which makes this change reasonable (job locking, execution timeouts).

[emteknetnz]

Mojmir, looking good, just a couple of questions around the destruct function

[mfendeksilverstripe]

@emteknetnz Pushed up new changes (configurable child process termination + docs), please review.