Allow configuration of max execution time when using MariaDB

[Findus23]

To provide feature equality between MySQL and MariaDB the same feature as implemented in #14858 (setting a max_execution_time for live queries) should also be supported when using MariaDB.

MariaDB uses a different syntax and seconds instead of milliseconds, but apart from that, the feature should work the same.

See https://mariadb.com/kb/en/library/aborting-statements/ for more information about this.

ping @tomper00 and @jorgeuos

[jorgeuos]

Hi,

Thanks for the ping, Here's a working patch that I haven't had time to create a PR for yet. But you could try if you'd like.

Tested with Matomo <= 4.3.1-4.5.0:

diff --git a/config/global.ini.php b/config/global.ini.php
index 5d18309b0f..a448298678 100755
--- a/config/global.ini.php
+++ b/config/global.ini.php
@@ -385,6 +385,13 @@ archiving_custom_ranges[] =
 ; This feature will not work with the MYSQLI extension.
 archiving_query_max_execution_time = 7200
 
+; Like stated above and on comments on Github, Above conf will not work for MariaDB.
+; This will add an simple if/else and use MariaDB specific
+; as mentioned in the comments: https://github.com/matomo-org/matomo/pull/14858#issuecomment-529997479
+; Change to 1 for MariaDB
+; Will also be useful for live_query_max_execution_time
+use_max_statement_time_for_mariadb = 0
+
 ; By default Matomo runs OPTIMIZE TABLE SQL queries to free spaces after deleting some data.
 ; If your Matomo tracks millions of pages, the OPTIMIZE TABLE queries might run for hours (seen in "SHOW FULL PROCESSLIST \g")
 ; so you can disable these special queries here:
diff --git a/core/DbHelper.php b/core/DbHelper.php
index ea0b4e5e4e..2d2ef7d5bd 100644
--- a/core/DbHelper.php
+++ b/core/DbHelper.php
@@ -283,6 +283,9 @@ class DbHelper
 
     /**
      * Adds a MAX_EXECUTION_TIME hint into a SELECT query if $limit is bigger than 1
+     * NOTE: The SELECT MAX_STATEMENT_TIME = N ... syntax is not valid in MariaDB.
+     * We need to use max_statement_time in conjunction with SET STATEMENT.
+     * And seconds instead of milliseconds.
      *
      * @param string $sql  query to add hint to
      * @param int $limit  time limit in seconds
@@ -297,14 +300,21 @@ class DbHelper
         $sql = trim($sql);
         $pos = stripos($sql, 'SELECT');
         if ($pos !== false) {
-
-            $timeInMs = $limit * 1000;
-            $timeInMs = (int) $timeInMs;
-            $maxExecutionTimeHint = ' /*+ MAX_EXECUTION_TIME('.$timeInMs.') */ ';
-
-            $sql = substr_replace($sql, 'SELECT ' . $maxExecutionTimeHint, $pos, strlen('SELECT'));
+            
+            $useMaxStatementTime = (int) Config::getInstance()->General['use_max_statement_time_for_mariadb'];
+
+            // Fix for MariaDB support
+            if($useMaxStatementTime == 1){
+                $maxExecutionTimeHint = 'SET STATEMENT max_statement_time='.$limit.' FOR ';
+                $sql = substr_replace($sql, $maxExecutionTimeHint . 'SELECT', $pos, strlen('SELECT'));
+            }
+            else{
+                $timeInMs = $limit * 1000;
+                $timeInMs = (int) $timeInMs;
+                $maxExecutionTimeHint = ' /*+ MAX_EXECUTION_TIME('.$timeInMs.') */ ';
+                $sql = substr_replace($sql, 'SELECT ' . $maxExecutionTimeHint, $pos, strlen('SELECT'));
+            }
         }
-
         return $sql;
     }

Br, Jorge

[sgiehl]

Hi @jorgeuos
Thanks for the diff. Guess we can use that to provide a proper fix for this for future releases

[tsteur]

Note that some other logic might check if a query starts with select and then add comments about the type of query etc which then might not work anymore. Also this one seems to require a specific MAX_STATEMENT_TIME permission.

For now we might just want to add a sentence eventually that MariaDB is not supported.

@jorgeuos if you provide a PR we'll be happy to review.

[sgiehl]

As there might be other features somewhen that might have special handling, we maybe should make it possible to have such things more global. Maybe like moving those method(s) to the database schema class and having an additional mariadb schema, that overwrites that specific method(s). That way it could be defined in the config with eg [database] schema = mariadb and maybe directly configured when setting up Matomo.

[sgiehl]

I have done the changes needed for what I had in mind. Don't really have time to tests or finish that, but thought I'll push it upstream so someone is able to pick it up eventually. See #18371

[fritzgithub]

Thanks for bringing this up. We are using MariaDB and would be happy to be able to solve frequent execution time issues.

[mattab]

@fritzgithub Thanks for your feedback. I have a follow up question, are you still using MariaDB with Matomo and if so, are you still getting regular error messages due to this bug?

[fritzgithub]

... are you still using MariaDB with Matomo and if so, are you still getting regular error messages due to this bug?

Yes, still MariaDB. And no, from my user experience (not logs), i do not remember execution time issues for quite some time.

[sgiehl]

Thanks for the feedback @fritzgithub. I guess it might still be worth to consider improving the mariadb support for such features, as mariadb is widely used.