Since 8.1 Cron::processQueues() does this:

$lease_time = isset($info['cron']['time']) ? : NULL;

setting lease_time to the maximum time new queue jobs may be executed.

Before it was the default 1 hour for claimItem.

Now, we do have a queue (cron is called every 5 minutes) where one task could take a little more than 5 minutes (downloading a large file). But most tasks are shorter, to be save, I set the queue time to 50 seconds (I don't want to start a queue item after that time).

What now happens is: processQueues sets the lease to time+50 sec, the task works for more than 5 minutes, the next cron call arrives before, finds lease has expired, and calls the same item again, again taking 5 minutes.

Suggestion: leave lease_time at 1 hour or add another optional annotation.

CommentFileSizeAuthor
#12 core_cron-lease-time_2833657-12_D8.patch751 bytesrkostov
#6 cron_uses_max_time_as_lease_time.patch751 bytestuneke

Comments

kuebel created an issue. See original summary.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

dropa’s picture

dropa commented

Actually, the $lease_time is either TRUE or NULL, which is a bug in my eyes. So I believe you haven't ever even gotten the +50seconds to the lease time.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

tuneke’s picture

tuneke commented
StatusFileSize
new cron_uses_max_time_as_lease_time.patch751 bytes

Here is a patch to actually use the maximum time new queue jobs may be executed as lease time.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

nuez’s picture

nuez
Primary language Dutch
Location Madrid, Spain
commented
Related issues: +#2883819: Cron does not use lease time correctly when claiming item
nuez’s picture

nuez
Primary language Dutch
Location Madrid, Spain
commented
Related issues: +#2893933: claimItem in the database and memory queues does not use expire correctly
nuez’s picture

nuez
Primary language Dutch
Location Madrid, Spain
commented
Status: Active » Closed (duplicate)

This issue is closely related to

rlmumford’s picture

rlmumford
Primary language English
Location Manchester
commented
Title: Lease time should not be queue work time » Cron Queue items are always claimed with a lease time of 1 second
Version: 8.6.x-dev » 9.2.x-dev
Category: Feature request » Bug report
Priority: Minor » Major
Status: Closed (duplicate) » Needs review

I'm reopening this, as the supposed duplicate makes a large API change that will take a long time to be accepted and this issue is a major bug. The patch in #6 resolves the immediate issue.

rkostov’s picture

rkostov commented
StatusFileSize
new core_cron-lease-time_2833657-12_D8.patch751 bytes

In case anyone needs it, a reroll for the 8.9.x branch.

rlmumford’s picture

rlmumford
Primary language English
Location Manchester
commented
Status: Needs review » Reviewed & tested by the community
pfrenssen’s picture

pfrenssen
Location Sofia
commented
Status: Reviewed & tested by the community » Closed (duplicate)

Thanks for the reroll!

I agree with @nuez however that this issue is a duplicate of #2893933: claimItem in the database and memory queues does not use expire correctly. This one is not so close to being committed as it may seem, since it lacks test coverage. The other issue has tests to prove that the bug is conclusively fixed, so it is further ahead in the race to be included in core.

I agree that the scope of this is narrower than the other issue, but still I would propose to focus efforts in reviewing that issue instead of spending the effort to duplicate the tests in this issue. It would just introduce a merge conflict that adds again more work.