XXX items could not be indexed. Check logs for details.

Thanks for reporting this issue!
I see how this might happen. Unfortunately, with our current architecture it’s not really possible to tell how many items actually failed to be indexed as Index::indexItems() doesn’t return that information.
The easiest fix would probably be to just phrase that warning message more carefully – up to XXX items might not have been indexed, but it’s also possible everything was fine. (See the attached patch.) That might be a bit less confusing, but it’s of course still not very helpful.

You’re probably right that the correct solution here is to apply locking when indexing so that two separate processes don’t index items concurrently – especially since I think this will also sometimes mean that the same item is indexed more than once, which is just a waste of ressources. (In fact, this seems like it would be much more common than the problem reported here, but just not really noticeable for the user.)
However, this would be a bit tricky to implement. I’ll try to look into it, but would also be very grateful for any patches.

The last submitted patch, 2: 3367890-2--false_unindexed_items_warning.patch, failed testing. View results →

Fixing the test fail.

This would additionally implement a simple locking mechanism for indexing operations. (I think we only need to lock those for “index N items”, not those that index specific items, e.g., when saving an entity. However, this means that saving an entity while an indexing batch is in process might still lead to the described misleading behavior. I’d therefore say we should also change the warning message, as proposed.)

I guess this might still run into problems sometimes, when a cron job is executed right between two batch processing steps, but I don’t think it’s worth it to try and guard against that. (We’d essentially need to implement a lock that stays active across requests, which seems overly complicated.)

Please test/review!

The last submitted patch, 5: 3367890-5--false_unindexed_items_warning.patch, failed testing. View results →

Waiting for feedback before I try to fix the latest patch. First want to determine whether locking is even the right approach here.

Hello @drunken monkey,

Thank you for your work on this issue. Indeed, the situation has improved significantly with the patch. The wording of the message is clear, and the locking system has definitely resolved my issue. While I agree that the patch might not fix all possible cases, it's better than having no solution at all. The scenarios you mentioned are less common compared to this specific one, so I'm satisfied with this patch.

I do have one note regarding an improvement: When a SearchApiException is produced due to an available lock, the exception message is "Items are being indexed in a different process." However, in the IndexStatusForm::submitForm, the system catches this exception and only displays a warning message: "Failed to create a batch, please check the batch size and limit." I suggest aligning the warning message with the exception message for clearer communication to the users about what is happening.

Here is my test result:

Thanks a lot for your feedback, good to hear the patch worked for you and you agree with its direction.

I do have one note regarding an improvement: When a SearchApiException is produced due to an available lock, the exception message is "Items are being indexed in a different process." However, in the IndexStatusForm::submitForm, the system catches this exception and only displays a warning message: "Failed to create a batch, please check the batch size and limit." I suggest aligning the warning message with the exception message for clearer communication to the users about what is happening.

Also a very good find, thanks! In the attached revision I’m checking explicitly for the lock in submitForm() and display this warning if it’s not available:

Cannot start indexing because another indexing process (like a cron job or Drush command) is already running.

Does that sound good to you?

The patch should hopefully also resolve the test fails.

Hello @drunkey_monkey,

Thank you once again for your contribution. The rephrased error message is now much clearer. From an end-user's perspective, it's more understandable what is occurring. All tests are passing, so it looks good to me. I'm marking this issue as RTBC!

Thanks for reporting back, great to hear!
Merged. Thanks again!

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