I enabled the update feature that emails me whenever the site has an update available. I noticed that the time the email gets sent is always incremented by one cron run. For example, I have cron running hourly, so if there are updates needed it will email me at 8am, the next day at 9am, the next day at 10am, etc. But, based on the math, whatever interval cron runs at the email will likely always be one run later.
In hook_cron, the if statement requires the current time to be greater than interval (in my case, 24hrs). If it is exactly 24hrs, it will not pass.
function update_cron() {
$frequency = variable_get('update_check_frequency', 1);
$interval = 60 * 60 * 24 * $frequency;
// Cron should check for updates if there is no update data cached or if the
// configured update interval has elapsed.
if (!_update_cache_get('update_available_releases') || ((time() - variable_get('update_last_check', 0)) > $interval)) {
update_refresh();
_update_cron_notify();
}
}
Also, 'update_last_check' is set in _update_refresh(), after some work is done. You only need enough time between a cron run and the setting of 'update_last_check' for a second to turn over to guarantee this not running at the same time.
In my install, I moved the variable_set for 'update_last_check' from _update_refresh() to the first thing inside this if, and change the operator to >= (as the difference more often ends up exactly zero). It has been working for several days, sending the update email at the same time.
Also, it would be nice if there was a setting to say exactly when to send email updates: #3389183: Allow defining when to check for updates and send alert emails.
| Comment | File | Size | Author |
|---|---|---|---|
| #23 | 1137628-23.patch | 1.45 KB | ameymudras |
| #8 | update-fix-cron-time-1137628-8.patch | 884 bytes | NROTC_Webmaster |
| #7 | update-fix-cron-time-1137628-7.patch | 784 bytes | NROTC_Webmaster |
Comments
Comment #1
damien tournoud commentedMakes sense. I guess 7.x and 8.x are affected, so moving there for study.
Comment #2
davidhernandezI haven't tested 7/8, but did look over the code. Much of the code has changed, but the variable_set is still towards the end of the process, so I would also suspect 7/8 has the same issue.
Comment #3
bfroehle commentedwouldn't it be easier to just change the interval to something like
$interval = 60 * 60 * 24 * $frequency - 10;where the constant was chosen to be 10 seconds or whatever.Comment #4
davidhernandezI've been testing this over the last couple weeks, and I'm getting mixed results. Sometimes it works perfectly, and some times it still misses (it depends on the consistency of cron, and how long it takes to run)
@bfroehle - Yes, putting in some arbitrary delay would work. That is the other solution I've been playing with. However, I think both these solutions are a bit inaccurate and inelegant. They both have a set of cases where they won't work well for some setups.
I would rather something more exact, like specifying the time to send. Speaking from a sys admin/site maintainer point of view, I like to control when the emails go out. Most people have a work flow where they prefer to receive daily statuses within certain time frames, like in the morning. That might just be me though. I'm assuming the update is based on whenever the project version is committed? So this could be any time during the day?
Would setting a variable for the delay be acceptable? In the testing I've done, some of my sites would be fine with 10 seconds, some would need more than 30. I'm sure other people vary even more.
Comment #5
bfroehle commentedThe other thing to consider here is if this is even worthwhile... Is it in any way important that the email gets sent at a particular time every day?
Comment #6
davidhernandezScratching my own itch, as it were, it is worthwhile. As a sys admin, I prefer all emails to come at designated times. But, I agree that the population, in general, probably doesn't care.
Comment #7
NROTC_Webmaster commentedI took a little from both suggestions and this has been working for me. I think the important thing is to fix this for the majority of sites and not necessarily every single one. Making this small change now may help lead to fixing this issue completely in the future.
Since cron can only run every hour I took a minute off the interval and hopefully that will work for everyone since I can't imagine cron running for more than a minute on any site.
Comment #8
NROTC_Webmaster commentedWith comments this time.
Comment #9
kscheirer#8: update-fix-cron-time-1137628-8.patch queued for re-testing.
Comment #19
catchThis is still valid for Drupal 9:
Making the update interval a few minutes shorter, and the e-mail interval a few minutes shorter than that, seems like it'd be sensible.
Comment #23
ameymudras commentedSimilar to #7 and #8 subtracting 60 seconds to adjust time
Comment #25
kristen polThanks for the patch. Tagging... I understand the issue summary but it would be good to have explicit steps to reproduce and someone needs to test the patch manually.
Comment #26
smustgrave commentedAlso tagging for tests;
Comment #27
luke.stewart commentedMy suspicion would be that we need to consider Day Light savings here.
Additionally the REQUEST_TIME global constant has also been deprecated
https://www.drupal.org/node/2785211
Currently the patch assumes each day is 24 hours long. Perhaps the use of datetime.diff would prevent shifts occurring due to day light savings.
Comment #29
ressaI agree, it would be nice to be able to define when to send the email alert, and created #3389183: Allow defining when to check for updates and send alert emails.