From 46fe0a66dc6bfc0fb63bfc0344ea6ff70ca40c8c Mon Sep 17 00:00:00 2001
From: eileen <emcnaughton@wikimedia.org>
Date: Fri, 21 Jun 2019 18:26:35 -0400
Subject: [PATCH] Reduce deadlocks on inserting custom data by only using 'ON
 DUPLICATE' when it is not a new row

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)
---
 CRM/Contribute/BAO/Contribution.php |  5 ++++-
 CRM/Core/BAO/CustomValueTable.php   | 18 ++++++++++++++----
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php
index 12adfb8f00..11c423c327 100644
--- 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 ff9e634108..c25baac556 100644
--- 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);
     }
   }
 
-- 
2.25.1