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

Monitor specific queue for long wait if configured #1174

Closed
wants to merge 2 commits into from

Conversation

jasonmccreary
Copy link
Contributor

@jasonmccreary jasonmccreary commented Aug 11, 2022

This is a proposed solution for #1171 to monitor a specific queue if configured.

Currently, when running multiple queues an event is emitted for that "shared" queue. So there's no way to be notified when a specific queue within a "shared" queue has a long wait.

This PR allows you to receive an event for a specific queue, even if running within a multiple queue, when configured via waits.

For example:

    'waits' => [
        // ...
        'redis:automation' => 1800,
    ],

    // ...

    'environments' => [
        'worker' => [
            'shifts' => [
                // ...
                'queue' => ['automation', 'subscriber', 'paid', 'workbench'],
                'balance' => false,
            ],
        ],
    ],

In an effort to be backwards compatible, I did not change the underlying WaitTimeCalculator. Instead, I isolated the change to the listener. It now also monitors any queues configured via waits. This actually provides more flexibility as you may now monitor individual and multiple queues, but potentially combinations of queues which may not run under the same supervisor (would need more testing).

Combined with #1172, I hope this makes the waits configuration more robust for developers needs.

@jasonmccreary jasonmccreary force-pushed the specific-queue branch 6 times, most recently from 9978e07 to 4a2c1e4 Compare August 11, 2022 18:23
@jasonmccreary jasonmccreary marked this pull request as ready for review August 11, 2022 18:31
@jasonmccreary
Copy link
Contributor Author

Please help me with my cascading comment and collection pipeline. 😅

@taylorotwell
Copy link
Member

But is this actually the correct wait times? Maybe it was like this intentionally. If you have a queue worker working "foo, bar, baz, qux" queues... that means it will work the all of the other queues to empty before it processes ANY jobs from "qux". Meaning the wait time for qux would have to be the summation of the prior queues as well.

You can see that logic in the prior code.

@taylorotwell taylorotwell marked this pull request as draft August 15, 2022 18:44
@jasonmccreary
Copy link
Contributor Author

@taylorotwell, yes, that may be correct. I was trying to avoid changing the WaitTimeCalculator for fear of a breaking change.

In the end, this desired feature is really know which queue, in a multiple queue, might have a long wait time. I may need to add a new method to determine that.

@jasonmccreary
Copy link
Contributor Author

jasonmccreary commented Aug 18, 2022

After toying around a bit, there isn't a way to determine the wait time for a specific queue in a shared queue or multiple supervisor configuration without making some assumptions, and therefore not provide an accurate wait time.

As such, I am closing this PR.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants