Adding Pause Support on Redis

dbpolito commented 6 months ago

This PR adds support to use redis for pause / continue horizons instances, without removing existing behavior.

Motivation

The current implementation relies on you being able to ssh into the server is running horizon so you can pause, which can be limiting in the following scenarios:

Multiple Servers

If you have multiple horizons instances running, in case you need to pause, you would need to ssh on each instance and call pause.

Blue/green deploying on Kubernetes

On Kubernetes it's quite a challenge ssh into the container, specially if you have replicas, etc.

Also, if you want to pause while deploying for example, you probably want to use something like initContainer or before helm hook to run that, which will spawn a new container to run that.

On current implementation it's quite impossible.

In my case, as the old horizon is still running while we're deploying, we can have race conditions, running job on old code until we finish deploy and kill the old one. We should always recommend running pause before deploying i think (not sure it's already done somewhere) 🤔

Looking for Feedback

I kept the existing signals approach so we kind of do the pause logic twice... what we could do:

Add some config to choose between approaches
Remove signals and rely on cache / redis for this
I added a docker-compose.yml to make easier to test locally
I changed the test worker driver to predis as it skips the requirement of redis extension on local

dbpolito commented 6 months ago

@crynobone done

not sure i had to change back to ready to review...

taylorotwell commented 6 months ago

Don't quite have time to dig into this one right, but I'm also not sure it would totally solve all issues in this regard. Pausing wouldn't stop or prevent any jobs that had just started running. Those would continue to run on old code during your deployment. If they are long running jobs they could even run until after your new deployment is finished on the old code.

dbpolito commented 6 months ago

Well, in my case i use https://github.com/TheDragonCode/laravel-deploy-operations, if you push a new job into the Queue, the old horizon instance would pick the job and fail with class doesn't exists...

So in that scenario, not picking new jobs is enough...

But the existing behavior wouldn't kill the running jobs, right? So the behavior would be consistent, just not picking new jobs... which makes sense for me...

The problem is, with current implementation, we can't have a way to implement it using events...

@taylorotwell would you be willing to at least accept a PR adding an event like SupervisorLooping to dispatch before the loop start? around here? https://github.com/laravel/horizon/blob/5.x/src/Supervisor.php#L286

That would make at least possible to implement it on the user land.

laravel / horizon