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

Add queue:run-all command #6204

Open
wants to merge 5 commits into
base: 13.x
Choose a base branch
from

Conversation

DieterHolvoet
Copy link
Contributor

@DieterHolvoet DieterHolvoet commented Jan 7, 2025

Adds a queue:run-all command that does the same as queue:run, but for all available queues instead of a specific one. It takes the same options, but without the argument. The limit options count across all queues instead of a specific queue.

Also adds a --memory-limit option that takes either a number that ends with g/m/k (the format of the memory_limit ini setting) or a percentage. The command will run until that amount of memory is consumed. This option is added to both queue:run and queue:run-all.

Also adds a --daemon option that keeps the command running until items-limit, lease-limit or memory-limit is reached - if defined. Else, it keeps going indefinitely. This option is added to both queue:run and queue:run-all.

Fixes #6062.

@weitzman
Copy link
Member

weitzman commented Jan 7, 2025

Thanks for the POC. I'm still hesitant. maybe we doa refactor but keep the new command in contrib. Logging will be an issue. Its not clear which queue some log messages and errors are referring to.

@DieterHolvoet
Copy link
Contributor Author

I did some work on improving the logging:

  • every new queue where an item is claimed is announced (giving context to any errors that may follow)
  • every queue where an item has been claimed gets a summary at the end, the same one as queue:run
  • at the very end of the command execution, a global summary is shown (amount of items and time elapsed)

CleanShot 2025-01-07 at 16 14 02@2x

To be clear, 'Processed X items' does not include failed/skipped items.

@DieterHolvoet
Copy link
Contributor Author

I added the --daemon option as well, will work on the --memory-limit option now. If not for Drush core, for contrib.

@DieterHolvoet
Copy link
Contributor Author

I added the --memory-limit option as well. The MR is all ready now, I just started using the new command in a Supervisor config on a production site as test and it seems to be working so far.

If we decided not to merge this into Drush core, I will probably start a new module with the queue:run-all command. I don't think I'll be able to add the changes to the queue:run command (the new --daemon and --memory-limit) in contrib though.

That said, I am still fan of the idea of adding this to Drush core. It doesn't add a lot of code or complexity and it's very useful.

@weitzman
Copy link
Member

  1. I dont especially want to do more memory checking in Drush. We've done that in the migrate commands and it confuses people. See drush deploy disregards php's memory_limit parameter #4899 (comment)
  2. Why do you think the item-limit should span across queues? That seems weird to me.

Keep the command running indefinitely, or until one of the chosen limits has been reached

If the memory limit is reached, won't folks expect the command to restart itself? Should the above statement only apply to items and time limits?

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.

Run queue tasks continuously
2 participants