From 91cbc857c1ef34e90265fce7d423074a193ffa11 Mon Sep 17 00:00:00 2001
From: Eileen McNaughton <emcnaughton@wikimedia.org>
Date: Thu, 26 Jan 2023 10:25:28 +1300
Subject: [PATCH] Fix slow queries in snapshot process

SELECT COUNT(*) is slow to run on a large database but SELECT MAX(id) is
quick. The snapshot check only needs to be very rough - so MAX(id) should
be accurate enough.

This does make the message a little inaccurate - but my money says no-one
will notice & it's not worth creating work for the translators to change
---
 CRM/Core/DAO.php         |  2 ++
 CRM/Upgrade/Snapshot.php | 32 ++++++++++++++++++++++----------
 2 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/CRM/Core/DAO.php b/CRM/Core/DAO.php
index f5f376fa12..eb84c1651c 100644
--- a/CRM/Core/DAO.php
+++ b/CRM/Core/DAO.php
@@ -1726,9 +1726,11 @@ LIKE %1
    * @param array $params
    * @param bool $abort
    * @param bool $i18nRewrite
+   *
    * @return string|null
    *   the result of the query if any
    *
+   * @throws \CRM_Core_Exception
    */
   public static function &singleValueQuery(
     $query,
diff --git a/CRM/Upgrade/Snapshot.php b/CRM/Upgrade/Snapshot.php
index 9f5a493844..0f73ee3c5e 100644
--- a/CRM/Upgrade/Snapshot.php
+++ b/CRM/Upgrade/Snapshot.php
@@ -9,6 +9,8 @@
  +--------------------------------------------------------------------+
  */
 
+use Civi\Test\Invasive;
+
 /**
  * Provide helpers for recording data snapshots during an upgrade.
  */
@@ -35,6 +37,7 @@ class CRM_Upgrade_Snapshot {
 
   /**
    * Get a list of reasons why the snapshots should not run.
+   *
    * @return array
    *   List of printable messages.
    */
@@ -57,15 +60,16 @@ class CRM_Upgrade_Snapshot {
       static::$activationIssues = [];
       foreach ($limits as $table => $limit) {
         try {
-          $count = CRM_Core_DAO::singleValueQuery("SELECT count(*) FROM `{$table}`");
+          // Use select MAX(id) rather than COUNT as COUNT is slow on large databases.
+          $max = CRM_Core_DAO::singleValueQuery("SELECT MAX(id) FROM `{$table}`");
         }
-        catch (\Exception $e) {
-          $count = 0;
+        catch (CRM_Core_Exception $e) {
+          $max = 0;
         }
-        if ($count > $limit) {
+        if ($max > $limit) {
           static::$activationIssues["count_{$table}"] = ts('Table "%1" has a large number of records (%2 > %3).', [
             1 => $table,
-            2 => $count,
+            2 => $max,
             3 => $limit,
           ]);
         }
@@ -129,11 +133,13 @@ class CRM_Upgrade_Snapshot {
    *   Ex: '5.50'
    * @param string $name
    * @param \CRM_Utils_SQL_Select $select
+   *
+   * @return iterable
    * @throws \CRM_Core_Exception
    */
   public static function createTasks(string $owner, string $version, string $name, CRM_Utils_SQL_Select $select): iterable {
     $destTable = static::createTableName($owner, $version, $name);
-    $srcTable = \Civi\Test\Invasive::get([$select, 'from']);
+    $srcTable = Invasive::get([$select, 'from']);
 
     // Sometimes, backups fail and people rollback and try again. Reset prior snapshots.
     CRM_Core_DAO::executeQuery("DROP TABLE IF EXISTS `{$destTable}`");
@@ -164,7 +170,9 @@ class CRM_Upgrade_Snapshot {
   /**
    * @param \CRM_Queue_TaskContext $ctx
    * @param string $sql
+   *
    * @return bool
+   * @noinspection PhpUnusedParameterInspection
    */
   public static function insertSnapshotTask(CRM_Queue_TaskContext $ctx, string $sql): bool {
     CRM_Core_DAO::executeQuery($sql);
@@ -182,9 +190,13 @@ class CRM_Upgrade_Snapshot {
    *   The current version of CiviCRM.
    * @param int|null $cleanupAfter
    *   How long should we retain old snapshots?
-   *   Time is measured in terms of MINOR versions - eg "4" means "retain for 4 MINOR versions".
-   *   Thus, on v5.60, you could delete any snapshots predating 5.56.
+   *   Time is measured in terms of MINOR versions - eg "4" means "retain for 4
+   *   MINOR versions". Thus, on v5.60, you could delete any snapshots
+   *   predating 5.56.
+   *
    * @return bool
+   * @throws \CRM_Core_Exception
+   * @noinspection PhpUnusedParameterInspection
    */
   public static function cleanupTask(?CRM_Queue_TaskContext $ctx = NULL, string $owner = 'civicrm', ?string $version = NULL, ?int $cleanupAfter = NULL): bool {
     $version = $version ?: CRM_Core_BAO_Domain::version();
@@ -194,12 +206,12 @@ class CRM_Upgrade_Snapshot {
     $cutoff = $major . '.' . max(0, $minor - $cleanupAfter);
 
     $dao = new CRM_Core_DAO();
-    $query = "
+    $query = '
       SELECT TABLE_NAME as tableName
       FROM   INFORMATION_SCHEMA.TABLES
       WHERE  TABLE_SCHEMA = %1
       AND TABLE_NAME LIKE %2
-    ";
+    ';
     $tables = CRM_Core_DAO::executeQuery($query, [
       1 => [$dao->database(), 'String'],
       2 => ["snap_{$owner}_v%", 'String'],
-- 
2.25.1