Problem/Motivation
SqlContentEntityStorage::loadMultipleCardinalityFields() loads multiple dedicated multi-cardinality fields by joining their field tables into a single query.
When an entity has multiple multi-value fields, this produces a Cartesian product between the field table rows.
This can cause memory overflow issues when ever an entity that has multiple multi-value fields is loaded.
For example, if an entity has:
field_a: 5 values
field_b: 10 values
field_c: 4 values
The query can return: 5 * 10 * 4 = 200 rows
Steps to reproduce
Create a content entity type/bundle with several dedicated multi-value fields. For example, a node bundle with multiple unlimited entity reference or paragraph reference fields.
Add content where one entity has several values in each of those fields.
Example:
field_a: 5 values
field_b: 10 values
field_c: 4 values
Load the entity with entity storage:
$node = \Drupal::entityTypeManager()
->getStorage('node')
->load($nid);
Inspect the SQL query generated by SqlContentEntityStorage::loadMultipleCardinalityFields().
Proposed resolution
Change loadFromDedicatedTables() so that multiple-cardinality fields are loaded one field at a time instead of in chunks containing multiple multi-value fields.
Attached preliminary patch which bypasses the issue but this probably needs more work and tests.
Remaining tasks
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | drupal-core-load-multiple-cardinality-fields-separately-6.patch | 1.36 KB | jviitamaki |
Issue fork drupal-3607938
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
jviitamaki commentedComment #3
cilefen commentedI lowered this to major priority. I don't think it meets the criteria for critical, but we can discuss that.
Comment #4
thefancywizard commentedMy team caught this locally while testing an 11.4 upgrade before deploying to production. A node with 8 populated unlimited-cardinality fields produced a 14×13×11×8×7×6×3×2 = 4M row query, taking 134 seconds to load vs ~235ms on 11.3. This caused 502s and 800% CPU on our search pages. We would not have been able to deploy this upgrade.
Any bundle with several well-populated unlimited-cardinality fields will hit this. I'd argue this meets criteria for critical - it's a regression that causes outages with no workaround short of patching core.
The attached patch resolves it completely for us.
Comment #5
john.oltman commentedMost of our listings were unusable after updating to 11.4, so I've put this back to Critical, as unusable is one of the criteria for a Critical bug. Our database is nothing special, probably small to medium size (50k nodes).
Comment #6
jviitamaki commentedIn our case we have a node type which has 10 multi-value fields and for some nodes this causes 8,7M rows in a query. So yeah I too think this should stay critical.
Attached a new seemingly working preliminary patch to bypass the issue.
Comment #8
catchYeah agreed this is critical, fatal error on those pages.
I think the best thing to do here is the approach in #6 which I've turned into an MR.
We might be able to get the same query-reduction without the cartesian multiplication via a UNION query or similar, but that will take some time to get right.
Comment #10
catchPut an MR up.
Similar to the patch in #6, but switched to an INNER JOIN since we don't need a result if the one field we're checking has no rows. Also updated performance tests.
I'll also open a follow-up later to try to reintroduce loading multiple fields at once but without the cartesian multiplication.
Comment #11
kulturmensch commentedTested MR !16200 successfully on both a staging site and the corresponding production site.
Environment:
* Drupal 11.4.0
* PHP 8.5.7
* Drush 13.7.x
* MySQL/MariaDB
* Simple XML Sitemap 4.2.3
* Bilingual site with a content type containing several dedicated multi-value image and taxonomy fields
Before applying the MR:
* Sitemap generation reproducibly failed after processing exactly 290 entities.
* The failure occurred with PHP memory limits of 512 MB, 1 GB, and 2 GB at the same queue position.
* The stack trace ended in `SqlContentEntityStorage::loadMultipleCardinalityFields()` / `PdoTrait::execute()`.
* Skipping the first failing entity only moved the failure to another entity shortly afterwards.
* One affected bilingual node contained several populated multi-value fields whose theoretical Cartesian product was 19,595,520 rows per language.
* Another affected node produced a theoretical product of 518,400 rows per language.
* Increasing the memory limit therefore did not solve or meaningfully change the problem.
After applying MR !16200:
* The previously failing node loaded successfully in about 0.56 seconds.
* PHP peak memory while loading that node was about 55 MB.
* Staging sitemap generation completed 1,031 of 1,031 items in 28.58 seconds.
* Maximum resident set size on staging was 168,792 KB.
* Production sitemap generation completed 1,044 of 1,044 items in 28.55 seconds.
* Maximum resident set size on production was 167,844 KB.
* The queue was empty afterwards, no generation lock remained, and the generated sitemap returned HTTP 200.
* Drupal bootstrap, cache rebuilds, cron, and normal page requests continued to work correctly.
I also verified that the fix can be reapplied cleanly through `cweagans/composer-patches` after removing and reinstalling `drupal/core`.
This confirms the regression on a real-world site and confirms that the approach in MR !16200 resolves the memory exhaustion caused by Cartesian multiplication.
Comment #12
longwaveThis fix looks good to me. I did wonder about the sort loop following
loadMultipleCardinalityFields()and whether that can be removed, but it's not clear to me either way. Perhaps in a followup we can consider if the database can do the sorting here instead, because that might be more efficient?I also wondered about explicit test coverage but I think the performance test changes are likely good enough, although they won't necessarily prevent the same regression in the future this will be non-trivial to add actual coverage for.
Comment #13
catchOpened #3608184: Load multiple cardinality fields with a smaller result set for the follow-up.
Comment #23
godotislateCommitted and pushed b4c63e81 to main, c6b4102 to 11.x, and a2e4492 to 11.4.x. Thanks!
Comment #25
kulturmensch commentedFollow-up: I have now updated the production site from Drupal 11.4.0 with the temporary MR !16200 patch to Drupal 11.4.1 without the patch.
The previously affected sitemap generation completed successfully:
* 1,044 of 1,044 items processed
* Exit status: 0
* Elapsed time: 26.49 seconds
* Maximum resident set size: 168,248 KB
* Queue empty afterwards
* No generation locks remained
* Sitemap and front page both returned HTTP 200
This confirms that Drupal 11.4.1 contains and successfully applies the fix for this regression on the affected production site.
Comment #26
alexpottIt would be super awesome if @jviitamaki or @kulturmensch or @thefancywizard could test out the latest approach on #3608184: Load multiple cardinality fields with a smaller result set as the re-implements the change that caused this regression but without the unwanted side effects.