This issue has been introduced in #3622264: Avoid expensive operations while client waits for response and discovered by @scotwith1t in #3581562-14: Add option to disable Linkchecker (esp. entity hooks):
Drush calls kernel terminate once, when the command ends. Every fieldable entity saved during the run is queued in
LinkExtractorService::$entitiesandLinkCleanUp::$entityStack, and the extraction runs after the last save. Resetting the entity memory cache in the script does not free them; the two queues hold their own references.Measured on a site with 332k rows in
linkchecker_link: 200 published nodes with 5 links each, saved in onedrush php:scriptrun, entity caches reset every 50 saves, the site's other per-save side effects paused.
MR !156 applied, switch off (the HEAD code path) MR !156 applied, switch on The 200 saves 2.4 s 2.3 s Entities held until terminate 400 (each node and its content_moderation_state entity) 0 Memory after the saves and cache resets 74 MB 34 MB Work run at kernel terminate 272.8 s 0 So the command looks finished after 2.4 s and the process then runs for another four and a half minutes (each per-link existence query is a full-table scan on this site, 80 to 100 ms, see #3607810). Memory grows by roughly 200 KB per saved node with nothing able to reclaim it. A 30k-node import would hold roughly 6 GB at that rate; with any normal memory_limit the process dies first, and everything still queued is lost. Entities that already had an index row are then not revisited by cron, because
LinkExtractorBatch::processEntities()only selects entities with no row in linkchecker_index.
Maybe we need to revert parts of #3622264: Avoid expensive operations while client waits for response when running in batch mode.
None.
None.
None.
None.
Show commandsStart 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:
changes, plain diff MR !181changes, plain diff MR !180
Comments
Comment #2
claudiu.cristeaComment #4
claudiu.cristeaThis is ready for review
Comment #5
claudiu.cristeaIt would be nice if @scotwith1t can run the benchmark tests from #3581562-14: Add option to disable Linkchecker (esp. entity hooks) against this MR
Comment #8
joelpittetThanks @claudiu.cristea, and thanks for all the time you've put into this queue lately. I know your time is limited, so I've tried to keep this focused.
I opened MR 181 from MR 180 so your commits are preserved if I've taken this in the wrong direction.
1. Deleting an entity throws at terminate
doCleanupForDeletedEntities()reuses thelinkchecker_indexcondition group in thelinkcheckerlinkentity query, causing:QueryException: '("entity_type" = …) AND ("entity_id" IN (…))' not foundThat leaves the deleted entity's links behind and skips the rest of
destruct(). Building a separate condition group usingparent_entity_type_id/parent_entity_idfixes it.I added a kernel test that deletes a node and terminates the kernel. It also creates an
entity_testwith the same ID to verify only the node's records are removed.2. Queue IDs instead of entities
I think this is the main memory fix. The queues currently retain full entity objects until terminate, so long Drush imports keep every saved entity in memory.
MR 181 instead queues IDs, then loads and processes each entity in
destruct()and resets its storage cache afterward. This keeps #3626994: Queued entities of different types with the same ID overwrite each other's per-type separation.Saving 2000 nodes with 5 links each:
With IDs, memory stays flat as saves increase.
One small behaviour change: the old
array_shift()loop could process entities added duringdestruct(); the newforeachdoes not. Sincelinkcheckerlinksaves are no longer queued, I don't think anything realistic depends on that.Possible follow-ups, but not needed here:
loadMultiple().@scotwith1t, if you have a chance to rerun your #3581562-14: Add option to disable Linkchecker (esp. entity hooks) benchmark against this approach, that would be helpful.
Comment #9
claudiu.cristeaAssigning to fix the remarks
Comment #10
claudiu.cristeaComment #11
claudiu.cristeaLet's get a review
Comment #13
joelpittetWe made some headway here!
Queueing by ID is the big win.
Both MR versions hold about 16 MB where 2.1.x holds 115–134 MB. Terminate peaks at 22–40 MB instead of about 500 MB, and runs 2–5× faster.
Thanks for collaborating on this with me @claudiu.cristea! We can iterate on this I am sure.