When an entity is created, this module checks if the entity type should be added to the index table.
However, when an entity is deleted this module's code is always running without any checks of the entity's type:
This is what happens in our environment:
1) Drupal Commerce decies to refresh the cart:
commerce_cart_order_refresh() called at [.../sites/all/modules/commerce/modules/cart/commerce_cart.module:601]
2) Commerce Fees decides to delete a fee line item from the order:
commerce_fees_commerce_cart_order_refresh()
commerce_line_item_delete_multiple() called at [.../sites/all/modules/commerce_fees/commerce_fees.module:557]
3) The entity is deleted and all module's hooks fires:
call_user_func_array() called at [.../includes/module.inc:895]
apachesolr_entity_delete()
4) Apachesolr sends the delete query to the database without checking the entity type:
apachesolr_remove_entity() called at [.../sites/all/modules/apachesolr/apachesolr.module:2069]
apachesolr_index_delete_entity_from_index() called at [.../sites/all/modules/apachesolr/apachesolr.module:2084]
apachesolr_set_last_index_updated() called at [.../sites/all/modules/apachesolr/apachesolr.index.inc:711]
Proposed solution
Check the entity type like we do when adding entities by adding apachesolr_entity_should_index():
function apachesolr_entity_delete($entity, $entity_type) {
if (apachesolr_entity_should_index($entity, $entity_type)) {
list($entity_id) = entity_extract_ids($entity_type, $entity);
// Get all environments and delete it from their table and index
$environments = apachesolr_load_all_environments();
foreach ($environments as $environment) {
apachesolr_remove_entity($environment['env_id'], $entity_type, $entity_id);
}
}
}
Comment | File | Size | Author |
---|---|---|---|
#6 | 2605756-check-entity-type-on-entity-delete.patch | 899 bytes | volkerk |
Comments
Comment #2
periksson CreditAttribution: periksson commentedComment #3
jgrubb CreditAttribution: jgrubb as a volunteer commentedJust roll a patch if that solution works for you. Looks reasonable to me. Just to be clear - this isn't actually causing any problems, just extra calls to delete items from the index that don't exist in the first place, right?
Comment #4
volkerk CreditAttribution: volkerk commentedGranted this is a non issue in 'normal' operation. Anyway, if you are trying to mass-delete entity data which is not indexed, the missing check will significantly slow down the operation.
Comment #6
volkerk CreditAttribution: volkerk commentedRe-roll patch without path.
Comment #7
volkerk CreditAttribution: volkerk commentedComment #8
volkerk CreditAttribution: volkerk commentedComment #9
periksson CreditAttribution: periksson commentedThis issue causes some (minor) deadlocks in the database, due to increased traffic in the db for commerce entities related to the customer's order.
Thanks for the patch!
Comment #10
jgrubb CreditAttribution: jgrubb as a volunteer commentedI committed this locally but haven't had time to test it out yet. This will land soon.
Comment #11
sorinb CreditAttribution: sorinb as a volunteer commentedI feel that's the right approach, I ended up in the same implementation. Just thinking if Solr handles nested references cleanups when a file is deleted and refferenced from a parent document, but mainly this should be handled... but mostly that's architecture question and specific scenarions not sure it relates to how solr works in this case.
Comment #12
nvahalik CreditAttribution: nvahalik at Code and Salt commentedJust installed this patch locally and tested with
7.x-1.8
. Works as expected and in my instance produced a huge performance gain...Comment #13
janusman CreditAttribution: janusman at Acquia commentedRTBC
Comment #15
janusman CreditAttribution: janusman at Acquia commented