Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
When entity_load
is called on an entity save, search_api_track_item_insert()
is called and causes duplicate entry because it has been already inserted by SearchApiEntityDataSourceController::startTracking
Comment | File | Size | Author |
---|---|---|---|
#13 | entity_defaults_rebuild_lazy.patch | 4.82 KB | fago |
#11 | entity_rebuild.patch | 1.41 KB | fago |
#1 | 1267070-duplicate-entry.patch | 607 bytes | jsacksick |
Comments
Comment #1
jsacksick CreditAttribution: jsacksick commentedWorkaround for this issue :
Force the Entity API module to rebuild the search indexes and servers in
hook_modules_enabled()
Spécial thanks to Damien Tournoud for this workaround.
Comment #2
jsacksick CreditAttribution: jsacksick commentedComment #3
drunken monkeyI don't understand at all what the problem here is. Please elaborate.
Comment #4
jsacksick CreditAttribution: jsacksick commentedHere is a little explanation of what's happening in my case :
- I create a node during the installation
- I saved the search indexes in a feature.
When the feature is being installed, and the node entry is created in the search_api_item table, I get the following error :
Duplicate entry in SearchApiAbstractDataSourceController->trackItemInsert()
Comment #5
drunken monkeyAs I see it, this is most likely not a bug in the Search API, but rather in Features, core or in whatever code creates the node. It seems that the node is first inserted, then the index is created and only then
hook_entity_insert()
is invoked for the node. Of course this would lead to problems, as it violates the contract for the insert hook.So probably you should try to debug this further, look at the control flow and find out where the problem might lay.
In not making this throw an exception and bring everything to a halt, this would probably be another point for #1253320: Improve error handling. I should be far more cautious in throwing exceptions from hook implementations.
Comment #6
drunken monkeyAh, OK, now I understand the problem! Sorry for being so slow. ;)
- Node is created,
hook_entity_insert()
is invoked.- Hence,
search_api_entity_insert()
is called.- There, indexes are loaded.
- Features/Entity API sees the new index in code and calls
hook_search_api_index_insert()
for it.- Tracking for the index is started, of course already including the new node.
- Code in
search_api_entity_insert()
continues, trying to add the node again to tracking.- Chaos ensues.
I guess the proper fix here is to include something like your patch on a more generic level in the Entity API (or in Features – I don't know the code distribution between the two in this respect that well) – this is bound to happen with other exportable-providing modules, too, I guess. Or, you could include the fix in the code which creates the node, if that is custom code.
Also, the above issue would at least remove the exception, and you could probably live with an unnecessary watchdog entry more easily.
Comment #7
Damien Tournoud CreditAttribution: Damien Tournoud commentedYep, you are right, let's move this to the Entity API.
@fago, let's discuss the lazy-rebuilding of the defaults. The problem we have here is that Search API does some processing when an exported entity is created, and this can happen at a very inconvenient time because of the lazy-rebuilding of the defaults.
In our case, it happened after a
node_save()
:I see two options:
entity_defaults_rebuild()
is called, so as to avoid entities being rebuilt at inconvenient times.Thoughts?
Comment #8
Damien Tournoud CreditAttribution: Damien Tournoud commentedComment #9
fagoOuch.
Currently defaults get rebuilt on cache-clear + on module install and fixed on uninstall. They are not immediately rebuilt though, but just marked to be rebuilt as soon as needed, i.e. on entity-load(). We could certainly immediately rebuild the entities provided in code when modules are enabled, however what's about the cache-clear case?
I'm a bit worried that rebuilding *all* exportable entities immediately upon cache clear might be too much. Also, it might in consequence trigger immediate cache-rebuilds of modules, what wouldn't be ideal on cache-clear either.
Of course we could introduce an option to disable cache-clear rebuilds for modules. That would probably require people to implement update hooks for changes to take affect then... ? Thoughts?
Comment #10
fagoJust ran into an issue caused by #1273046: don't call rules_invoke_event() for configuration in conjunction with this. I must agree that having entity_load() possibly triggering an entity_save() is really not optimal.
Alternatives?
Just do it immediately on cache-clear? That would result in an immediate cache-rebuild of entity-info, what's not optimal either. However, looking at drupal_flush_all_caches() it does a menu_rebuild() before invoking the hook, so it probably has been already built for the menu anyway...
Comment #11
fagook, here is a patch implementing the alternative method, i.e. immediately rebuild on cache-clear. The patch misses documentation updates and polishing, but suffices to give it a try.
Please test it on your sites, in particular on sites with lots of exportable entities in code and report whether it works for you.
Comment #12
mh86 CreditAttribution: mh86 commentedsubscribing
Comment #13
fagoProbably it's a good idea to do that, so here is a more polished patch. Seems to work for me, please test!!
(in particular on sites with lots of exportable entities in code)
Comment #14
fagoWorks fine on some sites -> Committed.
Comment #16
rfayThis commit seems to have caused #1316140: Default rule configurations are no longer being rebuilt for tax types / rates and related #1321564: Delay in creating rules component menu links?.
I reverted Entity API commit e502b408 (this one) on the testbot and the tests ran fine.
Comment #17
fago@rfay: I've responded there, so setting this back to closed.
Comment #17.0
fagoAdd precisions
Comment #18
kenorb CreditAttribution: kenorb commented