laravel / horizon

Dashboard and code-driven configuration for Laravel queues.
https://laravel.com/docs/horizon
MIT License
3.87k stars 657 forks source link

Database driver support #421

Closed barryvdh closed 5 years ago

barryvdh commented 6 years ago

I know Horizon currently does not support the Database driver and that there are currently no plans for this. But are you open to a PR to add this? In that case, I would consider creating a proof-of-concept for the database.

I can get Horizon working fine, by just adding the readyNow method to the driver, but would probably need to invest some more time for the actual jobs to show up.

(Yes I know Redis is probably better, but I have a use-case with extra filtering using relations in the jobs which I can't really do with Redis)

driesvints commented 6 years ago

Ping @taylorotwell

barryvdh commented 6 years ago

FYI, this is my current implementation: https://gist.github.com/barryvdh/573049cc167624ecfeae06c6baa68f29 It works next to the Redis queues, showing the database jobs when they are added, processed and failed. It doesn't show in the failed jobs or show any stacktrace, but jobs are processed en visible, which was most important.

halaei commented 6 years ago

By the way, the fact that for every horizon driver, we have to extend some existing laravel class is a little bit troublesome and probably in the way of a clean solution to support multiple drivers. The trouble happened to me when I wanted to change the how laravel interacts with redis, and it happened twice. First I wanted to add blocking pop support before the feature was added to laravel. Here is what I had to do: I wrote a new MyLaravelRedisQueue class to replace it with Laravel's RedisQueue. Then, for horizon support, I had to either:

  1. Extend MyHorizonRedisQueue from MyLaravelRedisQueue and copy some codes from HorizonRedisQueue to it or
  2. Extend MyHorizonRedisQueue from MyLaravelRedisQueue and copy some codes from MyHorizonRedisQueue. Both the solutions are messy. A fix is going to be using composition instead of inheritance. That way we can switch between drivers easier too.
halaei commented 6 years ago

I also think breaking laravel queue classes into 2 is also beneficial. One class should only be responsible for database communication, while the other takes care of other stuff like job encoding decoding before delivering the rest to the first class.

barryvdh commented 6 years ago

Yeah it would be nicer if it would be possible using events or something, instead of replacing the driver itself.

barryvdh commented 5 years ago

So is this something that would be considered? Should I look into a PR?

driesvints commented 5 years ago

@barryvdh I'll take this up with @taylorotwell on Monday when he's back from vacation and we'll get back to you 👍

driesvints commented 5 years ago

@barryvdh atm we're a bit concerned that a DB driver would get deadlocks under high load. Is there any chance you can provide this as a package?

barryvdh commented 5 years ago

I'll look into it. An alternative would be to make sure the Redis version is as easily extendable with filtering etc, but that's hard with Lua script (for me)

I haven't encountered any deadlocks, been running since october. But I'm only running 1 queue with 1 worker on Mysql, rest is on Redis.

driesvints commented 5 years ago

@barryvdh I believe for this to be done we'd need testing on a high load server with multiple processes and supervisors to make sure that we don't run into scaling issues. If you can do that and reply with the results we might consider this further. Feel free to reply once you have some testing done :)

freekmurze commented 5 years ago

@barryvdh

It doesn't show in the failed jobs or show any stacktrace

I wonder why not. Did you not need this functionality, or did you hit any roadblocks? Are still running the code shared via your gist (thank you very much for that btw)?

barryvdh commented 5 years ago

It didn't work for some reason, but didn't investigate further because I didn't need it.

barryvdh commented 5 years ago

FYI, I did encounter 2 deadlocks since my last reply, but they seemed to resolve quickly automatically. But it's not a 'big' queue, just 1 process for a specific task.

mfn commented 5 years ago

I just too a brief look at Laravels DatabaseQueue, but the fact I nowhere can spot logic using SKIP LOCKED is an indicator that the implementation will never truly work.

The current row locking implementation simply doens't cut it for these kind of workloads: if your database does not support true SKIP LOCKED (or an proprietary equivalent), you will always have deadlock race conditions.

Good read (postgres specific though; I've no idea about MySQL): https://www.2ndquadrant.com/en/blog/what-is-select-skip-locked-for-in-postgresql-9-5/

barryvdh commented 5 years ago

Well, there is a reason they suggest using Redis over MySQL ;)

mfn commented 5 years ago

Exactly 👍 :-)

peterpan666 commented 5 years ago

Hi all,

Can anybody explain why deadlocks would be a concern in Horizon but not with Laravel's DB queues?

Or is that what @mfn is saying like we shouldn't really use Laravel's DB queues in production?

Thanks!

barryvdh commented 5 years ago

That last one, you shouldn't use it with high loads.

peterpan666 commented 5 years ago

Thanks for your answer @barryvdh! Any idea what should be considered a high load?

barryvdh commented 5 years ago

I would just always use Redis with Horizon ;)

peterpan666 commented 5 years ago

For the records, there are many issues about this on the laravel/framework repo https://github.com/laravel/framework/issues?q=is%3Aissue+deadlock+queue.

And here is Taylor's answer on the amount of workers https://github.com/laravel/framework/issues/7046#issuecomment-277491802 (1 or 2)

PDXfoster commented 5 years ago

...but what if I'm not running high loads?

If I'm understanding correctly, this is all an attempt to steer dev's away from using the database queue driver - which has its own problems separate from a dashboard that sits on top of that system.

Maybe I'm misunderstanding something about horizon (I'm admittedly a noob to queues in general...) but it seems a little heavy handed to prevent the use of a dashboard for fear that some devs might implement it on something that won't scale well. I just want to have a dashboard to track if my batch emails didn't send, or if a long-running report fails. I'd probably never have more than 2 workers, and if I did, I'd probably look into Redis at that point.

So to that point:

And here is Taylor's answer on the amount of workers laravel/framework#7046 (comment) (1 or 2)

2 workers is a realistic use case, and having an interface for that would be really nice so I don't have to build something out...

barryvdh commented 5 years ago

Horizon needs Redis anyway, so if you're running Redis already, it's just as easy to just use Redis for the queue also.

T-Spoon commented 4 years ago

Is this worth looking at again now that https://github.com/laravel/framework/pull/31287 is merged? So in theory we can run production queues with MySQL/Postgres

yahya-uddin commented 4 years ago

Is it worth re-opening this issue based on https://github.com/laravel/framework/pull/31287 ?

devfaysal commented 4 years ago

In case anyone still looking for queue monitor for Database driver: https://github.com/romanzipp/Laravel-Queue-Monitor

RdeWilde commented 3 years ago

Is there any way to implement/use a database driver? I don't agree with the arguments for not being able to use it at all, and just want to keep my jobs in the database.

halaei commented 3 years ago

@RdeWilde Please check https://github.com/romanzipp/Laravel-Queue-Monitor

julie-langanay commented 3 years ago

Hello @driesvints @barryvdh ! Is there any chance to allow something else than Redis ? We are currently switching to Amazon DynamoDB database to get ride of Redis for our cache & session system and we wonder if we would be able to still use Horizon ? Is there any other laravel package we can switch to otherwise ? Thank you in advance!

driesvints commented 3 years ago

@ibeona sorry but we have no plans beyond Redis atm.