Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
attached patch stores agrcache's file list in a separate table.
under heavy load, this reduces the impact of css/js rebuilds causing multiple variable-cache rebuilds, which can have nasty effects on a busy site.
Comment | File | Size | Author |
---|---|---|---|
#13 | 2069473.patch | 6.95 KB | catch |
#12 | 2069473.patch | 6.95 KB | catch |
#11 | 2069473.patch | 6.96 KB | catch |
#10 | 2069473-no-variables-10.patch | 7.16 KB | erikwebb |
#9 | 2069473-no-variables-9.patch | 7.54 KB | erikwebb |
Comments
Comment #1
catchThe custom table is missing indexes - at least needs an index on file type and could use a primary key as well.
Would it be worth caching the files on get - would save a couple of database hits each page when using the alternative cache storage.
Comment #2
Anonymous (not verified) CreditAttribution: Anonymous commentednew patch with some indexes and caching.
Comment #3
Anonymous (not verified) CreditAttribution: Anonymous commentedComment #4
Anonymous (not verified) CreditAttribution: Anonymous commentedComment #5
erikwebb CreditAttribution: erikwebb commentedThe
cache_clear_all()
call was missing an explicit cache bin.Otherwise this works well for me.
Comment #6
erikwebb CreditAttribution: erikwebb commentedComment #7
erikwebb CreditAttribution: erikwebb commentedActually discovered an issue where the file list was updating for the 'footer' scope, then being fully overwritten by the 'header' scope later within the same request. This prevented 'footer' from every being preserved in the database. I've changed the logic so that
Comment #8
erikwebb CreditAttribution: erikwebb commentedI believe the merge operation here actually creates a possibility for a race condition. Since a cache_get() followed by the array_merge(), any changes to the agrcache array between these operations could result in lost files. This implementation will reduce that, but without locking it's still possible.
Comment #9
erikwebb CreditAttribution: erikwebb commentedI refactored this slightly so that, rather than relying on an existing array structure in the database, the module can be more resilient if the database table is empty. This also means we can remove the custom INSERTs at install.
Comment #10
erikwebb CreditAttribution: erikwebb commentedAccidentally added the d.o .info snippet
Comment #11
catchUntil variable_set() stampedes are fixed/mitigated in core, treating this as an agrcache bug.
The update should not use the schema API but specify the table definition directly, re-rolled for that and a conflict.
Comment #12
catchAnother update - the watchdog warning happens straight after updating the module, changed to a notice.
Comment #13
catchAnd db_update() instead of db_query('UPDATE...
Comment #15
catchWith that, this is testing OK. Committed/pushed to 7.x-1.x, thanks!