Recursive loading of relationships causes memory errors on save

Created on 20 April 2023, over 1 year ago
Updated 22 February 2024, 9 months ago

Problem/Motivation

On content that has many relationships, saving the content entity may cause out of memory errors. This is due to the gatsby module recursively adding relationship json, while also having the related entity types trigger a build, AND using inline entity forms to edit those related entities.

Steps to reproduce

Note: We use storage entities with nodes, but this can be reproduced with any related entity type.

  1. Configure gatsby module to trigger builds for nodes and storage entities.
  2. Create storage entity bundles with fields that may or may not relate to additional entities.
  3. Create a content type with a field to relate an unlimited number of storage entities.
  4. Configure the field to use Inline Entity Forms, so that related storage entities can be edited directly on the node.
  5. Create a node and add several storage entities, including some with relationships to other entities.
  6. Eventually there will be enough related entities to cause memory errors. In order to reproduce, it may be necessary to continue adding + editing storage entities.

This is because the node save triggers gatsby, which recurses through the relationships, AND when the storage entities are created or edited at the same time, those are also saved, which also triggers gatsby. Each entity that is saved has multiple relationships, and eventually there are so many entities to process in the one request that Drupal will run out of memory.

Proposed resolution

A quick fix is to add a configuration option to choose whether or not to recurse through the relationships. We do need to trigger the builds for the storage entities, as any given storage entity may be related to one or more nodes, and also may be saved outside the context of the related nodes. Regardless of where or how it's edited or saved, it should be updated in Gatsby, and all Gatsby pages that reference it (nodes in Drupal in this case) should be rebuilt.

Remaining tasks

Patch incoming

User interface changes

One additional configuration option, to allow administrators the option to disable recursion of relationships. It will default to the current state to prevent breaking backwards compatibility.

πŸ› Bug report
Status

Needs review

Version

2.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States lisagodare@gmail.com

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @lisagodare@gmail.com
  • πŸ‡ΊπŸ‡ΈUnited States lisagodare@gmail.com

    First pass at a patch to add the config option

  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 8
    last update over 1 year ago
    Build Successful
  • Open on Drupal.org β†’
    Core: 9.5.5 + Environment: PHP 7.4 & MySQL 5.7
    last update over 1 year ago
    Waiting for branch to pass
  • πŸ‡ΊπŸ‡ΈUnited States lisagodare@gmail.com

    Re-roll to work with patch in https://www.drupal.org/project/gatsby/issues/3356434 πŸ› Large number of log entries leads to memory/timeout issues Fixed

  • First commit to issue fork.
  • Open on Drupal.org β†’
    Core: 9.5.5 + Environment: PHP 7.4 & MySQL 5.7
    last update over 1 year ago
    Waiting for branch to pass
  • @slydevil opened merge request.
  • πŸ‡¨πŸ‡¦Canada slydevil Halifax
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update over 1 year ago
    Build Successful
  • πŸ‡¨πŸ‡¦Canada slydevil Halifax

    I've tested out this patch in hopes that it fixed the issue I'm encountering, but it does not. My issue is saving an Entityqueue in Draft causes a memory error:

    Fatal error: Allowed memory size of 536870912 bytes exhausted (tried to allocate 20480 bytes) in /code/web/core/modules/jsonapi/src/Normalizer/NormalizerBase.php on line 43

    I'll keep trying with this issue, but I may spin up a new one if it doesn't look to be related.

  • πŸ‡ΊπŸ‡ΈUnited States apmsooner

    There is this setting: Do not store data for entity types that are self referenced available. I don't know if you all have tried that or not. Memory issues sound related to this as it could point to an infinite loop of relationships possibly.

  • πŸ‡¨πŸ‡¦Canada slydevil Halifax

    I do have that checked @apmsooner. I've recommended to the team that we don't version entity queues for the short term. I'll create another issue to look into it.

  • πŸ‡ΊπŸ‡ΈUnited States lisagodare@gmail.com

    That setting doesn't help us, as the directly related entities aren't the same type/bundle, and this common scenario for us:

    • Have a Node related to multiple Storage entities.
    • Have those Storage entities related to multiple Nodes, of the same or different bundle, as they are reusable components.
    • Have a need to trigger Gatsby builds when Storage entities are saved, as content editors will also edit Storage entities directly (not just through Node edit forms).
    • Use Inline Entity Forms on the node bundle, so that content editors can add/edit Storage entities directly on the Node edit form.

    This means when a Storage is edited on the Node form, and the node is saved:

    1. The Node save triggers Gatsby, which recurses through all the relationships of the Node.
    2. Inline Entity Forms triggers a save on the edited Storage entity, which triggers Gatsby, recursing through all the relationships of that Storage entity.
    3. With the option Do not store data for entity types that are self referenced checked:
      • #1 does not recurse through Nodes of the same bundle. It does recurse through other relationships of the related Storage entities, which may include Nodes of a different bundle - and all of their relationships.
      • #2 is a Storage entity that trigged Gatsby, so it gathers all of the relationships of that Storage entity - which will include the original Node, and not exclude any other Nodes of that bundle, so there may be many. And each of those nodes may be related to 0 or more additional Storage entities, of a variety of bundles. Which may also be related to other Nodes, of the same bundle as the original.

    We do not need the recursive relationships in this scenario. If we do not recurse through them, the Node that was edited will have one level of relationship, which will include the Storage that was edited. The Storage that was edited and saved at the same time will have one level of relationship, which will include all other Nodes that reference this Storage. That's all we need updated in the fastbuild, and does not cause memory errors.

  • Enabling this patch has brought some insight into a @todo comment in GatsbyEntityLogger.php.

    // @todo Document why this is necessary for this entity type but not others.
        if ($action !== 'delete') {
          if ($entity->getEntityTypeId() === 'paragraph' && $this->config->get('recursive_relationships')) {
            return;
          } ...
    

    With this patched option enabled, paragraphs are completely siloed from being included in fastbuilds. I believe the assumption was they'd be captured in recursive crawl of relationships of parent nodes. Which makes sense to reduce overall gatsby logs.

    I think this check should be enhanced a bit to at least check the new config option is set, and maybe also that paragraphs are explicitly set as an entity to pass to build/previews. Updating that legacy @todo comment would be good too.

  • πŸ‡ΊπŸ‡ΈUnited States kamkejj WI

    Patch to add $this->config->get('recursive_relationships' for

    if ($entity->getEntityTypeId() === 'paragraph' && $this->config->get('recursive_relationships')) {
    

    per comment #11 πŸ› Recursive loading of relationships causes memory errors on save Active

  • πŸ‡ΊπŸ‡ΈUnited States ben.hamelin Adirondack Mountains, NY

    Thanks for this work! We were seeing some PHP errors on occasion:
    TypeError: Drupal\gatsby\GatsbyEntityLogger::getJson(): Argument #3 ($recursive) must be of type bool, null given, called in /var/www/html/web/modules/contrib/gatsby/src/GatsbyEntityLogger.php on line 82 in Drupal\gatsby\GatsbyEntityLogger->getJson() (line 247 of /var/www/html/web/modules/contrib/gatsby/src/GatsbyEntityLogger.php).

    Added a test to resolve that here.

  • Status changed to Needs review 9 months ago
  • Open on Drupal.org β†’
    Core: 9.5.5 + Environment: PHP 7.4 & MySQL 5.7
    last update 9 months ago
    Waiting for branch to pass
  • Open on Drupal.org β†’
    Core: 9.5.5 + Environment: PHP 7.4 & MySQL 5.7
    last update 9 months ago
    Waiting for branch to pass
  • πŸ‡ΊπŸ‡ΈUnited States ben.hamelin Adirondack Mountains, NY
Production build 0.71.5 2024