The current cron implementation is as below:

function job_queue_cron() {
  $job_count = db_result(db_query('SELECT count(*) FROM {job_queue}'));
  $max_execution_time = ini_get('max_execution_time');
  if (empty($max_execution_time)) {
    $max_execution_time = 5 * 60;
  }
  while ($job_count > 0 && job_queue_dequeue()) {
    if ((timer_read('page') / 1000) > ($max_execution_time / 2)) {
      break; // Stop once we have used over half of the maximum execution time or exceeds the original number of jobs.
    }
  $job_count -= 1;
  }
}

There are a few issues here:

  1. Other modules executing before job_queue may already consistently use up 50% of the available cron time.

    The above code may always fail in some cases. There are other modules as well (such as Link Checker) which have the same code. There is no guarantee that the other module / modules do not consistently, always use up the first 50% of the max execution time.

  2. Other functions may call set_time_limit()

    If a function invoked during the processing of queued functions calls set_time_limit(), they could potentially shorten the amount of time available for processing to less than job_queue thinks it has, leading to a script timeout.

    Alternatively, if a function executing before job_queue_cron() calls set_time_limit(), $max_execution_time may be empty (handled) or, in some environments, contain the integer that was initially passed to set_time_limit() which may be smaller than double the current page timer causing no queued jobs to ever be processed.

  3. Setting $max_execution_time to 300 if it isn't defined is a recipe for disaster

    Simply because chances are, if the max_execution_time was unset by a call to set_time_limit(), the new time limit may be smaller than 150 seconds, leading to a script timeout.

I don't know a perfect solution to these issues, but we may want to reimplement hook_cron() with them in consideration.

Comments

OliverColeman’s picture

OliverColeman commented

After a quick look around I couldn't find a PHP function that will tell how long you've got till execution times out (which I guess is why job_queue_cron just attempts to use half the total execution time).

Perhaps one (imperfect but hopefully better) solution is:

  • make sure job_queue_cron always executes last by adjusting its module weight
  • add a global variable at bootstrap time (hook_boot) with the time stamp of when Drupal started executing (maybe this already happens?)
  • at the beginning of job_queue_cron use the global variable and max_execution_time to determine time remaining, then attempt to use all of it minus a few seconds of padding.

Like the poster says, this could break down if set_time_limit() is called in a way that lessens the time that would otherwise be remaining, but how many modules do that, if any? Could put a watchdog warning in if it is found that max_execution_time isn't set to alert the admin?

Perhaps could also allow optionally setting the job_queue execution time from an admin page for job_queue, and then calling set_time_limit() at the beginning of job_queue_cron if this is set (and adding a few seconds of padding to allow Drupal shutting down). Could also display the warning that max_execution_time isn't set on the admin page.

fasdalf@fasdalf.ru’s picture

fasdalf@fasdalf.ru commented

add a global variable at bootstrap time (hook_boot) with the time stamp of when Drupal started executing (maybe this already happens?)

This already done by PHP core. Use $_SERVER['REQUEST_TIME'] for that.

brianV’s picture

brianV commented

> Like the poster says, this could break down if set_time_limit() is called in a way that lessens the time that would otherwise be remaining, but how many modules do that, if any?

I believe Boost is one example.

dalin’s picture

dalin
he/him
Primary language English
Location 🇨🇦, 🇲🇽, 🌍
commented

It is not possible to decrease the PHP max execution time. From
http://hk.php.net/manual/en/function.set-time-limit.php

When called, set_time_limit() restarts the timeout counter from zero. In other words, if the timeout is the default 30 seconds, and 25 seconds into script execution a call such as set_time_limit(20) is made, the script will run for a total of 45 seconds before timing out.

dalin’s picture

dalin
he/him
Primary language English
Location 🇨🇦, 🇲🇽, 🌍
commented

I guess I mis-read that. It is possible by doing something like
set_time_limit(1);
Doing so would only give you 1 more second of PHP time, regardless of how many seconds were left before the call to set_time_limit().

buzzman’s picture

buzzman commented

what's the conclusion? i mean, can anyone plz summarize the right thing to do? thnx