Issue 4032 queue run lease time #4322
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… of the queue instead of using the default of 60 minutes.
…sing like core
…nknown exception during queue processing
…ues at play now
* master: Remove latest unstable chiclet. Its not helpful. Remove mentions of Shippable. (drush-ops#4079) Add mysql service to Travis config. (drush-ops#4078) Fix link in docs. More docs about dynamic users in site.yml files. Fix drush-ops#4055. Sort inside namespaces in list command. (drush-ops#4057) Print class name in drush_op() Update dependencies. (drush-ops#4051) UNISH_NO_TIMEOUTS is unused since we moved to CLITrait. (drush-ops#4047) Add jsonapi command (drush-ops#4045) Fixes drush-ops#4034: Ensure that status code is nonzero on invalid shutdown (drush-ops#4037) Add webflo/drupal-core-require-dev (drush-ops#4041) Fix TTY detection for runserver command. (drush-ops#4033) Add vagrant transport to Drush. (drush-ops#4036)
…run-lease-time # Conflicts: # src/Drupal/Commands/core/QueueCommands.php
|
Hoping @juampynr, @damiankloip or @davereid can chime in here. Vaeguely recall you guys contributing to this command in the past. |
|
Pity we couldn't get any reviews on this. I just added a bit of polish. I'll merge when it comes back green. I don really follow the lease-time example in the OP but the code is simple to follow so in it goes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #4032. Builds on #4083. This includes the work from @lems3 and @m4olivei.
I've resolved the merge issues due to more recent changes. I think there's value to separating out the queue lease time, so I've done that here also. As an example, let's say we're using the same value both time limit and queue lease and I know queue worker X takes 30s on average, so I use
--time-limit=45to be on the safe side.On one run, it processes item A in 40s. There are still 5s left, so it claims item B, but item B is claimed with an expiry that's only 5s in the future and we know that it's very unlikely to complete within that time frame, leaving the possibility that Drupal will reset item B and a second worker will claim it while it's still being processed by the first worker.
By defaulting to an hour (which the
QueueInterfacesays is the default) and allowing someone to specifically allocate the lease time, we can avoid this issue.