Skip to content

Include sleep option in queue config #564

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

Merged
merged 3 commits into from
May 8, 2025

Conversation

chr15k
Copy link
Contributor

@chr15k chr15k commented Apr 22, 2025

This PR exposes the sleep argument for the queue:work command. The motivation behind this change is to reduce the delay between dispatching an asynchronous job and a worker picking it up. This delay can create a noticeable lag in the UI, which I'm currently experiencing. In my case, setting sleep=0 resolves the issue.

Copy link
Member

@PeteBishwhip PeteBishwhip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thanks for your first contribution!

Could you perhaps also propose a documentation update on the nativephp.com repo for this?

I'm requesting review from other core team members to make sure this aligns with expectation. :)

@PeteBishwhip PeteBishwhip requested review from SRWieZ and gwleuverink May 7, 2025 10:00
Copy link
Contributor

@gwleuverink gwleuverink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! I don't think this has any negative impact on existing apps

@chr15k
Copy link
Contributor Author

chr15k commented May 7, 2025

Looks good to me. Thanks for your first contribution!

Could you perhaps also propose a documentation update on the nativephp.com repo for this?

I'm requesting review from other core team members to make sure this aligns with expectation. :)

Hi @PeteBishwhip, thanks! I've created a PR for the docs here: NativePHP/nativephp.com#125

Copy link
Member

@SRWieZ SRWieZ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you tell me what the CPU usage is when you set it to 0? I'm concerned that it will result in high CPU usage. Also, this is currently using SQLite as a driver, which may result in locking your database.

I would like to set a better default for nativephp, maybe 1? I'll try after this PR is merged.

@SRWieZ SRWieZ merged commit 7552867 into NativePHP:main May 8, 2025
18 of 19 checks passed
@chr15k
Copy link
Contributor Author

chr15k commented May 8, 2025

Can you tell me what the CPU usage is when you set it to 0? I'm concerned that it will result in high CPU usage. Also, this is currently using SQLite as a driver, which may result in locking your database.

I would like to set a better default for nativephp, maybe 1? I'll try after this PR is merged.

Hi @SRWieZ great points, I actually ended up using sleep=0.1 to avoid the tight polling loop, and in my project noticed no difference in responsiveness between 0 and 0.1. It starts to become noticeable at 2-3 seconds. Agree a good default would be 1.

Re. the CPU usage, sleep=0 caused a rise of around 10-20% on my M1 Mac, compared to setting it to 1.

@SRWieZ
Copy link
Member

SRWieZ commented May 8, 2025

Thanks for the feedback.

0.1 works ? It's declared as an int in this PR.

@chr15k
Copy link
Contributor Author

chr15k commented May 8, 2025

Thanks for the feedback.

0.1 works ? It's declared as an int in this PR.

Good catch. I tested float sleep outside of native, and only in more recent tests realized that Laravel's Worker::sleep() implementation actually supports sub second granularity. Happy to open a new PR to fix the type hint.

Update: PR created #583

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.

4 participants