Problem/Motivation

I have request to skip unpublished content in linkchecker for entity types implementing EntityPublishedInterface. That might be a nice extension. I also noticed, that LinkExtractorBatch::processEntities() can be improved, which might have impact to performance.

Proposed resolution

I'd like to propose solution to:

  1. Skip checking links in unpublished content. If selected in linkchecker configuration, links in unpublished content will be ignored (with status 0).
  2. I see that by default batch size is set to 20. I'd like to propose to use this Settings::get('entity_update_batch_size', 50) to determine batch size. This is something which is used by core. And by default batch size will be 50. This should be applied on LinkExtractorBatch and LinkCleanUp.
  3. Improve performance of LinkExtractorBatch::processEntities() by loading all entities at once. So, changing:
    $storage = $this->entityTypeManager->getStorage($entityType->id());
    foreach ($ids as $id) {
      $entity = $storage->load($id);
      ...

    to:

    $entities = $this
      ->entityTypeManager
      ->getStorage($entityType->id())
      ->loadMultiple($ids);
    foreach ($entities as $entity) {
    ...
  4. LinkExtractorBatch::processEntities uses SQL LEFT JOIN, which is expensive query. And this function is called every cron run. So it’s good to check first if there is anything to process (which is fast). And call query with LEFT JOIN only if you know that there is data to process (because it’s slow). So at the beginning of mentioned function add this:
    public function processEntities($numberOfItems = NULL) {
      $numberOfProcessedItems = 0;
      // This function is used in batch to extract all links on demand and it's
      // also called on every cron run (see linkchecker_cron()). Because it uses
      // SQL LEFT JOIN, it's quite expensive. So, first check if there is anything
      // to process. If yes, then use query with LEFT JOIN to retrieve entities to
      // be processed.
      if ($this->getTotalEntitiesToProcess() <= $this->getNumberOfProcessedEntities()) {
        return $numberOfProcessedItems;
      }
    
    // Call SQL LEFT JOIN and process entities
    ...
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:

Comments

gugalamaciek created an issue. See original summary.

gugalamaciek’s picture

Proposed patch adds feature described in point 1. and implements performance improvements described in 2. - 4.

gugalamaciek’s picture

Issue summary: View changes
Status: Active » Needs review
gugalamaciek’s picture

Issue summary: View changes
joseph.olstad’s picture

Status: Needs review » Active

patch 27 didn't work out, working on a new one.

joseph.olstad’s picture

Status: Active » Needs review

Sorry wrong issue

joseph.olstad’s picture

Hi Gugalamaciek, I'm submitting a patch to re-use our setting that we had for another patch , but using your logic as ours was not working correctly but the setting I don't want to lose it and have to do more work with configs that are already ignored.

joseph.olstad’s picture

not sure if patch #7 is good, if you're testing, try patch 2 instead. I'm testing #7

joseph.olstad’s picture

Patch 7 works well, basically the same as patch 2 except I renamed the config setting key to work with our settings (yes selfish me).

gugalamaciek’s picture

@Joseph Fine for me :) That's why it's on review ;) I checked briefly, and I think you should change linkchecker.schema.yml as well.

As I see you've changed setting:

check.skip_unpublished

to:

search_published_contents_only

But when I check linkchecker.schema.yml, I have a feeling, that this property should be under check. So it should be rather check.search_published_contents_only. Also, if you want this change, I suggest to rename everywhere (e.g. form property as well). It will be less confusion for developers reading this code ;)

joseph.olstad’s picture

joseph.olstad’s picture

Thanks again gugalamaciek, we really appreciate your great work on this solution! It has been deployed into our production environment.

gugalamaciek’s picture

joseph.olstad I'm glad I could help :)
FYI #11 looks good, I've added it to our site :)

gugalamaciek’s picture

mahesh bandhiya’s picture

#11 tested and works fine, thanks joseph.olstad and gugalamaciek

mahesh bandhiya’s picture

Refined patch #11

Changes :

changed search_published_contents_only to search_published_content_only

changed Search published contents only to Search published content only

joelpittet’s picture

Status: Needs review » Reviewed & tested by the community

It would be nice to have these 2 features separate but the second part is exactly what I need as the unpublished nodes are outdated content in a lot of cases.

Anyways RTBC #17 with the text changes.

joseph.olstad’s picture

Assigned: gugalamaciek » Unassigned

***EDIT***
Double checking
***END EDIT***

rp7’s picture

As I already mentioned by others, perhaps we should idd look into separating the 2 features.
Skipping unpublished content could use/benefit from the solution being worked on in #3454641: Allow entities to be skipped programmatically.

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

vladimiraus’s picture

Status: Reviewed & tested by the community » Fixed

Thank you! Committed! 🍨

Status: Fixed » Closed (fixed)

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