From ab8ffbc8db1c05adfacdfbeb4c6fb15fc9b6dc9e Mon Sep 17 00:00:00 2001 From: Hugues Peccatte Date: Thu, 22 Aug 2019 23:36:53 +0200 Subject: [PATCH 1/4] =?UTF-8?q?Fix=20#14245=20Advisor=20fails=20on=20MySQL?= =?UTF-8?q?=20=E2=89=A58.0.3=20as=20many=20MySQL=20variables=20were=20remo?= =?UTF-8?q?ved?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As many MySQL variables were removed, we need to disable the rules using these variables. For this, a variable identifying MySQL had been created and is used with the MySQL version to check if the rule should be checked or not. Signed-off-by: Hugues Peccatte --- libraries/advisory_rules.txt | 14 +++++++------- libraries/classes/Advisor.php | 4 ++++ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/libraries/advisory_rules.txt b/libraries/advisory_rules.txt index 46446c0b8b..875a99b1d7 100644 --- a/libraries/advisory_rules.txt +++ b/libraries/advisory_rules.txt @@ -143,7 +143,7 @@ rule 'MySQL Architecture' # Query cache # Lame: 'ON' == 0 is true, so you need to compare 'ON' == '0' -rule 'Query cache disabled' +rule 'Query cache disabled' [IS_MYSQL == false || PMA_MYSQL_INT_VERSION < 80003] query_cache_size value == 0 || query_cache_type == 'OFF' || query_cache_type == '0' The query cache is not enabled. @@ -157,42 +157,42 @@ rule 'Query caching method' [!fired('Query cache disabled')] You are using the MySQL Query cache with a fairly high traffic database. It might be worth considering to use memcached instead of the MySQL Query cache, especially if you have multiple slaves. The query cache is enabled and the server receives %d queries per second. This rule fires if there is more than 100 queries per second. | round(value,1) -rule 'Query cache efficiency (%)' [Com_select + Qcache_hits > 0 && !fired('Query cache disabled')] +rule 'Query cache efficiency (%)' [(IS_MYSQL == false || PMA_MYSQL_INT_VERSION < 80003) && Com_select + Qcache_hits > 0 && !fired('Query cache disabled')] Qcache_hits / (Com_select + Qcache_hits) * 100 value < 20 Query cache not running efficiently, it has a low hit rate. Consider increasing {query_cache_limit}. The current query cache hit rate of %s% is below 20% | round(value,1) -rule 'Query Cache usage' [!fired('Query cache disabled')] +rule 'Query Cache usage' [(IS_MYSQL == false || PMA_MYSQL_INT_VERSION < 80003) && !fired('Query cache disabled')] 100 - Qcache_free_memory / query_cache_size * 100 value < 80 Less than 80% of the query cache is being utilized. This might be caused by {query_cache_limit} being too low. Flushing the query cache might help as well. The current ratio of free query cache memory to total query cache size is %s%. It should be above 80% | round(value,1) -rule 'Query cache fragmentation' [!fired('Query cache disabled')] +rule 'Query cache fragmentation' [(IS_MYSQL == false || PMA_MYSQL_INT_VERSION < 80003) && !fired('Query cache disabled')] Qcache_free_blocks / (Qcache_total_blocks / 2) * 100 value > 20 The query cache is considerably fragmented. Severe fragmentation is likely to (further) increase Qcache_lowmem_prunes. This might be caused by many Query cache low memory prunes due to {query_cache_size} being too small. For a immediate but short lived fix you can flush the query cache (might lock the query cache for a long time). Carefully adjusting {query_cache_min_res_unit} to a lower value might help too, e.g. you can set it to the average size of your queries in the cache using this formula: (query_cache_size - qcache_free_memory) / qcache_queries_in_cache The cache is currently fragmented by %s% , with 100% fragmentation meaning that the query cache is an alternating pattern of free and used blocks. This value should be below 20%. | round(value,1) -rule 'Query cache low memory prunes' [Qcache_inserts > 0 && !fired('Query cache disabled')] +rule 'Query cache low memory prunes' [(IS_MYSQL == false || PMA_MYSQL_INT_VERSION < 80003) && Qcache_inserts > 0 && !fired('Query cache disabled')] Qcache_lowmem_prunes / Qcache_inserts * 100 value > 0.1 Cached queries are removed due to low query cache memory from the query cache. You might want to increase {query_cache_size}, however keep in mind that the overhead of maintaining the cache is likely to increase with its size, so do this in small increments and monitor the results. The ratio of removed queries to inserted queries is %s%. The lower this value is, the better (This rules firing limit: 0.1%) | round(value,1) -rule 'Query cache max size' [!fired('Query cache disabled')] +rule 'Query cache max size' [(IS_MYSQL == false || PMA_MYSQL_INT_VERSION < 80003) && !fired('Query cache disabled')] query_cache_size value > 1024 * 1024 * 128 The query cache size is above 128 MiB. Big query caches may cause significant overhead that is required to maintain the cache. Depending on your environment, it might be performance increasing to reduce this value. Current query cache size: %s | ADVISOR_formatByteDown(value, 2, 2) -rule 'Query cache min result size' [!fired('Query cache disabled')] +rule 'Query cache min result size' [(IS_MYSQL == false || PMA_MYSQL_INT_VERSION < 80003) && !fired('Query cache disabled')] query_cache_limit value == 1024*1024 The max size of the result set in the query cache is the default of 1 MiB. diff --git a/libraries/classes/Advisor.php b/libraries/classes/Advisor.php index b9e0e6400f..68c8194454 100644 --- a/libraries/classes/Advisor.php +++ b/libraries/classes/Advisor.php @@ -221,6 +221,10 @@ class Advisor $this->variables['system_memory'] = isset($memory['MemTotal']) ? $memory['MemTotal'] : 0; + // Add IS_MYSQL to globals + $isMariaDB = false !== strpos($this->getVariables()['version'], 'MariaDB'); + $this->globals['IS_MYSQL'] = !$isMariaDB; + // Step 2: Read and parse the list of rules $this->setParseResult(static::parseRulesFile()); // Step 3: Feed the variables to the rules and let them fire. Sets From 99b2dfade4f57944df1cda02b2108c2f94f8b788 Mon Sep 17 00:00:00 2001 From: Hugues Peccatte Date: Fri, 23 Aug 2019 22:32:35 +0200 Subject: [PATCH 2/4] Split advisory rules files in many to manage variables removed in some versions Signed-off-by: Hugues Peccatte --- ...y_rules.txt => advisory_rules_generic.txt} | 50 ---------------- .../advisory_rules_mysql_before80003.txt | 57 +++++++++++++++++++ libraries/classes/Advisor.php | 40 +++++++++---- 3 files changed, 85 insertions(+), 62 deletions(-) rename libraries/{advisory_rules.txt => advisory_rules_generic.txt} (84%) create mode 100644 libraries/advisory_rules_mysql_before80003.txt diff --git a/libraries/advisory_rules.txt b/libraries/advisory_rules_generic.txt similarity index 84% rename from libraries/advisory_rules.txt rename to libraries/advisory_rules_generic.txt index 875a99b1d7..2a103ec012 100644 --- a/libraries/advisory_rules.txt +++ b/libraries/advisory_rules_generic.txt @@ -142,14 +142,6 @@ rule 'MySQL Architecture' # # Query cache -# Lame: 'ON' == 0 is true, so you need to compare 'ON' == '0' -rule 'Query cache disabled' [IS_MYSQL == false || PMA_MYSQL_INT_VERSION < 80003] - query_cache_size - value == 0 || query_cache_type == 'OFF' || query_cache_type == '0' - The query cache is not enabled. - The query cache is known to greatly improve performance if configured correctly. Enable it by setting {query_cache_size} to a 2 digit MiB value and setting {query_cache_type} to 'ON'. Note: If you are using memcached, ignore this recommendation. - query_cache_size is set to 0 or query_cache_type is set to 'OFF' - rule 'Query caching method' [!fired('Query cache disabled')] Questions / Uptime value > 100 @@ -157,48 +149,6 @@ rule 'Query caching method' [!fired('Query cache disabled')] You are using the MySQL Query cache with a fairly high traffic database. It might be worth considering to use memcached instead of the MySQL Query cache, especially if you have multiple slaves. The query cache is enabled and the server receives %d queries per second. This rule fires if there is more than 100 queries per second. | round(value,1) -rule 'Query cache efficiency (%)' [(IS_MYSQL == false || PMA_MYSQL_INT_VERSION < 80003) && Com_select + Qcache_hits > 0 && !fired('Query cache disabled')] - Qcache_hits / (Com_select + Qcache_hits) * 100 - value < 20 - Query cache not running efficiently, it has a low hit rate. - Consider increasing {query_cache_limit}. - The current query cache hit rate of %s% is below 20% | round(value,1) - -rule 'Query Cache usage' [(IS_MYSQL == false || PMA_MYSQL_INT_VERSION < 80003) && !fired('Query cache disabled')] - 100 - Qcache_free_memory / query_cache_size * 100 - value < 80 - Less than 80% of the query cache is being utilized. - This might be caused by {query_cache_limit} being too low. Flushing the query cache might help as well. - The current ratio of free query cache memory to total query cache size is %s%. It should be above 80% | round(value,1) - -rule 'Query cache fragmentation' [(IS_MYSQL == false || PMA_MYSQL_INT_VERSION < 80003) && !fired('Query cache disabled')] - Qcache_free_blocks / (Qcache_total_blocks / 2) * 100 - value > 20 - The query cache is considerably fragmented. - Severe fragmentation is likely to (further) increase Qcache_lowmem_prunes. This might be caused by many Query cache low memory prunes due to {query_cache_size} being too small. For a immediate but short lived fix you can flush the query cache (might lock the query cache for a long time). Carefully adjusting {query_cache_min_res_unit} to a lower value might help too, e.g. you can set it to the average size of your queries in the cache using this formula: (query_cache_size - qcache_free_memory) / qcache_queries_in_cache - The cache is currently fragmented by %s% , with 100% fragmentation meaning that the query cache is an alternating pattern of free and used blocks. This value should be below 20%. | round(value,1) - -rule 'Query cache low memory prunes' [(IS_MYSQL == false || PMA_MYSQL_INT_VERSION < 80003) && Qcache_inserts > 0 && !fired('Query cache disabled')] - Qcache_lowmem_prunes / Qcache_inserts * 100 - value > 0.1 - Cached queries are removed due to low query cache memory from the query cache. - You might want to increase {query_cache_size}, however keep in mind that the overhead of maintaining the cache is likely to increase with its size, so do this in small increments and monitor the results. - The ratio of removed queries to inserted queries is %s%. The lower this value is, the better (This rules firing limit: 0.1%) | round(value,1) - -rule 'Query cache max size' [(IS_MYSQL == false || PMA_MYSQL_INT_VERSION < 80003) && !fired('Query cache disabled')] - query_cache_size - value > 1024 * 1024 * 128 - The query cache size is above 128 MiB. Big query caches may cause significant overhead that is required to maintain the cache. - Depending on your environment, it might be performance increasing to reduce this value. - Current query cache size: %s | ADVISOR_formatByteDown(value, 2, 2) - -rule 'Query cache min result size' [(IS_MYSQL == false || PMA_MYSQL_INT_VERSION < 80003) && !fired('Query cache disabled')] - query_cache_limit - value == 1024*1024 - The max size of the result set in the query cache is the default of 1 MiB. - Changing {query_cache_limit} (usually by increasing) may increase efficiency. This variable determines the maximum size a query result may have to be inserted into the query cache. If there are many query results above 1 MiB that are well cacheable (many reads, little writes) then increasing {query_cache_limit} will increase efficiency. Whereas in the case of many query results being above 1 MiB that are not very well cacheable (often invalidated due to table updates) increasing {query_cache_limit} might reduce efficiency. - query_cache_limit is set to 1 MiB - # # Sorts rule 'Percentage of sorts that cause temporary tables' [Sort_scan + Sort_range > 0] diff --git a/libraries/advisory_rules_mysql_before80003.txt b/libraries/advisory_rules_mysql_before80003.txt new file mode 100644 index 0000000000..e28b7f41d3 --- /dev/null +++ b/libraries/advisory_rules_mysql_before80003.txt @@ -0,0 +1,57 @@ +# phpMyAdmin Advisory rules file +# +# See doc in advisory_rules_generic.txt +# + +# +# Query cache + +# Lame: 'ON' == 0 is true, so you need to compare 'ON' == '0' +rule 'Query cache disabled' + query_cache_size + value == 0 || query_cache_type == 'OFF' || query_cache_type == '0' + The query cache is not enabled. + The query cache is known to greatly improve performance if configured correctly. Enable it by setting {query_cache_size} to a 2 digit MiB value and setting {query_cache_type} to 'ON'. Note: If you are using memcached, ignore this recommendation. + query_cache_size is set to 0 or query_cache_type is set to 'OFF' + +rule 'Query cache efficiency (%)' [Com_select + Qcache_hits > 0 && !fired('Query cache disabled')] + Qcache_hits / (Com_select + Qcache_hits) * 100 + value < 20 + Query cache not running efficiently, it has a low hit rate. + Consider increasing {query_cache_limit}. + The current query cache hit rate of %s% is below 20% | round(value,1) + +rule 'Query Cache usage' [!fired('Query cache disabled')] + 100 - Qcache_free_memory / query_cache_size * 100 + value < 80 + Less than 80% of the query cache is being utilized. + This might be caused by {query_cache_limit} being too low. Flushing the query cache might help as well. + The current ratio of free query cache memory to total query cache size is %s%. It should be above 80% | round(value,1) + +rule 'Query cache fragmentation' [!fired('Query cache disabled')] + Qcache_free_blocks / (Qcache_total_blocks / 2) * 100 + value > 20 + The query cache is considerably fragmented. + Severe fragmentation is likely to (further) increase Qcache_lowmem_prunes. This might be caused by many Query cache low memory prunes due to {query_cache_size} being too small. For a immediate but short lived fix you can flush the query cache (might lock the query cache for a long time). Carefully adjusting {query_cache_min_res_unit} to a lower value might help too, e.g. you can set it to the average size of your queries in the cache using this formula: (query_cache_size - qcache_free_memory) / qcache_queries_in_cache + The cache is currently fragmented by %s% , with 100% fragmentation meaning that the query cache is an alternating pattern of free and used blocks. This value should be below 20%. | round(value,1) + +rule 'Query cache low memory prunes' [Qcache_inserts > 0 && !fired('Query cache disabled')] + Qcache_lowmem_prunes / Qcache_inserts * 100 + value > 0.1 + Cached queries are removed due to low query cache memory from the query cache. + You might want to increase {query_cache_size}, however keep in mind that the overhead of maintaining the cache is likely to increase with its size, so do this in small increments and monitor the results. + The ratio of removed queries to inserted queries is %s%. The lower this value is, the better (This rules firing limit: 0.1%) | round(value,1) + +rule 'Query cache max size' [!fired('Query cache disabled')] + query_cache_size + value > 1024 * 1024 * 128 + The query cache size is above 128 MiB. Big query caches may cause significant overhead that is required to maintain the cache. + Depending on your environment, it might be performance increasing to reduce this value. + Current query cache size: %s | ADVISOR_formatByteDown(value, 2, 2) + +rule 'Query cache min result size' [!fired('Query cache disabled')] + query_cache_limit + value == 1024*1024 + The max size of the result set in the query cache is the default of 1 MiB. + Changing {query_cache_limit} (usually by increasing) may increase efficiency. This variable determines the maximum size a query result may have to be inserted into the query cache. If there are many query results above 1 MiB that are well cacheable (many reads, little writes) then increasing {query_cache_limit} will increase efficiency. Whereas in the case of many query results being above 1 MiB that are not very well cacheable (often invalidated due to table updates) increasing {query_cache_limit} might reduce efficiency. + query_cache_limit is set to 1 MiB diff --git a/libraries/classes/Advisor.php b/libraries/classes/Advisor.php index 68c8194454..5070885178 100644 --- a/libraries/classes/Advisor.php +++ b/libraries/classes/Advisor.php @@ -221,12 +221,15 @@ class Advisor $this->variables['system_memory'] = isset($memory['MemTotal']) ? $memory['MemTotal'] : 0; - // Add IS_MYSQL to globals - $isMariaDB = false !== strpos($this->getVariables()['version'], 'MariaDB'); - $this->globals['IS_MYSQL'] = !$isMariaDB; + $ruleFiles = $this->defineRulesFiles(); // Step 2: Read and parse the list of rules - $this->setParseResult(static::parseRulesFile()); + $parsedResults = []; + foreach ($ruleFiles as $ruleFile) { + $parsedResults[] = $this->parseRulesFile($ruleFile); + } + $this->setParseResult(call_user_func_array('array_merge_recursive', $parsedResults)); + // Step 3: Feed the variables to the rules and let them fire. Sets // $runResult $this->runRules(); @@ -433,6 +436,22 @@ class Advisor $this->runResult[$type][] = $rule; } + /** + * Defines the rules files to use + * + * @return array + */ + protected function defineRulesFiles() + { + $isMariaDB = false !== strpos($this->getVariables()['version'], 'MariaDB'); + $ruleFiles = ['libraries/advisory_rules_generic.txt']; + // If MariaDB (= not MySQL) OR MYSQL < 8.0.3, add another rules file. + if ($isMariaDB || $this->globals['PMA_MYSQL_INT_VERSION'] < 80003) { + $ruleFiles[] = 'libraries/advisory_rules_mysql_before80003.txt'; + } + return $ruleFiles; + } + /** * Callback for wrapping links with Core::linkURL * @@ -484,23 +503,23 @@ class Advisor * Reads the rule file into an array, throwing errors messages on syntax * errors. * + * @param string $filename Name of file to parse + * * @return array with parsed data */ - public static function parseRulesFile() + public static function parseRulesFile($filename) { - $filename = 'libraries/advisory_rules.txt'; $file = file($filename, FILE_IGNORE_NEW_LINES); $errors = array(); $rules = array(); - $lines = array(); if ($file === false) { $errors[] = sprintf( __('Error in reading file: The file \'%s\' does not exist or is not readable!'), $filename ); - return array('rules' => $rules, 'lines' => $lines, 'errors' => $errors); + return array('rules' => $rules, 'errors' => $errors); } $ruleSyntax = array( @@ -534,10 +553,8 @@ class Advisor $ruleLine = 1; $ruleNo++; $rules[$ruleNo] = array('name' => $match[1]); - $lines[$ruleNo] = array('name' => $i + 1); if (isset($match[3])) { $rules[$ruleNo]['precondition'] = $match[3]; - $lines[$ruleNo]['precondition'] = $i + 1; } } else { $errors[] = sprintf( @@ -575,7 +592,6 @@ class Advisor $rules[$ruleNo][$ruleSyntax[$ruleLine]] = chop( mb_substr($line, 1) ); - $lines[$ruleNo][$ruleSyntax[$ruleLine]] = $i + 1; ++$ruleLine; } @@ -585,7 +601,7 @@ class Advisor } } - return array('rules' => $rules, 'lines' => $lines, 'errors' => $errors); + return array('rules' => $rules, 'errors' => $errors); } /** From efbec55643dabd38de1dd148d417a6c1cde7463a Mon Sep 17 00:00:00 2001 From: Hugues Peccatte Date: Fri, 23 Aug 2019 22:40:34 +0200 Subject: [PATCH 3/4] Fix tests regarding Advisor class changes Signed-off-by: Hugues Peccatte --- libraries/classes/Advisor.php | 7 +++++-- test/classes/AdvisorTest.php | 4 ++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/libraries/classes/Advisor.php b/libraries/classes/Advisor.php index 5070885178..8c5903e331 100644 --- a/libraries/classes/Advisor.php +++ b/libraries/classes/Advisor.php @@ -23,6 +23,9 @@ use Symfony\Component\ExpressionLanguage\ExpressionLanguage; */ class Advisor { + const GENERIC_RULES_FILE = 'libraries/advisory_rules_generic.txt'; + const BEFORE_MYSQL80003_RULES_FILE = 'libraries/advisory_rules_mysql_before80003.txt'; + protected $dbi; protected $variables; protected $globals; @@ -444,10 +447,10 @@ class Advisor protected function defineRulesFiles() { $isMariaDB = false !== strpos($this->getVariables()['version'], 'MariaDB'); - $ruleFiles = ['libraries/advisory_rules_generic.txt']; + $ruleFiles = [self::GENERIC_RULES_FILE]; // If MariaDB (= not MySQL) OR MYSQL < 8.0.3, add another rules file. if ($isMariaDB || $this->globals['PMA_MYSQL_INT_VERSION'] < 80003) { - $ruleFiles[] = 'libraries/advisory_rules_mysql_before80003.txt'; + $ruleFiles[] = self::BEFORE_MYSQL80003_RULES_FILE; } return $ruleFiles; } diff --git a/test/classes/AdvisorTest.php b/test/classes/AdvisorTest.php index 219428dc4b..569b639efc 100644 --- a/test/classes/AdvisorTest.php +++ b/test/classes/AdvisorTest.php @@ -70,7 +70,7 @@ class AdvisorTest extends PmaTestCase public function testParse() { $advisor = new Advisor($GLOBALS['dbi'], new ExpressionLanguage()); - $parseResult = $advisor->parseRulesFile(); + $parseResult = $advisor->parseRulesFile(Advisor::GENERIC_RULES_FILE); $this->assertEquals($parseResult['errors'], array()); } @@ -127,7 +127,7 @@ class AdvisorTest extends PmaTestCase public function testAddRule($rule, $expected, $error) { $advisor = new Advisor($GLOBALS['dbi'], new ExpressionLanguage()); - $parseResult = $advisor->parseRulesFile(); + $parseResult = $advisor->parseRulesFile(Advisor::GENERIC_RULES_FILE); $this->assertEquals($parseResult['errors'], array()); $advisor->setVariable('value', 0); $advisor->addRule('fired', $rule); From 01c52f5fc5db61fd288b8a77a65fc22b7574d6c4 Mon Sep 17 00:00:00 2001 From: Hugues Peccatte Date: Sat, 31 Aug 2019 11:18:59 +0200 Subject: [PATCH 4/4] Fix AdvisorTest Signed-off-by: Hugues Peccatte --- test/classes/Server/Status/AdvisorTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/test/classes/Server/Status/AdvisorTest.php b/test/classes/Server/Status/AdvisorTest.php index 6b27d715a9..9fcc93d535 100644 --- a/test/classes/Server/Status/AdvisorTest.php +++ b/test/classes/Server/Status/AdvisorTest.php @@ -82,6 +82,7 @@ class AdvisorTest extends TestCase "automatic_sp_privileges" => "ON", "back_log" => "50", "big_tables" => "OFF", + 'version' => '8.0.2' ); $fetchResult = array(