Hi,

We've been having isolated but constant issues that break the whole process when the exception occurs. For us, the rules module is executed and try to index the entity created (sometimes users, nodes or custom entities) and breaks the execution, avoiding the execution of code that comes after the indexation process and causing very strange bugs.

One of our exceptions, and the most common one, is the following:
PDOException: SQLSTATE[40001]: Serialization failure: 1213 Deadlock found when trying to get lock; try restarting transaction: UPDATE {search_api_item} SET changed=:db_update_placeholder_0 WHERE (index_id = :db_condition_placeholder_0) AND (changed = :db_condition_placeholder_1) AND (item_id IN (:db_condition_placeholder_2)) ; Array ( [:db_update_placeholder_0] => 1590004964 [:db_condition_placeholder_0] => 226 [:db_condition_placeholder_1] => 0 [:db_condition_placeholder_2] => 84657 ) in SearchApiAbstractDataSourceController->trackItemChange() (line 655 of /mnt/www/html/example/www/sites/all/modules/contrib/search_api/includes/datasource.inc).

What we've realized is that, on trackItemChange, there could be an Exception and is not properly handled.

CommentFileSizeAuthor
#7 3154538-7--catch_db_exceptions_in_tracking.patch3.8 KBdrunken monkey
#7 3154538-7--catch_db_exceptions_in_tracking--interdiff.txt3.75 KBdrunken monkey
#2 3154538-2-unhandled-pdoexception-on-trackItemChange.patch830 bytesisholgueras
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

isholgueras created an issue. See original summary.

isholgueras’s picture

isholgueras CreditAttribution: isholgueras at Bluespark commented
Priority: Normal » Major
Status: Active » Needs review
FileSize
3154538-2-unhandled-pdoexception-on-trackItemChange.patch830 bytes

Here is the patch I've created to handle, log the exception and avoid halt the execution.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
CreditAttribution: drunken monkey as a volunteer commented
Component: General code » Framework
Priority: Major » Normal

I’m not quite sure whether catching this exception is a good idea. After all, this would mean that the entity change goes unnoticed and your index potentially goes stale over time. And the exception is, at least from the looks of it, not in our responsibility, so little we can do to avoid it. It seems you should just investigate and fix the underlying problem instead of trying to cover it.

I’m not 100% sure about this and ready to be convinced otherwise, but in general it’s always good to be wary about just catching and logging exceptions.

solideogloria’s picture

solideogloria CreditAttribution: solideogloria commented

It'd be better to find the cause of the deadlock, I'd think.

isholgueras’s picture

isholgueras CreditAttribution: isholgueras at Bluespark commented

The Deadlock can occur because of many reasons. Maybe because of the search_api code, or rules or maybe because MySQL itself.

I agree with trying to fix and focus the Deadlock, but I was not able to find where is occurring this Deadlock, because it's only happening on our Production environment and without a pattern. It could be caused because the server is a bit stressed at this time or don't know.

In my opinion, adding this try for the update is like adding a try in a $node->save(). You depend on a third service to be ready and enable or the process could fail.

For us, and maybe for more users, this Exception breaks the execution, avoiding completely the execution of the following methods. Let me explain more clearly.

  1. The order is saved
  2. The order is indexed
  3. The order moves from "pending" to "completed"
  4. The order is sent to the customer

If the Exception occurs on step 2, steps 3 and 4 are not being executed.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
CreditAttribution: drunken monkey as a volunteer commented

Thanks for the more detailed description. After a bit of deliberation, I now tend to agree with you. Having this throw an exception (which should then, at the very least, be documented) really only causes yet more problems, without solving anything. Really, the only way we could get back to a “consistent” state from an exception there is to roll back the complete entity save. As that’s not possible, it’s still better to allow other modules to do their thing than to bring the whole page request to a halt. If that works, then the only “damage” done is wrong tracking information for that item in the Search API – which is pretty much inevitable at that point anyways.
So, probably catching and logging is really the way to go there.

However, I also linked this issue in #1188562: [meta] Important project announcements to maybe get a bit more input. Changing exception behavior is always fraught with pitfalls – better to let this rest for another week or two and make sure everyone who wants can have their say, than to make a maybe detrimental change.

Still, thanks again for keeping on this!

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
CreditAttribution: drunken monkey as a volunteer commented
FileSize
3154538-7--catch_db_exceptions_in_tracking--interdiff.txt3.75 KB
3154538-7--catch_db_exceptions_in_tracking.patch3.8 KB

Here is also a revision to your patch, with proper logging and also covering the other tracking methods.

isholgueras’s picture

isholgueras CreditAttribution: isholgueras at Bluespark commented

Yeah, definitely you patch makes much more sense than mine :). That's what I've missed in the first sight.

Thanks!

drunken monkey’s picture

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

OK, seems no-one else has an opinion on this, so should be fine.
Committed.
Thanks again!

Status: Fixed » Closed (fixed)

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