Caches (views) are not invalidated on indexing objects.

\Drupal\search_api\Entity\Index::indexSpecificItems() already invalidates that cache tag. Do you know why that doesn’t happen in your case?
If you can reproduce this problem from a clean site install, with just the necessary modules, it would be great to get a (failing) regression test for it.

I confirm that cache tag invalidated twice, but the issue with caching could take a place. The cache tags are cleared as a last action in the method (generally as should be), but exception thrown during indexing or results post-processing in $dispatcher->dispatch(new ItemsIndexedEvent($this, $processed_ids), SearchApiEvents::ITEMS_INDEXED); leads that execution stopped and cache tags aren't invalidated.

In addition if "index_directly" option is enabled, indexing takes place after response was sent. In this way user doesn't see that any error was happened. It could be found in Drupal log, but if it is PHP error (out of memory/time limit), it could be found in server logs only.

In this way from user viewpoint everything should be done, but cache isn't cleared and outdated results are shown.

So, this is a question, should the cachetags be invalidated twice before and after indexing, or should try/catch be added (doesn't help on PHP errors), or it is and environment case and generally works as designed.

Hm, that’s a good point, seems like that might indeed happen. Of course, any fatal errors occurring during indexing should be investigated and resolved anyways, but it’s true that the search server state might have changed anyways in that case, so the caches must be invalidated nevertheless.

Patch attached, please test/review. (I’d still like to keep the cache invalidation in the same method.)

Could someone please test/review?

TBH at Index::indexSpecificItems(), I would simply wrap every line from the first one potentially throwing a SearchApiException into a try / finally statement, and move the cache tag invalidation into finally. (So, trusting your judgement, from the line where you add the extra cache tag invalidation now.):

/**
 * {@inheritdoc}
 */
public function indexSpecificItems(array $search_objects) {
  if (!$search_objects || $this->read_only) {
    return [];
  }
  ...
  // Remove all items still in $items from $rejected_ids. Thus, only the
  // rejected items' IDs are still contained in $ret, to later be returned
  // along with the successfully indexed ones.
  foreach ($items as $item_id => $item) {
    unset($rejected_ids[$item_id]);
  }

  try {
    ...
  }
  finally {
    // Clear search api list caches.
    Cache::invalidateTags($tags_to_invalidate);
  }

  // Clear the static entity cache, to avoid running out of memory when
  // indexing lots of items in one process (especially via Drush).
  \Drupal::getContainer()->get('entity.memory_cache')->deleteAll();

  return $processed_ids;
}

I think there is a good chance that 🐛 View recalculated with wrong data from cache Needs review is related to this issue (not because the possibility of the exceptions thrown during indexing, but because of the cache metadata).

Feel free to remove it from the relations if you disagree.

You’re right, using a finally makes more sense here. Invalidating a cache tag might not be super-cheap, so doing it twice is just wasteful.
Also, I guess this could need a test, which also isn’t that hard to write. So, added that, too.
Please give it another test/review!

Based on your description in 🐛 View recalculated with wrong data from cache Needs review , I don’t think it’s related, but we can still leave the reference in case it helps someone.

Every single time …

The last submitted patch, 12: 3294051-12--invalidate_cache_before_indexing--tests_only.patch, failed testing. View results →

Does anyone still want to test/review or is this ready to be merged?

Merged. Thanks again, everyone!

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