Using the latest drush version (9.6-rc3) the indexing command consumes too much memory and fails if there is no memory left.
Reason for this is that in drush 9.6 the support for spawning a new process when memory limit is reached was removed - https://github.com/drush-ops/drush/pull/3909
I also create a PR for drush for this issue to revert this commit, but I'm not sure if this will go through or not - https://github.com/drush-ops/drush/pull/3952
The real issue for the high memory consumption while indexing can't be solved by search api, I think the following issue on d.o would be really helpful in this case - #2577417: Add an entity iterator to load entities in chunks
In the meantime while indexing the static caches could be cleared manually to not run in the high memory consumption in the first place.
Setting to major as if user are updating to 9.6, this will break sapi-i if you have a lot of entities to index and a low max memory.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | 3036504-8--drush_batch_indexing_clear_static_cache.patch | 1.36 KB | drunken monkey |
| #2 | 3036504-1.patch | 1.39 KB | kfritsche |
Comments
Comment #2
kfritscheHere something work in progress, because I'm not sure how to continue best here.
The patch only help if using Drupal 8.6, as the only way to do it for 8.5 would be calling resetCaches, which also clears persistent caches - see comment in my patch. Not sure how this should be handled in search_api as 8.5 is still supported till April(?)
The other issue with this patch is that it uses the 'entity.memory_cache' service. In theory a entity storage could use a different service, but the used memory cache is protected by the storage and it can't be retrieved.
The deleteAll is fine in the context of the indexing process. As only entities which will be indexed are loaded, so there is no real need to only invalidate the entries from the entity types of the index. There are only entities from the index in it anyway. If something like this is wanted than the other problem is that also the memoryCacheTag is protected inside the entity storage. So a more pretty solution would require patching d.o to add a resetStaticCache method or similar. Or like mentioned above better to work on #2577417: Add an entity iterator to load entities in chunks
Comment #3
kfritscheComment #4
kfritsche> [notice] Message: Successfully indexed 7179 items.
>
> [notice] Message: Successfully indexed 30540 items.
>
> [notice] Message: Successfully indexed 36493 items.
>
> [notice] Message: Successfully indexed 32133 items.
>
> [notice] Message: Successfully indexed 106 items.
>
> [notice] Message: Successfully indexed 7067 items.
With the applied patch the batch process never used more than 100MB memory, but was a bit slower because we use the same entities for some indexes (with different settings) and in this case it needed to load them from the DB again.
With drush 9.5 oder patched 9.6 it needed to respawn the batch process like 10 times because it exceeded 500MB memory usage (we limited drush to 1GB)
Comment #5
drunken monkeyThanks a lot for reporting this, and for pointing me to that Core issue – does seem interesting in any case!
However, it seems the Drush change was reverted anyways (thank God!) so I guess this is resolved?
Comment #6
kfritscheThanks to the Drush Team, we could quickly resolve this and re-integrate this functionality.
As the provided patch has a lot of todos, where I'm not sure how to improve it I agree to dump this for now.
I'm not sure if you maybe want to create a follow up issue or "convert" this issue to implement #2577417: Add an entity iterator to load entities in chunks as soon as it fixed. So more or less as an example issue to keep open, so people see there why this functionality is really needed in core.
Comment #7
jonas139 commentedI'm currently running drupal 8.7.9 with drush 9.7.1 and I'm having the same issue.
When I run
drush sapi-i search_indexmy batch-process is consuming all my memory and the indexing stops.If I apply the patch above, this isn't the case anymore and I can run the entire index process AND it's a lot faster (thanks kfritsche).
My question is, is the patch still applicable or is there another solution for this issue where I'm not aware of?
And again, thanks for the patch!
Comment #8
drunken monkey@ jonas139: Thanks for reporting this problem!
However, from the looks of the code, it seems to me like you’d need to set a PHP memory limit (for CLI) for the built-in Drush workaround to have any effect. Is it possible you disabled that memory limit for CLI? Then maybe try with a reasonable limit set and see if that already helps (without this patch).
Otherwise, if this is really due to another Drush change or something like that, I believe the attached patch should also solve this, and with a bit more updated code (also only running when inside Drush). Please give that a try as well! (No interdiff since I completely moved the code to a different location, so the interdiff would be useless.)
@ kfritsche: Do you know anything about this, could the change have crept in again? (Didn’t see anything like it from a quick look in the Drush repo, but not sure.)
Comment #9
jonas139 commentedHi @drunken monkey,
You're right... I didn't set any limit for CLI.
If I set a limit (128M) and I start indexing, it will create another batch when it reaches it's memory limit and still keeps indexing BUT it still uses too much memory and crashes (even with your patch).
Allowed memory size of 134217728 bytes exhausted (tried to allocate 16384 bytes) in core/lib/Drupal/Core/Cache/DatabaseBackend.php on line 167I'm starting to think this is only an issue for my environment so I'll keep testing and post my findings here.
Comment #10
bendev commentedI had the same issue
I tested patch 8 successfully and it solves the memory issue
AFAIC this can be committed.
Thank you
Comment #11
drunken monkeyHm, OK, I guess it can’t really hurt. It shouldn’t break anything, and only actually trigger in this specific case (during a Drush batch), and if it helps a few people avoid errors – why not?
Committed.
Thanks again, everyone!
Comment #13
drunken monkey