This patch adds more caching, now for "Explore languages" and "Explore projects" pages. It also refactors a previous caching patch a little bit (new cache cid) and adds a settings variable l10n_community_stats_cache_lifetime to define cache lifetime. Default is 60 seconds, d.org will probably set it to 1 day.

hook_cron is changed not only clears all caches but also recalculate them (can take a long time). Projects and Languages sums were moved to a separate functions l10n_community_get_string_count_languages. The process for cron recalculating is now:

  1. Recalculate sums for projects/languages
  2. Clear cache for projects/languages
  3. Set new cache

Thanks to this, there is ALWAYS some cache variable and the gap between "clear cache" & "set new cache" should be minimal so it shouldn't happen that any user will publicly hit a page without caches set and therefore wait 500-600 seconds for the page.

This patch is for 6.x as Gabor requested to move all development to 6.x branch.

CommentFileSizeAuthor
#5 l10n_community_cache_3.patch13.91 KBmeba
#3 l10n_community_cache_2.patch13.93 KBmeba
l10n_community_cache.patch14.12 KBmeba

Comments

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Needs work

Looks good, but I would not do rebuilding all counts on cron. The user would only hit a page requiring regeneration of one of the myriads of caches right, not the whole array of caches the module does? So then why not let the user hit pages, and regenerate the individual caches, instead of trying to go and regenerate all caches on cron, even though we might not even need some caches (less visited language pages) before the cache will be invalidated again. What do you think?

meba’s picture

meba commented

Seems reasonable. And what about we clear & rebuild only the caches which actually exist? It means that after you install l10n server, somebody will hit the page and rebuild the cache. Then every cron run, it will do:

foreach ($languages as $lang) {
  if (cache_get('l10n:...$lang')) {
    // rebuild
  }
}

Therefore only existing pages will get rebuilt and the others may be invalidated after for example 24 hours...

Anyway, I would be happy to remove recalculating for language stats but Projects and Languages are taking more than 5 minutes to rebuild and the user is never going to wait that long :)

meba’s picture

meba commented
Status: Needs work » Needs review
StatusFileSize
new l10n_community_cache_2.patch13.93 KB

Rerolling without language overall statistics recalculating. Cache system now:

1) Caches Quick stats, Progress stats, Explore languages, Explore projects screens
2) Clears cache every configured interval
3) Recalculates overall Quick and Progress stats without language, recalculates Explore languages/projects stats every configured interval

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Needs work

This looks like a work in progress. I've found at least two debug/incomplete things:

- l10n_community_get_string_count_languages() lacks an $id parameter, while the code inside it tries to use it right away. Overall the use of this language counting does not account for the fact that it might be restricted by project id (including the cache handling)
- there is a print_r($projects_sums); in the code.

Otherwise looks good.

meba’s picture

meba commented
Status: Needs work » Needs review
StatusFileSize
new l10n_community_cache_3.patch13.91 KB

Thanks for the suggestions, I knew I left print_r somewhere and the first one was a bug on Project details page I was trying to track down but I didn't know it's related to this.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Closed (fixed)

Similar caching was committed earlier with other patches. The overall project and language list caches are regenerated on cron and the highlighted project cache is also regenerated. The others are regenerated when needed/visited. So this looks like already fixed elsewhere.