Problem/Motivation

Drupal\Core\Cache\MemoryBackend::garbageCollection is a no-op.

When testing a long-running drush process that creates 100s of thousands entities we stumbled upon a memory issue. RAM consumption was growing with every iteration. This was not unusual. It's a single process and all the created entities were landing in the entity cache. Easy-peasy, let's just clear the entity cache for the unwanted entity types with $storage->resetCache(), right?

Not really. :) This had no effect on the memory consumption. MemoryBackend::resetCache only updates the expires property and removal should be done in ::garbageCollection, which looks like this:

public function garbageCollection() {
}

Steps to reproduce

  1. Load an entity
  2. Call $storage->resetCache()
  3. Run \Drupal::service('entity.memory_cache')->garbageCollection();
  4. Get the cache item with $allow_invalid set to TRUE

Proposed resolution

Remove invalid cache items when ->garbageCollection() is invoked.

Remaining tasks

None.

User interface changes

None.

Introduced terminology

None.

API changes

None.

Data model changes

None.

Release notes snippet

Drupal\Core\Cache\MemoryBackend::garbageCollection() removes invalid cache items from memory.

Issue fork drupal-3480025

Command icon 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:

About issue forks

Comments

blazey created an issue. See original summary.

blazey’s picture

blazey commented
Issue summary: View changes
blazey’s picture

blazey commented
Issue summary: View changes

blazey opened merge request !9815

blazey’s picture

blazey commented
Version: 11.0.x-dev » 11.x-dev

blazey closed merge request !9815

blazey opened merge request !9816

blazey’s picture

blazey commented
Assigned: blazey » Unassigned
Issue summary: View changes
Status: Active » Needs review

Added the method body.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs tests

Thanks for reporting!

Will most likely need a test case showing this bug

Nice issue summary!

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented

Thanks for this, this is very helpful!

I have run into this issue lots of times and have come into the habit of copy-pasting the following stanza to various services in various projects for exactly this purpose:

  protected static function reclaimMemory(): void {
    $memory_cache = \Drupal::service('entity.memory_cache');
    assert($memory_cache instanceof MemoryCacheInterface);
    $memory_cache->deleteAll();
    gc_collect_cycles();
  }

I never figured out how to properly solve this issue, though, great work on that! Not sure how to write a test for this, as the concrete memory usage at any given point is always a bit unpredictable, but maybe someone will have an idea. Definitely bookmarking this and will give this a spin when I get a chance.

dcam made their first commit to this issue’s fork.

dcam’s picture

dcam commented
Issue tags: -Needs tests

I added a test.

dcam’s picture

dcam commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented
Issue tags: +Needs change record

Test coverage appears to be there https://git.drupalcode.org/issue/drupal-3480025/-/jobs/7435887

Since we are touching memory can we add a simple CR? May be overkill but think it would be nice.

dcam’s picture

dcam commented
Issue tags: -Needs change record

I created the change record at https://www.drupal.org/node/3559908. In this case, there isn't anything that implementing code needs to change, so I didn't include before and after examples.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Thanks for taking care of that!

catch’s picture

catch
he/him
Primary language English
commented
Title: There is no way to remove entity cache items » Implement ::garbageCollection in the memory backend

The problem in the issue summary should already be fixed by #3498154: Use LRU Cache for static entity cache in the sense that there's a cap on the number of items in the entity memory cache now, so manual memory management shouldn't be necessary.

However implementing this method seems fine, didn't review the MR yet.

  • catch committed 04880eac on 11.3.x 
    fix: #3480025 Implement ::garbageCollection in the memory backend

By:...

  • catch committed e91a9a56 on 11.x 
    fix: #3480025 Implement ::garbageCollection in the memory backend

By:...
catch’s picture

catch
he/him
Primary language English
commented
Version: 11.x-dev » 11.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x and cherry-picked to 11.3.x, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

catch closed merge request !9816

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.