Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance issue with table existence query #7049

Closed
wordpressfan opened this issue Oct 22, 2024 · 5 comments · Fixed by #7050
Closed

Performance issue with table existence query #7049

wordpressfan opened this issue Oct 22, 2024 · 5 comments · Fixed by #7050
Assignees
Labels
effort: [XS] < 1 day of estimated development time priority: medium Issues which are important, but no one will go out of business. severity: moderate Feature isn't working as expected but has work around to get same value
Milestone

Comments

@wordpressfan
Copy link
Contributor

Describe the bug
In the PR #6968 we started using information_schema.tables to detect the existence of our tables, exactly here:

$query = 'SELECT table_name FROM information_schema.tables WHERE table_name = %s LIMIT 1';

Then we got a report that this query will return values for all databases on the server even for databases that the user doesn't have access to.

To Reproduce
Steps to reproduce the behavior:

I don't have a clear way to reproduce the issue on our local sites but seems like we may need to create two mysql users on the same server and for each user, create a database, and see if the query will return values from all databases.

Expected behavior
We should only detect the table existence on the current database that we have proper access to.

Possible solution

we already use the same concept in database optimization query, here:

$count = $wpdb->get_var( "SELECT COUNT(table_name) FROM information_schema.tables WHERE table_schema = '" . DB_NAME . "' and Engine <> 'InnoDB' and data_free > 0" );

@piotrbak piotrbak added priority: medium Issues which are important, but no one will go out of business. needs: grooming severity: moderate Feature isn't working as expected but has work around to get same value labels Oct 22, 2024
@jeawhanlee jeawhanlee self-assigned this Oct 22, 2024
@piotrbak piotrbak added this to the 3.17.2 milestone Oct 22, 2024
@remyperona remyperona added effort: [XS] < 1 day of estimated development time and removed needs: grooming labels Oct 22, 2024
@chrisdavidmiles
Copy link

Hey original reporter here, seeing the issue for Bluehost, Hostgator, and a few others.

To add some context, the query will make an attempt to gather values for all databases on a server, even for databases it doesn't own. While it doesn't get the data, checking permissions and failing on every DB is time consuming and resource intensive, especially on Shared hosting environments. The result is performance impact not only for the site the plugin is running on, but for every site across the server.

More details around this behavior (and optimization recommendations) can be found at https://dev.mysql.com/doc/refman/5.7/en/information-schema-optimization.html

To replicate, as far as I can tell, this needs to be tested with an user that doesn't have 'SUPER' privileges. (With SUPER it doesn't do the same).

Or if your test environment only had a few users for testing each with just a few DBs, it would be difficult to see the issue.

@chrisdavidmiles
Copy link

chrisdavidmiles commented Oct 22, 2024

The issue only started on the latest version 3.17.1 but it's causing enough performance degradation in shared hosting environments with many users and DBs that we're looking at patching existing sites ourselves. I suspect other hosts may take more aggressive remediation efforts.

For reference:

3.17.0.2 (previous version, no issue)
wp-content/plugins/wp-rocket/inc/Engine/Admin/Database/Optimization.php:        $count = $wpdb->get_var( "SELECT COUNT(table_name) FROM information_schema.tables WHERE table_schema = '" . DB_NAME . "' and Engine <> 'InnoDB' and data_free > 0" );
wp-content/plugins/wp-rocket/inc/Engine/Admin/Database/OptimizationProcess.php:        $query = $wpdb->get_results( "SELECT table_name, data_free FROM information_schema.tables WHERE table_schema = '" . DB_NAME . "' and Engine <> 'InnoDB' and data_free > 0" );
3.17.1 (latest version, creating performance issues)
wp-content/plugins/wp-rocket/inc/Engine/Admin/Database/Optimization.php:        $count = $wpdb->get_var( "SELECT COUNT(table_name) FROM information_schema.tables WHERE table_schema = '" . DB_NAME . "' and Engine <> 'InnoDB' and data_free > 0" );
wp-content/plugins/wp-rocket/inc/Engine/Admin/Database/OptimizationProcess.php:        $query = $wpdb->get_results( "SELECT table_name, data_free FROM information_schema.tables WHERE table_schema = '" . DB_NAME . "' and Engine <> 'InnoDB' and data_free > 0" );
wp-content/plugins/wp-rocket/inc/Engine/Common/Database/Queries/AbstractQuery.php:    $query    = 'SELECT table_name FROM information_schema.tables WHERE table_name = %s LIMIT 1';
wp-content/plugins/wp-rocket/inc/Engine/Common/PerformanceHints/Database/Queries/AbstractQueries.php:    $query    = 'SELECT table_name FROM information_schema.tables WHERE table_name = %s LIMIT 1';

@piotrbak
Copy link
Contributor

Hello @chrisdavidmiles, thanks for your input here. Do you see any possibility to test this PR on your environment? Just to confirm that it fixes the problem on your end.

In the meantime we'll be running our tests, of course.

@chrisdavidmiles
Copy link

We will test this patch and report back. Thanks!

@chrisdavidmiles
Copy link

chrisdavidmiles commented Oct 31, 2024

Thank you @piotrbak. A few hours after the performance issue was introduced in v3.17.1, Bluehost created a patch and rolled it out to their sites to maintain performance. The proposed wp-rocket patch here 9f3b827 seems to take the same approach as we did.

Your patch looks good! Thanks for responding to this and providing a solution to our mutual customers. Looking forward to seeing this in v3.17.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort: [XS] < 1 day of estimated development time priority: medium Issues which are important, but no one will go out of business. severity: moderate Feature isn't working as expected but has work around to get same value
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants