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

[5.3] let the worker sleep 1 second more when app is down for maintenance #15520

Merged
merged 1 commit into from
Sep 21, 2016

Conversation

halaei
Copy link
Contributor

@halaei halaei commented Sep 20, 2016

If the queue driver, like sqs, supports long polling, it is better to set --sleep=0 when running queue:work console command. But if it is done so, the CPU will reach 100% when application is down by artisan app:down.

If you find this PR valid, please decide if it is backward compatible or not, and if it should be applied to earlier versions as well.

Thanks.

@GrahamCampbell
Copy link
Member

👎 Don't use zero for sleeping. Makes no sense.

@taylorotwell
Copy link
Member

@halaei can you explain more why you use sleep=0?

@halaei
Copy link
Contributor Author

halaei commented Sep 21, 2016

Short answer

long polling

Long answer

Sleeping is one of the worst practical solutions to wait for job when there is nothing in the queue. Because you don't want your jobs to be delayed because your workers are currently sleeping.
You only want your jobs to be delayed when all the workers are really busy.

One better solution is called long polling.

Amazon SQS supports long polling.

I also wrote my own Redis-based queue driver that supports long polling.

With long polling option available, it is sleeping that makes no sense any more. Because the queue server does the waiting for you in a better way.

@GrahamCampbell GrahamCampbell changed the title let the worker sleep 1 second more when app is down for maintenance [5.3] let the worker sleep 1 second more when app is down for maintenance Sep 21, 2016
@taylorotwell taylorotwell merged commit 3572187 into laravel:5.3 Sep 21, 2016
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.

3 participants