Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect status on duplicate worker #1461

Closed
jasonmccreary opened this issue Jun 6, 2024 · 7 comments
Closed

Incorrect status on duplicate worker #1461

jasonmccreary opened this issue Jun 6, 2024 · 7 comments

Comments

@jasonmccreary
Copy link
Contributor

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

  • Start two identical workers with the same supervisor and horizon config
  • Run horizon:pause on one
  • Start the other worker
  • Run horizon:status on the other
Copy link

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!

@driesvints driesvints added the bug label Jun 17, 2024
@mfrieswyk
Copy link
Contributor

mfrieswyk commented Jun 18, 2024

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
Copy link
Contributor Author

@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
Copy link
Contributor

@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
Copy link
Member

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

@driesvints
Copy link
Member

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
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants