I ended up in a situation where Entity B needs to wait until Entity A is indexed before it updates its values. So I have put Entity B to be updated when "SearchApiEvents::ITEMS_INDEXED" event triggers, but it further does not index the Entity B due to late scheduling as the indexing is already in progress.

Going to submit a patch for this...

CommentFileSizeAuthor
#5 3098350-5--post_request_indexing_recursion.patch1011 bytesdrunken monkey
#5 3098350-5--post_request_indexing_recursion--interdiff.txt1.03 KBdrunken monkey
#3 3098350-3--post_request_indexing_recursion.patch1.02 KBdrunken monkey
#3 3098350-3--post_request_indexing_recursion--interdiff.txt1.18 KBdrunken monkey
#2 3098350.patch738 bytesabrar_arshad

Comments

abrar_arshad created an issue. See original summary.

abrar_arshad’s picture

abrar_arshad commented
StatusFileSize
new 3098350.patch738 bytes

Here is the patch. Just re-checking the operations variable for any item and if so, calling the destruct() again

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Component: General code » Framework
Category: Bug report » Feature request
Status: Active » Needs review
StatusFileSize
new 3098350-3--post_request_indexing_recursion--interdiff.txt1.18 KB
new 3098350-3--post_request_indexing_recursion.patch1.02 KB

Thanks for reporting this problem and already providing a patch!
Please set the status to “Needs review”, though, when posting a patch that works for you. This makes it easier to spot issues that already have patches.

In any case, while the patch looks simple enough, I had to clean up the code style a bit (see the Drupal coding standards) and also made sure that this won’t lead to an infinite recursion.
Please test/review!

abrar_arshad’s picture

abrar_arshad commented

Noted. Thanks for reviewing it. I will soon review the patch but it looks good. Just one concern, as there could be potentially other cases where iteration could go beyond one-time recursion. Technically the limitation still exists, in my opinion.

- There could be a global setting to adjust the recursiveness
- Or, allow at least 5 recursions in the acknowledgement of the limit with the acceptable default behaviour as apposed to one-time recursion.

What do you say?

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
StatusFileSize
new 3098350-5--post_request_indexing_recursion--interdiff.txt1.03 KB
new 3098350-5--post_request_indexing_recursion.patch1011 bytes

I would say that this scenario is already extremely unlikely. I’m not aware of any code in any project I know that triggers indexing based on an indexing operation, and doing this more than once seems pretty absurd. Does this actually happen in your use case, or is this a hypothetical?
It’s also worth noting that not recursing when needed doesn’t really break anything – indexing will just wait until the next cron run instead of happening immediately.
So, for the sake of simplicity, I think I’d prefer recursing just once. But if there’s a reasonable use case for this, I’m also open to recursing more often (change is simple – patch attached).

Would also be interesting to hear other’s input on this.

abrar_arshad’s picture

abrar_arshad commented

Thanks and apologize for delay. This patch is good.

I have another sort of a related concern pls - already provided a patch: https://www.drupal.org/project/search_api/issues/3108219.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Needs review » Fixed

Thanks for reporting back, and no worries about the short delay – as you see, I’m the last who should complain about that. ;)
Great to hear the patch still works for you. Then: committed.
Thanks again!

abrar_arshad’s picture

abrar_arshad commented

Thanks for committing.

Status: Fixed » Closed (fixed)

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