Problem/Motivation

This is a D7 backport of: #2319177: Cron lock time limit is too short and does not prevent multiple, concurrent cron runs

Patch from #9 was working, but we decided to add a possibility to opt-out of this change. Therefore we want to introduce a new config option in setting.php.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#2 interdiff-9_2.txt1.29 KBpoker10
#2 3308929-2.patch1.45 KBpoker10

Comments

poker10 created an issue. See original summary.

poker10’s picture

poker10 commented
Status: Active » Needs review
Issue tags: +Needs change record, +Needs manual testing
StatusFileSize
new 3308929-2.patch1.45 KB
new interdiff-9_2.txt1.29 KB

Adding a patch which adds a new settings.php config variable to allow sites override this cron lock expiration timeout. Interdiff is with the patch #9 from the parent issue.

I do not think we can easily create an automated test for this, so setting as Needs manual testing. Also this would need a change record.

mcdruid’s picture

mcdruid commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs change record

Draft CR: https://www.drupal.org/node/3324216

I think this looks good.

One thought; do we want to adjust the time limit that's set just before this too?

Noting that PHP's time limit does not count elapsed wall time and that D9 still has the same 240s limit:

https://git.drupalcode.org/project/drupal/-/blob/9.4.8/core/lib/Drupal/C...

I think that would definitely be one for a follow-up and backport (if none already exists) but seems a little odd that this is still there and hard-coded. Because it only counts PHP execution time, it's probably irrelevant in most cases anyway (unless anyone's mining crypto in PHP on cron!).

  • mcdruid committed 0a9c6cc on 7.x 
    Issue #3308929 by poker10, swentel, fago, catch, alexpott, Berdir: [D7]...

mcdruid credited alexpott.

mcdruid credited Berdir.

mcdruid credited catch.

mcdruid credited fago.

mcdruid credited swentel.

mcdruid’s picture

mcdruid commented
Status: Reviewed & tested by the community » Fixed

I did a little manual testing of this with Xdebug to ensure that a cron lock is now acquired with a 900s timeout.

Thank you!

poker10’s picture

poker10 commented
Issue tags: -Needs manual testing
Related issues: +#2183483: Cron lock time can be outrun by max execution time, +#547998: Make cron time limit a variable

Noting that PHP's time limit does not count elapsed wall time and that D9 still has the same 240s limit:

https://git.drupalcode.org/project/drupal/-/blob/9.4.8/core/lib/Drupal/C...

I think that would definitely be one for a follow-up and backport (if none already exists) but seems a little odd that this is still there and hard-coded. Because it only counts PHP execution time, it's probably irrelevant in most cases anyway (unless anyone's mining crypto in PHP on cron!).

Probably these two issues are related to the potential next steps:

#2183483: Cron lock time can be outrun by max execution time (D7 issue)
#547998: Make cron time limit a variable (D9 issue)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.