In \Drupal\search_api\Entity\Index::setHasReindexed(), a call to \Drupal::state()->set() is made regardless of the current state. As a result, performing bulk operations on multiple indexed entities could result in a lot of state sets (seems like two per Node create/update), which is a complex operation which results in a lot of database operations.
My team is having performance issues as a result of this, and would prefer it if Search API only updated the state if the operation is necessary.
Check the existing state has_reindexed state before updating it, to avoid multiple unnecessary database writes while updating entities.
Review the provided patch.
None.
None.
None.
| Comment | File | Size | Author |
|---|---|---|---|
| search-api-avoid-state-set.patch | 576 bytes | samuel.mortenson |
Comments
Comment #2
borisson_This looks like a solid improvement and it passes all the tests, I don't think we need to have a new test for this either.
Comment #3
drunken monkeyWell, it's not as bad as the IS suggests, unless I'm mistaken:
setHasReindexed()is only called when the nodes are actually indexed, not when they are only updated and marked for indexing (unless I'm mistaken) – and in this case, I'm pretty sure the indexing operation will take far more time than the writes to the state variable. Also, I'm really not sure where the two writes would come from.Before committing this (and I guess it makes sense in any case – doing sometimes a read and a write is still better than always writing, unless the "only read" case is rare enough) it would be great to get to the bottom of this and make sure there isn't some other bug.
Can you confirm that you have "Index items immediately" enabled for the index? And could you find out from where
setHasReindexed()is called (twice) for you?Comment #4
samuel.mortensonYes, "Index items immediately" is enabled for my index. I tried to replicate the two re-indexes problem on a standard Drupal site and couldn't get something working.On the site I noticed this issue on, the callers of
setHasReindexedaresearch_api_entity_updateandsearch_api_node_access_records_alter. Maybe this occurs when entity access isn't cached? We use a lot of contrib modules so it's hard to track down.Comment #6
drunken monkeyThanks a lot for reporting back with this information!
It seems it's called twice because nodes will actually be indexed twice under these circumstances!
See #2922525: Index items only at end of page request for a proposed fix.
In any case, as said, I think this makes the problem a lot less critical: indexing takes quite some time and resources in any case, so having one or two database writes on top of that won't really matter one way or the other.
Still, as also said, no argument against improving performance a little bit.
Committed.
Thanks again, everyone!