Problem/Motivation

We're using lovely Linkchecker in a large project and just had to run a batch update on many entities. We were wondering why it became extremely slow and saw MANY Linkchecker queries causing this.

I tried #3313343: Add option to disable cron activity, but then had to find out that it's not a cron thing, but Linkchecker also uses hook_entity_* a lot:


/**
 * Implements hook_cron().
 */
function linkchecker_cron() {
  if (\Drupal::config('linkchecker.settings')->get('check.disable_cron')) {
    return;
  }
  \Drupal::service('linkchecker.extractor_batch')->processEntities();
  \Drupal::service('linkchecker.checker')->queueLinks();
}

/**
 * Implements hook_entity_insert().
 */
function linkchecker_entity_insert(EntityInterface $entity) {
  if ($entity instanceof LinkCheckerLinkInterface) {
    return;
  }

  if ($entity instanceof FieldableEntityInterface && !$entity->isNew()) {
    /** @var \Drupal\linkchecker\LinkExtractorService $extractor */
    $extractor = \Drupal::service('linkchecker.extractor');
    $links = $extractor->extractFromEntity($entity);

    if (!empty($links)) {
      $extractor->saveLinkMultiple($links);
      $extractor->updateEntityExtractIndex($entity);
    }

    \Drupal::service('linkchecker.clean_up')->cleanUpForEntity($entity);
  }
}

/**
 * Implements hook_entity_update().
 */
function linkchecker_entity_update(EntityInterface $entity) {
  if ($entity instanceof LinkCheckerLinkInterface) {
    return;
  }

  if ($entity instanceof FieldableEntityInterface) {
    /** @var \Drupal\linkchecker\LinkExtractorService $extractor */
    $extractor = \Drupal::service('linkchecker.extractor');
    $links = $extractor->extractFromEntity($entity);

    if (!empty($links)) {
      $extractor->saveLinkMultiple($links);
      $extractor->updateEntityExtractIndex($entity);
    }

    \Drupal::service('linkchecker.clean_up')->cleanUpForEntity($entity);
  }
}

/**
 * Implements hook_entity_delete().
 */
function linkchecker_entity_delete(EntityInterface $entity) {
  if ($entity instanceof LinkCheckerLinkInterface) {
    \Drupal::service('linkchecker.clean_up')->cleanUpQueues($entity->id());
    return;
  }

  if ($entity instanceof FieldableEntityInterface) {
    \Drupal::service('linkchecker.clean_up')->cleanUpForEntity($entity);
  }
}

As Drupal 8+ has no "disable module" functionality, there's no way to disable it. Uninstalling removes configuration and all LinkChecker entities.

So it would be great to have a simple setting to disable linkchecker (for a while)!

Disabling link checker on cron run should still be a separate setting (#3313343: Add option to disable cron activity) as the use-cases for both are different.

Workaround

As a workaround I just commented out these hooks now, but that's not a nice user-facing solution. After doing that, the batch ran fast again!

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork linkchecker-3581562

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

anybody created an issue. See original summary.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Issue summary: View changes
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Title: Add option to disable Linkchecker » Add option to disable Linkchecker (entity hooks)
Issue tags: +Performance

Just came across this again and thought maybe we should think about the performance implications of the hook_entity_insert / *_update / *_delete again.

Our case showed that may have heavy performance implications on (mass) enttiy CRUD. So I think being able to disable them is crucial, but maybe there's even room for further improvements (in a follow-up)?

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Title: Add option to disable Linkchecker (entity hooks) » Add option to disable Linkchecker (esp. entity hooks)
Issue summary: View changes
tomefa’s picture

tomefa commented

Yes i agree also that the actual hook_entity_insert / *_update / *_delete are slowing down the website.
In some case with many link, it take more than 1 minute to save a content because of all the link extraction.

I could imagine a queue worker that will be filled with the insert/update entity id and extract the links in it later.
It doesn't need to be done right away.
For the delete action, i'm not sure.

We have patch just a bit the insert/update hook to limit to extraction to only node and media entities. Specific for our case, that already fix some performance and DB query issues because the it was run on entity that don't have any active linkchecker field (flag, group, user, etc...).
For example on a galera cluster, there were db locks issues because linkchecker was trying to check the flag entity even if there is no field that need to be checked.

falco010’s picture

falco010
Location 🇳🇱 The Netherlands
commented

+1 faced massive performance issues due to this module entity insert/update hooks (Some nodes can have a lot of links).

For now we created an internal patch which disable both hooks (would not advice this, but for us this is very much needed at this point). This was extra problematic for us as we have all our traffic going through CDN which has a strict timeout and therefore even caused 504 errors.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Priority: Normal » Major

Maybe someone could prepare a MR with such a setting?
Is a boolean option the best solution or are there other alternatives we should think about here? I like the idea of

I could imagine a queue worker that will be filled with the insert/update entity id and extract the links in it later.

in #5
So are we talking about modes here and how would this be self-explaining in the admin form? (UX)

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

scotwith1t opened merge request !156

scotwith1t’s picture

scotwith1t
Location Birmingham, AL
commented

Opened !156 with a first pass at this.

Answering the UX question in #7: went with a simple boolean checkbox ("Disable link extraction on every entity save") on the existing settings form, backed by state rather than config, since (in my mind, anyway) this is meant to be an operational, per-environment switch someone flips on for the duration of a specific run (a migration, a bulk import) rather than a deployable site setting -- storing it in config risked it accidentally traveling into an export/deploy from whichever environment it was last toggled on.

Only insert/update are gated; delete still runs its cleanup so removed entities don't leave orphaned tracked links behind.

Also opened #3614738 / !155 as a related, independent fix: even without this toggle, saveLink()/cleanUpForEntity() were doing roughly 2N individual queries per save for an entity with N links, which compounds the problem reported here. Worth a look together, but neither depends on the other.

c-logemann’s picture

c-logemann
he/him
Primary language German
Location Frankfurt/M, Germany
commented
Status: Active » Needs review

Thanks @scotwith1t for adding code we can review and discuss. This feature request sounds very needed. I hope we can fix this soon.

codebymikey’s picture

codebymikey commented
Status: Needs review » Needs work

I think this functionality can be implemented similarly to how Geocoder handled it in #3301512: Extend the ability to skip geocoding when processing a large number of entity updates, like migrations or workspace publishing

We can have a configuration/state as well as a request-level linkchecker_disabled attribute that's checked before doing any of those processing.

That way, those needing to do a batch run e.g. migrations/custom batch updates can temporarily disable it for the current request using event subscribers without touching their configs (which might be useful to still have legitimately enabled).

The MR also has merge conflicts.