-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
[Messenger] Messenger: add --duration
option to messenger:stop-workers
command
#60517
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
base: 7.4
Are you sure you want to change the base?
[Messenger] Messenger: add --duration
option to messenger:stop-workers
command
#60517
Conversation
Hey! I see that this is your first PR. That is great! Welcome! Symfony has a contribution guide which I suggest you to read. In short:
Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change. When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor! I am going to sit back now and wait for the reviews. Cheers! Carsonbot |
Please add a line in the changelog. Please use 7.4 there, as 7.3 is already in feature freeze 🥶 It looks that your committer email is not associated with your GitHub account, you might want to check this. |
3c603a7
to
779fcb2
Compare
7.4 | ||
--- | ||
|
||
* Add `--duration` option to `messenger:stop-workers` command to keep workers in paused state. |
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.
* Add `--duration` option to `messenger:stop-workers` command to keep workers in paused state. | |
* Add `--duration` option to `messenger:stop-workers` command to keep workers in paused state |
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.
Done :)
--duration
option to messenger:stop-workers
command
b604d41
to
60b0c31
Compare
@@ -44,6 +48,11 @@ protected function configure(): void | |||
Each worker command will finish the message they are currently processing | |||
and then exit. Worker commands are *not* automatically restarted: that | |||
should be handled by a process control system. | |||
|
|||
Use the --duration option to keep the workers in a paused state (not processing messages) for the given duration (in seconds). |
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.
Use the --duration option to keep the workers in a paused state (not processing messages) for the given duration (in seconds). | |
Use the <comment>--duration</comment> option to keep the workers in a paused state (not processing messages) for the given duration (in seconds). |
@@ -35,7 +37,9 @@ public function __construct( | |||
protected function configure(): void | |||
{ | |||
$this | |||
->setDefinition([]) | |||
->setDefinition([ | |||
new InputOption('duration', 'd', InputOption::VALUE_REQUIRED, 'Duration in seconds to keep the workers stopped'), |
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.
new InputOption('duration', 'd', InputOption::VALUE_REQUIRED, 'Duration in seconds to keep the workers stopped'), | |
new InputOption('duration', 'd', InputOption::VALUE_REQUIRED, 'Duration in seconds during which workers are paused (not processing messages)'), |
$this->restartSignalCachePool->save($cacheItem); | ||
|
||
$io->success('Signal successfully sent to stop any running workers.'); | ||
if ($duration > 0) { | ||
$io->info(sprintf('Workers will be stopped for next %s seconds.', $duration)); |
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.
$io->info(sprintf('Workers will be stopped for next %s seconds.', $duration)); | |
$io->info(sprintf('Workers will be paused for %s seconds.', $duration)); |
if ($this->shouldRestart()) { | ||
$event->getWorker()->stop(); | ||
$remainingStopSeconds = ceil($this->getEndOfStopTime()) - time(); | ||
$this->logger?->info(sprintf('Worker is stopped and message processing is paused for the next %d seconds.', $remainingStopSeconds)); |
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.
$this->logger?->info(sprintf('Worker is stopped and message processing is paused for the next %d seconds.', $remainingStopSeconds)); | |
$this->logger?->info(sprintf('The worker is paused and message processing will resume in %d seconds.', $remainingStopSeconds)); |
@fabpot done update: Rebased due to some failing tests ([Windows / x86 / minimal-exts)) 😸 |
9e94973
to
a75bc99
Compare
@@ -44,6 +48,11 @@ protected function configure(): void | |||
Each worker command will finish the message they are currently processing | |||
and then exit. Worker commands are *not* automatically restarted: that | |||
should be handled by a process control system. | |||
|
|||
Use the <comment>--duration</comment> option to keep the workers in a paused state (not processing messages) for the given duration (in seconds). |
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.
Use the <comment>--duration</comment> option to keep the workers in a paused state (not processing messages) for the given duration (in seconds). | |
Use the <info>--duration</info> option to keep the workers in a paused state (not processing messages) for the given duration (in seconds). |
if (null !== $duration = $input->getOption('duration')) { | ||
if (!is_numeric($duration) || 0 >= $duration) { | ||
throw new InvalidOptionException(\sprintf('Option "duration" must be a positive integer, "%s" passed.', $duration)); | ||
} | ||
} |
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.
if (null !== $duration = $input->getOption('duration')) { | |
if (!is_numeric($duration) || 0 >= $duration) { | |
throw new InvalidOptionException(\sprintf('Option "duration" must be a positive integer, "%s" passed.', $duration)); | |
} | |
} | |
$duration = (int) $input->getOption('duration') |
$this->restartSignalCachePool->save($cacheItem); | ||
|
||
$io->success('Signal successfully sent to stop any running workers.'); | ||
if ($duration > 0) { | ||
$io->info(sprintf('Workers will be paused for %s seconds.', $duration)); |
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.
$io->info(sprintf('Workers will be paused for %s seconds.', $duration)); | |
$io->info(\sprintf('Workers will be paused for %s seconds.', $duration)); |
if ($this->shouldRestart()) { | ||
$event->getWorker()->stop(); | ||
$remainingStopSeconds = ceil($this->getEndOfStopTime()) - time(); | ||
$this->logger?->info(sprintf('The worker is paused and message processing will resume in %d seconds.', $remainingStopSeconds)); |
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.
$this->logger?->info(sprintf('The worker is paused and message processing will resume in %d seconds.', $remainingStopSeconds)); | |
$this->logger?->info(\sprintf('The worker is paused and message processing will resume in %d seconds.', $remainingStopSeconds)); |
a75bc99
to
9c32d93
Compare
Maybe I miss something here, but calling a stop command with a duration, which does not stop anything sounds weird. Why not a dedicated "pause" command? |
@OskarStark using this option does not change the default behavior of the command. I’m open to suggestions regarding creating a new command, but from my perspective this functionality extends the existing behavior of stopping workers, so I added it as an option to the existing command. |
Hi!
My proposal is to add a
--duration
option to themessenger:stop-workers
command, which would allow to "pause" the workers for the amount of time specified in this option.For example, if we pass
--duration=10
, then for the next 10 seconds, runningmessenger:consume
will return a message indicating how many seconds are left until the "pause" ends.I think this option would be helpful during deployments (we're using it this way in my company).