Opened 11 months ago
Closed 11 months ago
#60764 closed defect (bug) (fixed)
Translation file cache never expires, causes issues for atomic filesystems
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.5 | Priority: | normal |
Severity: | normal | Version: | 6.5 |
Component: | I18N | Keywords: | has-patch commit fixed-major dev-reviewed |
Focuses: | multisite | Cc: |
Description
In #58919 caching was added for the .mo
file lookups to use a persistent cache.
The only ways that this cache is cleared are:
- When the language pack installer upgrades the files
- When the persistent cache expires, the code specifically specifies no expiry.
External object caches have various limits on what a "no expiry" cache is, with some persisting it literally forever, and others limiting it to a month.
Unfortunately, due to this potentially very long cache, for systems which do not use the language pack installer, this can cause new translation files to never be picked up.
For example, on a system with an atomic filesystem, where WordPress cannot modify the files directly and files are remotely deployed, the file modifications might occur in a wp-cli
instance on a staging environment which doesn't use the production cache.
Additionally; The translations
group is defined as per-site, and not per-WordPress instance, this causes a language pack update on the main site (blog_id=1
) to clear the cache, but on a second site (blog_id=2
) to still have the old file cache.
I also observe; The translations
group doesn't appear to be used anywhere else, given the only content of this is a key which is dynamic, it makes more sense to use a unique group with dynamic keys. For example, here's an example of a current key, and what I'd expect:
{flush_group}:{blog_id}:{group}:{key} 1701761180267934:40:translations:cached_mo_files_a505c11b7ff533560642ea0d95447745 # current 1701761180267934:40:translation_files:a505c11b7ff533560642ea0d95447745 # with the group alteration 1701761180267934:translation_files:a505c11b7ff533560642ea0d95447745 # With the group alteration, and per-instance rather than per-blog
I propose:
- An explicit expiry be added to the
cached_mo_files_*
cache. I would say an hour is plenty, but a day would also suffice probably - The group be renamed, to
translation_files
to remove the key prefix - The cache group be defined as a global group
PR incoming.
Change History (11)
This ticket was mentioned in PR #6264 on WordPress/wordpress-develop by @dd32.
11 months ago
#1
- Keywords has-patch added
This ticket was mentioned in Slack in #polyglots by dd32. View the logs.
11 months ago
#4
@
11 months ago
Another option for those with atomic filesystems would be to flush the cache group after deploy, which is probably needed anyway with this caching in place.
wp_cache_flush_group( 'translations' )
#5
@
11 months ago
- Milestone changed from Awaiting Review to 6.5
If the PR is "too big" given the time until release, the minimal change would be adding an expiry to wp_cache_set()
and the change to wp-includes/load.php
.
Adding to 6.5 milestone for discussion.
#6
@
11 months ago
- Keywords commit added
- Owner set to swissspidy
- Status changed from new to reviewing
Thanks for opening this.
Adding an expiry time and changing the group like this sounds reasonable to me.
I was thinking maybe 12 * HOUR_IN_SECONDS
would be a good expiry, matching update checks, but an hour should also be fine. Still better than no caching or no expiry.
Tentatively marking as ready for commit
/cc @mreishus who reported the original ticket & tested the caching on dotcom.
#8
@
11 months ago
- Keywords dev-feedback fixed-major added
- Resolution fixed deleted
- Status changed from closed to reopened
Reopening for backport sign-off.
@swissspidy commented on PR #6264:
11 months ago
#9
Committed in https://core.trac.wordpress.org/changeset/57831
Trac ticket: https://core.trac.wordpress.org/ticket/60764