prolic / HumusAmqp

PHP 7.4 AMQP library
https://humusamqp.readthedocs.io/
MIT License
76 stars 17 forks source link

Add script-wrapper to manage consumers via PCNTL messages #45

Closed fritz-gerneth closed 4 years ago

fritz-gerneth commented 7 years ago

Idle consumers currently cannot be stopped from the CLI using system signals. This is due to queue->consume being a blocking call. SO pcntl_signal_dispatch does not get called to terminate the script.

Idea would be to create a wrapper script around the actual start consumer script that listens to the system signals instead and turns interrupting signals into a shutdown message sent to the consumer's queue. This would shut down the consumer right away (as the queue is empty).

The signal itself still would need to be send to the consumer directly as well to handle situations where the queue is not empty (and hence the shutdown messages is further down the queue). This can be quite tricky to handle as this could potentially lead to situations where the previously send shutdown message is received later once the consumer is restarted.

prolic commented 7 years ago

@fritz-gerneth can you take care of it?

oqq commented 7 years ago

No Script-Wrapper is required.. the extension and library both observes signals to cancel consumers. But this cancelation was not called the right way from this library, so i have moved some code around.

58 should fix that issue without any wrapper workaround.

prolic commented 7 years ago

@oqq I don't think so, see also the readme here: https://github.com/pdezwart/php-amqp

I quote:

Keeping track of the workers

It is a good practice to keep php processes (i.e workers/consumers) under control. Usually, system administrators write their own scripts which ask services about current status or performs some desired actions. Usually request is sent via UNIX signals.
Because amqp consume method is blocking, pcntl extension seems to be useless.

php-signal-handler extension uses signal syscall, so it will work even if blocking method was executed. Some use cases are presented on extension's github page and examples are available here. Note, PHP 5 only.
oqq commented 7 years ago

Same here. Using a Script-Wrapper to kill the whole application is not a clean implementation to get rid of this behavior.

It would be much better to use a endless loop over blocking requests having a timeout (eg. the tv_usec parameter for socket_select). Not sure how to implement this with the c library. But I am pretty sure there is also a non blocking way for that plugin.

prolic commented 6 years ago

@oqq problem is when you have a timeout of 1min f.e. your signals are not coming in for a long time. but what a wrapper process, you have a loop to fetch the signal, and react in time to send a message to the consumer to stop right away. this way the consumer would stop in about 1sec without waiting for a whole minute until the blocking call is done. this is not needed for php-amqplib, but the performance difference is so huge, that I would use php-amqplib only in development environment when you are too lazy to install the extension.

jvdlaar commented 6 years ago

Shouldn't the declare(ticks = 1) be replaced with this function: http://php.net/manual/en/function.pcntl-async-signals.php ? (http://php.net/manual/en/function.pcntl-signal.php#121894) would require php 7.1 though.

prolic commented 4 years ago

pcntl async signals are used now