Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
I have a problem with update core from 8.6.x to 8.7.x. My site use default content type article with field_tags. Currently, I have about 600k term and when I can't finish drush updb
because memory leak issue. When post update Taxonomy terms have been converted to be revisionable run about 90k this task using about 4G ram and still increase.
Comment | File | Size | Author |
---|---|---|---|
#11 | interdiff.txt | 828 bytes | amateescu |
#11 | 3074949-11.patch | 786 bytes | amateescu |
#9 | 3074949-8.patch | 763 bytes | amateescu |
#5 | 3074949.patch | 774 bytes | doidd |
Comments
Comment #2
doidd CreditAttribution: doidd commentedComment #3
cilefen CreditAttribution: cilefen at Institute for Advanced Study commentedThis could be a duplicate of #3056540: taxonomy_post_update_make_taxonomy_term_revisionable() timeouts with large numbers of terms or one of the issues related to #3052464: Cannot update to 8.7.0 because of taxonomy_post_update_make_taxonomy_term_revisionable.
Comment #4
doidd CreditAttribution: doidd commented@cilefen
My Stack: docker, nginx, php 7.3, postgresql
Comment #5
doidd CreditAttribution: doidd commentedI fixed this issue by the patch.
Comment #6
catchComment #7
catchComment #8
amateescu CreditAttribution: amateescu for Pfizer, Inc. commented@doidd, very nice find!
Manually tested this with ~13K terms by checking memory usage with and without the patch, and the results speak for themselves :)
HEAD: 600.7 MiB
Patch: 81.4 MiB
$entity_identifiers
could be a list of ID or revision IDs, so we can't pass it directly toresetCache()
. Instead, we should callresetCache()
without any argument in order to clear everything.Fixed in the patch attached.
Comment #9
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedOops, forgot to attach the patch.
Comment #10
catchI think the patch as it stands will result in clearing the entire persistent entity cache each time, see #2635440: Document what cache clearing from ContentEntityStorageBase::resetCache() actually clears. However it should now be possible to empty entity.memory_cache directly - see https://www.drupal.org/node/2973262
Another issue that would help with this is #1199866: Add an in-memory LRU cache.
Comment #11
amateescu CreditAttribution: amateescu for Pfizer, Inc. commented@catch, I was thinking that a generic
resetCache()
call would also take care of the revision cache (when we'll have it), but that will probably use the sameentity.memory_cache
service so we should be fine with a more targeted approach. I tested this new patch in the same scenario as #8 and got the same results, the memory leak is fixed.Comment #14
catchCommitted/pushed to 8.8.x, and cherry-picked to 8.7.x, thanks!
Comment #15
catchdouble post
Comment #16
catchI just realised that #3055443: Switch to a null backend for all caches for running the database updates (which I just committed), renders this issue a bit moot. However I think we should probably leave it as is, since the code could potentially be used outside an update (like a drush command or similar), and because I didn't backport the other patch to 8.7
Comment #17
catchDidn't mean to change the status, leaving fixed.