Is this status report really required if the admin asked for manual check of available translations? Simply bailing out if set to manual would solve the problem of occasionally slow checks harming the status and admin pages. I would suggest:
function l10n_update_requirements($phase) {
if (variable_get('l10n_update_check_frequency', 0) == 0) {
return array();
}
...
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | l10n_update-l10nu-cache-bin-1347440-10.patch | 5.16 KB | sutharsan |
| #6 | l10n_update-l10nu-cache-bin-1347440-6.patch | 3.73 KB | sutharsan |
| #5 | l10n_update-l10nu-cache-bin-1347440-5.patch | 3.4 KB | sutharsan |
Comments
Comment #1
sutharsan commentedThat looks like a good first step. Is there more we could do? If hook_requirements() 'runtime' is called frequently it is worth to squeezing everything out of it.
Comment #2
sutharsan commentedComment #3
sutharsan commentedThe problem lies deeper :(
Localization Update module uses drupal's cache system to store information of available translation updates, retrieved from l.d.o. But the cache is cleared very often (cron, enable/disable module, etc.) and the operation to fetch the update status is very expensive (in my sandbox approx. 450 msec per project). This results frequently in waiting 10+ seconds when visiting the configuration page or the status report page. It also results in way too many calls to l.d.o. Update module had a similar problem: #220592: Core cache API breaks update.module: fetches data way too often, kills site performance, etc
Ignoring the available updates when in manual mode is only a partial solution. And will result in an 'unknown' state on the status page. The problem will remain when daily/weekly frequency is selected. IMHO the volatility of the cache is the root cause.
Abandoning the core cache and making a custom cache system is an option. Perhaps the CTools object cache can be used. Any suggestions?
Comment #4
sutharsan commentedRelated: #1138944: Improve cron updates
Comment #5
sutharsan commentedA new cache bin for l10n_update seems to do the trick. The attached patch has no upgrade path yet, you need to re-install the module to create the new cache table. It needs some work in clearing the cache too.
I don't understand why Update module has chosen to implement a private caching system and whether that is applicable for Localization Update module: http://api.drupal.org/api/drupal/modules--update--update.module/group/up...
Comment #6
sutharsan commentedIncluding cache clearing when running update.php (just like Update module does).
Comment #7
sutharsan commentedI've been working on rewriting the status update process to a batch process, but this turns out to be a major operation. I will continue on that later, but for this issue, I will stick to the above patches.
Your review and test results are highly appreciated.
Comment #8
sutharsan commentedCommitted. Thanks, and sorry for the delay.
Comment #9
sutharsan commentedOops, wrong issue. Back to needs review.
Comment #10
sutharsan commentedPatch slightly changed. Using hook_module_uninstalled() to cleanup after uninstalling a module.
No tests received :( Attached patch is committed.
Comment #11
sutharsan commentedI've made an initial patch to fetch status updates as a batch process: #1430546: Better performance when fetching available update status
Review is appreciated.