Reduce deadlocks on inserting custom data by only using 'ON DUPLICATE' when it is...

ON Duplicate is used in the customData.create function so that a row can be inserted and if it exists
mysql adapts and updates the existing row. This is more expensive and more prone to deadlocks than a straight
'INSERT' but is probably better than figuring it out at the php layer when you don't know if it could be
an update rather than an insert.

However, in many of the cases we already know this information - ie. if we are creating a new contribution
the custom data is created afterwards so we can use this information from Contribution.create to opt for the
cheaper & less deadlocky version.

We deployed this fix before our main fundraiser due to handful of daily deadlocks on this under peak load
and this form of deadlock did not bother us again during our main fundraiser when we were processing large volumes.
The patch has been in production around 6 months at this point.

Note that the reason deadlocks are encountered is that the 'next row' index is locked when inserting, and
it's either locked for longer or more aggressively when it;'s also checking for a deadlock at the same time (not
sure which)

diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php
index 12adfb8f006b7f84f4fb306fbc70d233f0fa7224..11c423c3279ae1c1fddf23435471fe623acb75c7 100644 (file)
--- a/CRM/Contribute/BAO/Contribution.php
+++ b/CRM/Contribute/BAO/Contribution.php
@@ -496,6 +496,9 @@ class CRM_Contribute_BAO_Contribution extends CRM_Contribute_DAO_Contribution {
    * @return CRM_Contribute_BAO_Contribution
    */
   public static function create(&$params, $ids = []) {
+    $contributionID = CRM_Utils_Array::value('contribution', $ids, CRM_Utils_Array::value('id', $params));
+    $action = $contributionID ? 'edit' : 'create';
+
     $dateFields = [
       'receive_date',
       'cancel_date',
@@ -524,7 +527,7 @@ class CRM_Contribute_BAO_Contribution extends CRM_Contribute_DAO_Contribution {
     if (!empty($params['custom']) &&
       is_array($params['custom'])
     ) {
-      CRM_Core_BAO_CustomValueTable::store($params['custom'], 'civicrm_contribution', $contribution->id);
+      CRM_Core_BAO_CustomValueTable::store($params['custom'], 'civicrm_contribution', $contribution->id, $action);
     }
 
     $session = CRM_Core_Session::singleton();

diff --git a/CRM/Core/BAO/CustomValueTable.php b/CRM/Core/BAO/CustomValueTable.php
index ff9e634108be7dad93d2c30257d2154f92198742..c25baac556bdd74b4f3d8bb96ddb97002a9c44be 100644 (file)
--- a/CRM/Core/BAO/CustomValueTable.php
+++ b/CRM/Core/BAO/CustomValueTable.php
@@ -36,10 +36,15 @@ class CRM_Core_BAO_CustomValueTable {
 
   /**
    * @param array $customParams
+   * @param string $parentOperation Operation being taken on the parent entity.
+   *   If we know the parent entity is doing an insert we can skip the
+   *   ON DUPLICATE UPDATE - which improves performance and reduces deadlocks.
+   *   - edit
+   *   - create
    *
    * @throws Exception
    */
-  public static function create($customParams) {
+  public static function create($customParams, $parentOperation = NULL) {
     if (empty($customParams) ||
       !is_array($customParams)
     ) {
@@ -256,7 +261,7 @@ class CRM_Core_BAO_CustomValueTable {
             $fieldValues = implode(',', array_values($set));
             $query = "$sqlOP ( $fieldNames ) VALUES ( $fieldValues )";
             // for multiple values we dont do on duplicate key update
-            if (!$isMultiple) {
+            if (!$isMultiple && $parentOperation !== 'create') {
               $query .= " ON DUPLICATE KEY UPDATE $setClause";
             }
           }
@@ -337,8 +342,13 @@ class CRM_Core_BAO_CustomValueTable {
    * @param array $params
    * @param $entityTable
    * @param int $entityID
+   * @param string $parentOperation Operation being taken on the parent entity.
+   *   If we know the parent entity is doing an insert we can skip the
+   *   ON DUPLICATE UPDATE - which improves performance and reduces deadlocks.
+   *   - edit
+   *   - create
    */
-  public static function store($params, $entityTable, $entityID) {
+  public static function store($params, $entityTable, $entityID, $parentOperation = NULL) {
     $cvParams = [];
     foreach ($params as $fieldID => $param) {
       foreach ($param as $index => $customValue) {
@@ -375,7 +385,7 @@ class CRM_Core_BAO_CustomValueTable {
       }
     }
     if (!empty($cvParams)) {
-      self::create($cvParams);
+      self::create($cvParams, $parentOperation);
     }
   }