search

laravel / horizon

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

Fix worker scaling when balance mode is auto #1464

Closed michabbb closed 1 month ago

michabbb commented 1 month ago

fix https://github.com/laravel/framework/issues/51765 fix https://github.com/laravel/horizon/issues/1295

as explained: https://github.com/laravel/framework/discussions/48431

if horizon is in mode balance=auto, there is no need to start hundreds of workers if the queue is empty. it's okay to start with only 1 worker. if the queue fills, horizon scales up. that's the way it should work.

taylorotwell commented 1 month ago

I think this is just an opinion of how it should work and probably not worth changing on a patch release.

michabbb commented 1 month ago

@taylorotwell it's very frustrating to see that nobody at laravel understands the simple fact, that workers should scale up, and not down. If the queue is empty, why should the server run on max? Why? That makes no sense. This whole thing is glass clear. I have a project where 200 workers can scale up. Why the heck should all 200 workers start from the beginning and get ended after a few minutes, when horizon finally noticed: "oh, there is absolutely nothing to do for me, so let's kill all the works we didn't need in the first place" 🙈

So, what's needs to be done to fix that? 😏 Please, let us know.🙏

michabbb commented 1 month ago

Besides that: with all changes laravel had in the past, there was always the will to evolve, keep things simple, remove overhead, make things slim and minimal.

Saying, "it's just an opinion" that it makes sense to start something that is not needed (tons of workers when the queue is empty) - speaks completely against your own principles.

I appeal to your reason and ask you: please think again. Thank you.

vesper8 commented 6 days ago

@michabbb maybe if you make a benchmark that shows a slower launch or degraded performance as a result of the way it's currently implemented then they'll think it's more than just an opinion. I haven't tried it myself but I would guess that spawning this many workers and then de-spawning them would create a potentially huge memory spike for no justifiable reason.. if that's what's happening in your case. If not then.. I guess it doesn't really matter then.

michabbb commented 6 days ago

@vesper8 thanks for your willingness to help. I really think that everything is already said, multiple times.

Of course you have a huge spike, cpu, memory, maybe database connections, everything - and everything is completely Unnecessary. It's frustrating to see someone who invented laravel but at the same time is unwilling to understand such a simple fact 😔

It's NOT an option, as Taylor said - it's just wrong. The concept is wrong By design. Scaling in the world of queues just simply does not work like that. Since laravel is not a democracy, it's a one-man-decision show, we have to live with that.

At least it's open source, so in my ci process I will deal for the rest of my life with a tiny hack that changes one single line in horizon - it is, what it is. 🤷