Move methods related to optimizing database tables to schema classes (#…

…22355) * Use methods related to optimizing database tables to schema classes * apply review feedback
matomo-org · Jul 5, 2024 · d712ca6 · d712ca6
1 parent bd652c0
commit d712ca6
Show file tree

Hide file tree

Showing 19 changed files with 350 additions and 149 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,10 @@ The Product Changelog at **[matomo.org/changelog](https://matomo.org/changelog)*
 
 * The dependency `jQuery.dotdotdot` has been removed. Please use pure CSS instead or include the library in your plugin if needed.
 
+## Deprecations
+
+The methods `Db::isOptimizeInnoDBSupported` and `Db::optimizeTables` have been deprecated. Use `Db\Schema::getInstance()->isOptimizeInnoDBSupported` and `Db\Schema::getInstance()->optimizeTables` instead
+
 ## Matomo 5.1.0
 
 ### Breaking Changes

diff --git a/core/Db.php b/core/Db.php
@@ -11,6 +11,7 @@
 
 use Exception;
 use Piwik\Db\Adapter;
+use Piwik\Db\Schema;
 
 /**
  * Contains SQL related helper functions for Piwik's MySQL database.
@@ -466,63 +467,17 @@ public static function deleteAllRows($table, $where, $orderBy, $maxRowsPerQuery
      *                             Table names must be prefixed (see {@link Piwik\Common::prefixTable()}).
      * @param bool $force If true, the `OPTIMIZE TABLE` query will be run even if InnoDB tables are being used.
      * @return bool
+     * @deprecated  will be removed in Matomo 6
+     *              use Schema::getInstance()->optimizeTables() instead
      */
     public static function optimizeTables($tables, $force = false)
     {
-        $optimize = Config::getInstance()->General['enable_sql_optimize_queries'];
-
-        if (
-            empty($optimize)
-            && !$force
-        ) {
-            return false;
-        }
-
-        if (empty($tables)) {
-            return false;
-        }
-
-        if (!is_array($tables)) {
-            $tables = array($tables);
-        }
-
-        if (
-            !self::isOptimizeInnoDBSupported()
-            && !$force
-        ) {
-            // filter out all InnoDB tables
-            $myisamDbTables = array();
-            foreach (self::getTableStatus() as $row) {
-                if (
-                    strtolower($row['Engine']) == 'myisam'
-                    && in_array($row['Name'], $tables)
-                ) {
-                    $myisamDbTables[] = $row['Name'];
-                }
-            }
-
-            $tables = $myisamDbTables;
-        }
-
         if (empty($tables)) {
             return false;
         }
 
-        // optimize the tables
-        $success = true;
-        foreach ($tables as &$t) {
-            $ok = self::query('OPTIMIZE TABLE ' . $t);
-            if (!$ok) {
-                $success = false;
-            }
-        }
-
-        return $success;
-    }
-
-    private static function getTableStatus()
-    {
-        return Db::fetchAll("SHOW TABLE STATUS");
+        $tables = !is_array($tables) ? [$tables] : $tables;
+        return Schema::getInstance()->optimizeTables($tables, (bool) $force);
     }
 
     /**
@@ -912,19 +867,12 @@ public static function isQueryLogEnabled()
         return self::$logQueries;
     }
 
+    /**
+     * @deprecated will be removed with Matomo 6
+     *             use Schema::getInstance()->isOptimizeInnoDBSupported() instead
+     */
     public static function isOptimizeInnoDBSupported($version = null)
     {
-        if ($version === null) {
-            $version = Db::fetchOne("SELECT VERSION()");
-        }
-
-        $version = strtolower($version);
-
-        if (strpos($version, "mariadb") === false) {
-            return false;
-        }
-
-        $semanticVersion = strstr($version, '-', $beforeNeedle = true);
-        return version_compare($semanticVersion, '10.1.1', '>=');
+        return Db\Schema::getInstance()->isOptimizeInnoDBSupported();
     }
 }
diff --git a/core/Db/Schema.php b/core/Db/Schema.php
@@ -254,4 +254,30 @@ public function supportsComplexColumnUpdates(): bool
     {
         return $this->getSchema()->supportsComplexColumnUpdates();
     }
+
+    /**
+     * Returns if the schema supports `OPTIMIZE TABLE` statements for innodb tables
+     *
+     * @return bool
+     */
+    public function isOptimizeInnoDBSupported(): bool
+    {
+        return $this->getSchema()->isOptimizeInnoDBSupported();
+    }
+
+    /**
+     * Runs an `OPTIMIZE TABLE` query on the supplied table or tables.
+     *
+     * Tables will only be optimized if the `[General] enable_sql_optimize_queries` INI config option is
+     * set to **1**.
+     *
+     * @param string|array $tables The name of the table to optimize or an array of tables to optimize.
+     *                             Table names must be prefixed (see {@link Piwik\Common::prefixTable()}).
+     * @param bool $force If true, the `OPTIMIZE TABLE` query will be run even if InnoDB tables are being used.
+     * @return bool
+     */
+    public function optimizeTables(array $tables, bool $force = false): bool
+    {
+        return $this->getSchema()->optimizeTables($tables, $force);
+    }
 }
diff --git a/core/Db/Schema/Mariadb.php b/core/Db/Schema/Mariadb.php
@@ -37,4 +37,11 @@ public function addMaxExecutionTimeHintToQuery(string $sql, float $limit): strin
 
         return $sql;
     }
+
+    public function isOptimizeInnoDBSupported(): bool
+    {
+        $version = strtolower($this->getVersion());
+        $semanticVersion = strstr($version, '-', $beforeNeedle = true);
+        return version_compare($semanticVersion, '10.1.1', '>=');
+    }
 }
diff --git a/core/Db/Schema/Mysql.php b/core/Db/Schema/Mysql.php
@@ -12,6 +12,7 @@
 use Exception;
 use Piwik\Common;
 use Piwik\Concurrency\Lock;
+use Piwik\Config;
 use Piwik\Date;
 use Piwik\Db\SchemaInterface;
 use Piwik\Db;
@@ -688,6 +689,69 @@ public function getTableCreateOptions(): string
         return $options;
     }
 
+    public function optimizeTables(array $tables, bool $force = false): bool
+    {
+        $optimize = Config::getInstance()->General['enable_sql_optimize_queries'];
+
+        if (
+            empty($optimize)
+            && !$force
+        ) {
+            return false;
+        }
+
+        if (empty($tables)) {
+            return false;
+        }
+
+        if (
+            !$this->isOptimizeInnoDBSupported()
+            && !$force
+        ) {
+            // filter out all InnoDB tables
+            $myisamDbTables = array();
+            foreach ($this->getTableStatus() as $row) {
+                if (
+                    strtolower($row['Engine']) == 'myisam'
+                    && in_array($row['Name'], $tables)
+                ) {
+                    $myisamDbTables[] = $row['Name'];
+                }
+            }
+
+            $tables = $myisamDbTables;
+        }
+
+        if (empty($tables)) {
+            return false;
+        }
+
+        // optimize the tables
+        $success = true;
+        foreach ($tables as &$t) {
+            $ok = Db::query('OPTIMIZE TABLE ' . $t);
+            if (!$ok) {
+                $success = false;
+            }
+        }
+
+        return $success;
+    }
+
+    public function isOptimizeInnoDBSupported(): bool
+    {
+        $version = strtolower($this->getVersion());
+
+        // Note: This check for MariaDb is here on purpose, so it's working correctly for people
+        // having MySQL still configured, when using MariaDb
+        if (strpos($version, "mariadb") === false) {
+            return false;
+        }
+
+        $semanticVersion = strstr($version, '-', $beforeNeedle = true);
+        return version_compare($semanticVersion, '10.1.1', '>=');
+    }
+
     protected function getDatabaseCreateOptions(): string
     {
         $charset = DbHelper::getDefaultCharset();
@@ -715,6 +779,16 @@ private function getTablePrefix()
         return $this->getDbSettings()->getTablePrefix();
     }
 
+    protected function getVersion(): string
+    {
+        return Db::fetchOne("SELECT VERSION()");
+    }
+
+    protected function getTableStatus()
+    {
+        return Db::fetchAll("SHOW TABLE STATUS");
+    }
+
     private function getDb()
     {
         return Db::get();

diff --git a/core/Db/Schema/Tidb.php b/core/Db/Schema/Tidb.php
@@ -51,6 +51,17 @@ public function getTableCreateOptions(): string
         return $options;
     }
 
+    public function isOptimizeInnoDBSupported(): bool
+    {
+        return false;
+    }
+
+    public function optimizeTables(array $tables, bool $force = false): bool
+    {
+        // OPTIMIZE TABLE not supported for TiDb
+        return false;
+    }
+
     protected function getDatabaseCreateOptions(): string
     {
         $charset = DbHelper::getDefaultCharset();

diff --git a/core/Db/SchemaInterface.php b/core/Db/SchemaInterface.php
@@ -138,4 +138,24 @@ public function getDefaultPort(): int;
      * @return string
      */
     public function getTableCreateOptions(): string;
+
+    /**
+     * Returns if performing on `OPTIMIZE TABLE` is supported for InnoDb tables
+     *
+     * @return bool
+     */
+    public function isOptimizeInnoDBSupported(): bool;
+
+    /**
+     * Runs an `OPTIMIZE TABLE` query on the supplied table or tables.
+     *
+     * Tables will only be optimized if the `[General] enable_sql_optimize_queries` INI config option is
+     * set to **1**.
+     *
+     * @param array $tables The name of the table to optimize or an array of tables to optimize.
+     *                      Table names must be prefixed (see {@link Piwik\Common::prefixTable()}).
+     * @param bool $force If true, the `OPTIMIZE TABLE` query will be run even if InnoDB tables are being used.
+     * @return bool
+     */
+    public function optimizeTables(array $tables, bool $force = false): bool;
 }
diff --git a/plugins/CoreAdminHome/Commands/DeleteLogsData.php b/plugins/CoreAdminHome/Commands/DeleteLogsData.php
@@ -194,7 +194,7 @@ private function optimizeTables()
 
             $prefixedTable = Common::prefixTable($table);
 
-            $done = Db::optimizeTables($prefixedTable);
+            $done = Db\Schema::getInstance()->optimizeTables([$prefixedTable]);
 
             if ($done) {
                 $this->getOutput()->writeln("done. <comment>" . $timer . "</comment>");

diff --git a/plugins/CoreAdminHome/Commands/OptimizeArchiveTables.php b/plugins/CoreAdminHome/Commands/OptimizeArchiveTables.php
@@ -64,7 +64,7 @@ private function optimizeTable($dryRun, $table)
         if ($dryRun) {
             $output->write("[dry-run, not optimising table]");
         } else {
-            Db::optimizeTables(Common::prefixTable($table), $force = true);
+            Db\Schema::getInstance()->optimizeTables([Common::prefixTable($table)], $force = true);
         }
 
         $output->writeln("Done.");

diff --git a/plugins/CoreAdminHome/Commands/PurgeOldArchiveData.php b/plugins/CoreAdminHome/Commands/PurgeOldArchiveData.php
@@ -198,12 +198,12 @@ private function optimizeArchiveTables($dates)
         foreach ($dates as $date) {
             $numericTable = ArchiveTableCreator::getNumericTable($date);
             $this->performTimedPurging("Optimizing table $numericTable...", function () use ($numericTable) {
-                Db::optimizeTables($numericTable, $force = true);
+                Db\Schema::getInstance()->optimizeTables([$numericTable], $force = true);
             });
 
             $blobTable = ArchiveTableCreator::getBlobTable($date);
             $this->performTimedPurging("Optimizing table $blobTable...", function () use ($blobTable) {
-                Db::optimizeTables($blobTable, $force = true);
+                Db\Schema::getInstance()->optimizeTables([$blobTable], $force = true);
             });
         }
     }

diff --git a/plugins/CoreAdminHome/Tasks.php b/plugins/CoreAdminHome/Tasks.php
@@ -315,7 +315,7 @@ public function purgeInvalidatedArchives()
     public function optimizeArchiveTable()
     {
         $archiveTables = ArchiveTableCreator::getTablesArchivesInstalled();
-        Db::optimizeTables($archiveTables);
+        Db\Schema::getInstance()->optimizeTables($archiveTables);
     }
 
     /**

diff --git a/plugins/PrivacyManager/LogDataPurger.php b/plugins/PrivacyManager/LogDataPurger.php
@@ -105,7 +105,7 @@ public function purgeData($deleteLogsOlderThan, $deleteUnusedLogActions)
         Piwik::postEvent('PrivacyManager.deleteLogsOlderThan', array($dateUpperLimit, $deleteLogsOlderThan));
 
         // optimize table overhead after deletion
-        Db::optimizeTables($logTables);
+        Db\Schema::getInstance()->optimizeTables($logTables);
     }
 
     /**

diff --git a/plugins/PrivacyManager/ReportsPurger.php b/plugins/PrivacyManager/ReportsPurger.php
@@ -123,7 +123,7 @@ public function purgeData($optimize = false)
             }
 
             if ($optimize) {
-                Db::optimizeTables($oldBlobTables);
+                Db\Schema::getInstance()->optimizeTables($oldBlobTables);
             }
         }
 
@@ -152,7 +152,7 @@ public function purgeData($optimize = false)
             }
 
             if ($optimize) {
-                Db::optimizeTables($oldNumericTables);
+                Db\Schema::getInstance()->optimizeTables($oldNumericTables);
             }
         }
     }