search

laravel / horizon

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

Incorrect status on duplicate worker #1461

Closed jasonmccreary closed 4 months ago

jasonmccreary commented 5 months ago

Horizon Version

5.24.5

Laravel Version

10.48.12

PHP Version

8.3.1

Redis Driver

Predis

Redis Version

2.2.2

Database Driver & Version

No response

Description

A bit of an edge-case here, but I have duplicate workers that spin up which have the same configuration - supervisor and horizon.

I noticed that if I run horizon:pause on worker-1, then spin up a new worker (worker-2) horizon:status still says "Paused". However, the Horizon web dashboard correct shows the "play" icon for worker-2 and "paused" icon for worker-1.

I'm glad to dig into the code, but don't have time now. So I wanted to get this tracked somewhere. My guess is the logic the horizon:status command uses to check status is not the same as the logic the web dashboard uses.

I've attached some screenshots for a visual diff of the issue where the web dashboard show the status correctly, but CLI does not.

Horizon Dashboard

Horizon web dashboard

CLI

horizon:status command output

Steps To Reproduce

github-actions[bot] commented 5 months ago

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

mfrieswyk commented 5 months ago

The horizon:list command checks all registered supervisors, and if just one has a "paused" status it outputs "Horizon is paused". What would be the preferred output here @jasonmccreary? A table similar to the dashboard?

jasonmccreary commented 5 months ago

@mfrieswyk, I would expect if I am on the worker and run horizon:status it would show the current status for that worker.

In lieu of that, then yeah, a list of all worker statuses would be better, imo, than saying "paused" when the current worker is not the one that is paused.

mfrieswyk commented 5 months ago

@jasonmccreary I went ahead and made this PR to address this issue. It's not a full list of all worker statuses because I felt like changing the behavior of the main status command might be a bit much. Instead I made a new command in the style of the pause/continue commands that requires a name of the supervisor you would like the status for. Hope this helps.

driesvints commented 4 months ago

@mfrieswyk do you have time to answer Taylor's question on that PR?

driesvints commented 4 months ago

I'm going to close this one for now since it's been a while. Would still love to receive a working PR or work being picked up on the PR above. Thanks

mfrieswyk commented 3 months ago

@driesvints sorry for the delay, pushed updates to the PR