We've noticed incredibly frequent rebuilds of data in cache_discovery for a site I'm working on with a lot of editor traffic. In tracing it back, we realized that \Drupal::entityTypeManager()->clearCachedDefinitions();
was being called on every authenticated request that built a node edit/create form. Digging deeper, we discovered this came from this module, in the xmlsitemap_get_link_info()
. This function retrieves information about the various entity types and bundles that could be added to the sitemap. This data is cached, because it's expensive to build. There is also an escape hatch on the function - a $reset parameter that causes the cache to be rebuilt. The problem is twofold:
xmlsitemap_add_form_link_options()
- the function that adds the XML Sitemap settings to an entity form, callsxmlsitemap_get_link_info()
with $reset = TRUE. This causes the info cache to be rebuilt anytime an editor hits the node form.- Using the $reset = TRUE parameter actually causes a call to
\Drupal::entityTypeManager()->clearCachedDefinitions();
, causing a lot of extra data to get cleared out of cache_discovery.
Steps to reproduce
- Install XML Sitemap and enable it for one bundle of content entities.
- Note the "created" time for the entity_field_map cache entry in the "cache_discovery" table.
- Go to a node edit/create form for a node of the bundle you enabled earlier.
- Observe that the 'created' timestamp for the 'entity_field_map' cache entry has been updated.
Comment | File | Size | Author |
---|---|---|---|
#5 | 2924422-cache-clear-abuse-5.patch | 913 bytes | dpi |
|
Comments
Comment #2
rbayliss CreditAttribution: rbayliss at Last Call Media commentedMinimal patch to fix the behavior.
Comment #3
rbayliss CreditAttribution: rbayliss at Last Call Media commentedI'm not 100% sure what edge case the $reset = true and clearing of cached definitions was added for, but I'd argue that those things should be handled elsewhere. In my testing, I wasn't able to replicate a situation where the info wasn't updated following a change to the XML Sitemap settings, or to the bundles themselves.
I'm bumping this to critical, because the scope of the data that gets cleared here is massive - virtually every subsystem in Drupal 8 depends on those entity type definitions being available, so on a site with a lot of editor traffic, this is going to have a massive impact.
Comment #4
rbayliss CreditAttribution: rbayliss at Last Call Media commentedFor those running 8.1-alpha2, here is a patch that can be applied. This version of the patch isn't the one that should be committed though. See comment #2 for that version.
Comment #5
dpiFast forward, work on
79cfd857
.No other changes.
This is a really horrible bug.
clearCachedDefinitions
shouldnt be abused like this.+1 on critical state
Comment #6
jibranThanks, this looks ready to me.
Comment #7
renatogReally works good. +1
Comment #9
renatogCommitted to the dev branch.
Best,