-
Notifications
You must be signed in to change notification settings - Fork 659
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
[5.x] Add commands to pause and continue supervisors #914
Conversation
I've slightly modified the PR so that both full and short names of supervisor work. E.g. both of the below commands work:
The second one is the same name as appears in the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice, just a suggestion regarding the wording!
$this->info("Sending CONT Signal To Process: {$processId}"); | ||
|
||
if (! posix_kill($processId, SIGCONT)) { | ||
$this->error("Failed to kill process: {$processId} (".posix_strerror(posix_get_last_error()).')'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Failed to kill process
Just a small feedback that this is not what's happening from the user perspective: we're not trying to send SIGKILL or SIGTERM here, maybe it should be rephrased to "Cant send signal SIGCONT to…" or so?
$this->info("Sending USR2 Signal To Process: {$processId}"); | ||
|
||
if (! posix_kill($processId, SIGUSR2)) { | ||
$this->error("Failed to kill process: {$processId} (".posix_strerror(posix_get_last_error()).')'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Failed to kill process
Basically the same feedback, something like "Cant send signal SIGUSR2 to…" 🤔
@mfn can you send in a PR for those? |
@mfn, this resembles the pause/continue commands for the master supervisor. It stems from a funny unix history, where the unix |
See discussion outcome in #914 (comment)
@paras-malhotra can you maybe PR these new commands to the docs? |
@driesvints sent a PR to the docs: laravel/docs#6544 |
@paras-malhotra thanks! :) |
Currently, it is possible to pause and continue the master supervisor. But there is no command readily available to pause and continue specific supervisors. This could be a common use case in production.
For instance, say we want to pause a supervisor that runs a dedicated queue for unstable APIs. If the API is currently unavailable, pausing that specific supervisor really helps so that the jobs are queued up and can be continued (without exhausting their
tries
) once the API is up and running.EDIT: I've added separate commands instead of modifying existing
PauseCommand
andContinueCommand
(used for master supervisor) so that there's no breaking change and this PR can be included for 5x.