Problem/Motivation
When dealing with background jobs that for example export 10k nodes to an XML file with Views, Metatag queries become very, very, very slow. MySQL slow query log:
SELECT entity_id, data, language FROM metatag WHERE (entity_type = 'node') AND (entity_id IN ('529', .... 10k entries here ...)) AND revision_id IN ('529', .... 10k entries here ...)
# Query_time: 192.328458 Lock_time: 0.011021 Rows_sent: 4 Rows_examined: 271 Rows_affected: 0 Rows_read: 271
192 seconds, not bad! Of course this makes MySQL jump to 100% CPU usage and block lots of requests, effectively taking the site out.
Unfortunately I have no idea how you would optimize an IN query with 10k entries for MySQL.
Proposed resolution
Define an upper limit of entities where Metatag hooks in to avoid MySQL crashes. 500 entities should be enough, larger entity loads are probably background jobs like mine without an interest in metatags.
Remaining tasks
Review patch.
User interface changes
none.
API changes
none.
Data model changes
none.
Comments
Comment #2
klausiPatch.
Comment #4
damienmckennaThis would need to be a variable so that it could be customized per site.
Also, are you using entitycache? If so, I'd be concerned at possible unintended consequences from doing this, you'd probably what to clear the cache for those entities post-load to ensure that when the entities were loaded normally any overridden meta tags would still be loaded.
Comment #5
klausiI'm not using entitycache, so the metatags are only missing in the static cache of the drush queue worker, while all other requests get them from the DB.
But yeah, with this patch you will want to disable entitycache in any larger queue worker you have to not pollute the cache with incomplete entities.
hook_entity_load() is very dangerous performance wise, but I guess you heard that already a couple of times in the metatag issue queue :-P
Comment #6
dave reidCore establishes using hook_node/entity_load() for loading data that will not change unless the node is edited/saved. Contrib should be using it. FYI any queue worker I've written will load entities in batches of 10-100. Any more than that and I've found it's unreliable due to core itself, not even taking into consideration any contrib modules.
Comment #7
damienmckennaFurther, the onus is on the developer building the queries to adjust as necessary when you have to deal with large batches of data.
Comment #8
dave reidI'd get the same problem if I made a View that listed 500+ rendered entities. But I wouldn't do that, I would add limits and pagers. The same principal applies here.
Comment #9
klausiIf the rendering is fast enough like in views_data_export in a background job then it is perfectly fine to load 5000+ entities. The only problem I had was metatag_entity_load(), and I'm fine with hacking it with this small patch.
Yes, ideally we should fix both: using batches for processing large data sets and replacing metatag_entity_load() by using Field API or loading the stuff only when it is actually needed.
Comment #10
damienmckennaMetatag doesn't use Field API. End of story.
Comment #11
klausiThat's fine, it can also load its own stuff on demand instead of in hook_entity_load(). But yeah, you would have to open a new branch for that because people might now rely on the fact that metatag stuff is magically there when they load their entities.
Comment #12
klausiStill using this patch, rerolled against the latest metatag version.
Comment #13
damienmckennaIf you change this to work off a variable that defaults to ignore the option, I'd be willing to include this as an advanced option.
Comment #14
damienmckennaAlso please add an item for the variable to the settings page and include a note in the field's description that it shouldn't be used with EntityCache. Thanks.
Comment #15
lazysoundsystem commentedFor what it's worth, https://www.drupal.org/node/2835614 has made this query much much faster.
Comment #16
bluegeek9 commented