From: eileen <emcnaughton@wikimedia.org>
Date: Thu, 31 Oct 2019 02:55:44 +0000 (+1300)
Subject: dev/core#1355 Add contention handling for dedupe jobs
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=c9f05fb31c229c1ea180a70abaec9adc1c88ebc0;p=civicrm-core.git

dev/core#1355 Add contention handling for dedupe jobs

https://lab.civicrm.org/dev/core/issues/1355
---

diff --git a/CRM/Core/Exception/ResourceConflictException.php b/CRM/Core/Exception/ResourceConflictException.php
new file mode 100644
index 0000000000..c722320295
--- /dev/null
+++ b/CRM/Core/Exception/ResourceConflictException.php
@@ -0,0 +1,42 @@
+<?php
+/*
+ +--------------------------------------------------------------------+
+ | CiviCRM version 5                                                  |
+ +--------------------------------------------------------------------+
+ | Copyright CiviCRM LLC (c) 2004-2019                                |
+ +--------------------------------------------------------------------+
+ | This file is a part of CiviCRM.                                    |
+ |                                                                    |
+ | CiviCRM is free software; you can copy, modify, and distribute it  |
+ | under the terms of the GNU Affero General Public License           |
+ | Version 3, 19 November 2007 and the CiviCRM Licensing Exception.   |
+ |                                                                    |
+ | CiviCRM is distributed in the hope that it will be useful, but     |
+ | WITHOUT ANY WARRANTY; without even the implied warranty of         |
+ | MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.               |
+ | See the GNU Affero General Public License for more details.        |
+ |                                                                    |
+ | You should have received a copy of the GNU Affero General Public   |
+ | License and the CiviCRM Licensing Exception along                  |
+ | with this program; if not, contact CiviCRM LLC                     |
+ | at info[AT]civicrm[DOT]org. If you have questions about the        |
+ | GNU Affero General Public License or the licensing of CiviCRM,     |
+ | see the CiviCRM license FAQ at http://civicrm.org/licensing        |
+ +--------------------------------------------------------------------+
+ */
+
+/**
+ * Exception thrown when contention over a resource causes process to abort.
+ *
+ * @param string $message
+ *   The human friendly error message.
+ * @param string $error_code
+ *   A computer friendly error code. By convention, no space (but underscore allowed).
+ *   ex: mandatory_missing, duplicate, invalid_format
+ * @param array $data
+ *   Extra params to return. eg an extra array of ids. It is not mandatory, but can help the computer using the api.
+ * Keep in mind the api consumer isn't to be trusted. eg. the database password is NOT a good extra data.
+ */
+class CRM_Core_Exception_ResourceConflictException extends \CRM_Core_Exception {
+
+}
diff --git a/CRM/Dedupe/Merger.php b/CRM/Dedupe/Merger.php
index 339338bf2a..fd7edd2c4c 100644
--- a/CRM/Dedupe/Merger.php
+++ b/CRM/Dedupe/Merger.php
@@ -2062,6 +2062,9 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
     }
     $migrationInfo = [];
     $conflicts = [];
+    // Try to lock the contacts before we load the data as we don't want it changing under us.
+    // https://lab.civicrm.org/dev/core/issues/1355
+    $locks = self::getLocksOnContacts([$mainId, $otherId]);
     if (!CRM_Dedupe_Merger::skipMerge($mainId, $otherId, $migrationInfo, $mode, $conflicts)) {
       CRM_Dedupe_Merger::moveAllBelongings($mainId, $otherId, $migrationInfo, $checkPermissions);
       $resultStats['merged'][] = [
@@ -2083,6 +2086,7 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
     else {
       CRM_Core_BAO_PrevNextCache::deletePair($mainId, $otherId, $cacheKeyString);
     }
+    self::releaseLocks($locks);
     return $resultStats;
   }
 
@@ -2570,4 +2574,59 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
     return $locationBlock;
   }
 
+  /**
+   * Get a lock on the given contact.
+   *
+   * The lock is like a gentleman's agreement between php & mysql. It is reserved at the
+   * mysql level so it works across php processes but it doesn't actually lock the database.
+   *
+   * Instead php can check the lock to see if it has been acquired before taking an action.
+   *
+   * In this case we really don't want to attempt to dedupe contacts if another process is
+   * trying to act on the specific contact as it could result in messy deadlocks & possibly data corruption.
+   * In most databases this would be a rare event but if multiple dedupe processes are running
+   * at once (for example) or there is also an import process in play there is potential for them to crash.
+   * By throwing a specific error the calling process can catch it and determine it is worth trying again later without a lot of
+   * noise.
+   *
+   * As of writing no other processes DO grab contact locks but it would be reasonable to consider
+   * grabbing them doing contact edits in general as well as imports etc.
+   *
+   * @param array $contacts
+   *
+   * @return array
+   *
+   * @throws \CRM_Core_Exception
+   * @throws \CRM_Core_Exception_ResourceConflictException
+   */
+  protected static function getLocksOnContacts($contacts):array {
+    $locks = [];
+    if (!CRM_Utils_SQL::supportsMultipleLocks()) {
+      return $locks;
+    }
+    foreach ($contacts as $contactID) {
+      $lock = Civi::lockManager()->acquire("data.core.contact.{$contactID}");
+      if ($lock->isAcquired()) {
+        $locks[] = $lock;
+      }
+      else {
+        self::releaseLocks($locks);
+        throw new CRM_Core_Exception_ResourceConflictException(ts('Contact is in Use'), 'contact_lock');
+      }
+    }
+    return $locks;
+  }
+
+  /**
+   * Release contact locks so another process can alter them if it wants.
+   *
+   * @param array $locks
+   */
+  protected static function releaseLocks(array $locks) {
+    foreach ($locks as $lock) {
+      /* @var Civi\Core\Lock\LockInterface $lock */
+      $lock->release();
+    }
+  }
+
 }