Rewrote Magento\Framework\DB\Adapter\Pdo\Mysql->isTableExists #28516

jonashrem · 2020-06-04T15:42:31Z

Description

During heavier load on newer productive Magento2 installations (2.3 with MSI or 2.3.4+), we noticed increased execution time was spent in SHOW TABLE STATUS queries to MySQL. Analyzing this further, we found that these queries seemed to occur in higher frequency than in earlier Magento versions. This can be problematic, since the MySQL server uses an exclusive file system lock and a call to readdir() to answer them. Because of this, these calls can be slow. We also found that one web request can result in multiple calls of SHOW TABLE STATUS for the same tables, which seems very wasteful and inefficient.

In more detailed analysis of some of our customers, we found instances where SHOW TABLE STATUS queries were executed nearly 100 times when requesting a single category page with 48 products.

Looking at the code of the MySQL adapter, we identified several places that use the function showTablesStatus(), among them isTableExists().
With this patch, we eliminate the calls of SHOW TABLE STATUS for the common case of checking table existence.

Implementation details

In our implementation of isTableExists(), we use information from the INFORMATION_SCHEMA.TABLES table, which is provided without expensive file system operations and proved to be significantly faster in our tests (see below for details). Additionally, we use the already existing DDL cache to store a positive table existence, resulting in even fewer database calls.

According to the official MySQL documentation[link], using the views from INFORMATION_SCHEMA is actually recommended over using SHOW queries, since this has some advantages. INFORMATION_SCHEMA views are read-only and respect the access rights of the database user, so it should be readily available on supported MySQL platforms.

The raw performance advantages of using INFORMATION_SCHEMA over SHOW TABLES STATUS can easily be measured with MySQL itself. We’ve used MySQL 5.7.30 on Ubuntu 18.04, with datadir running on a high performance NVME SSD (Samsung PM1725b).
We’ve run 10 concurrent clients each executing 1,000,000 statements in a Magento database with demo data, once with the query SHOW TABLES STATUS LIKE ‘catalog_product_entity’; and once with our statement SELECT COUNT(1) AS tbl_exists FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_NAME = ‘catalog_product_entity’ AND TABLE_SCHEMA = DATABASE();.

Between these queries, we measured a difference of slightly over factor 10 in the execution time, favoring our solution with INFORMATION_SCHEMA. See below for details.
We assume the difference is even bigger when having more concurrent clients or slower hardware. As MySQL’s “SHOW TABLE STATUS” implementation is reading the whole directory, file system locking can have a stronger effect as well as which file system is used.

Our performance measurements:

create the files test1.sql and test2.sql with 1.000.000 rows of the statements to test.
run these commands
- time seq 1 10 | parallel -j 10 bash -c 'mysql < test1.sql > /dev/null’
- time seq 1 10 | parallel -j 10 bash -c 'mysql < test2.sql > /dev/null’

Results:
For SHOW TABLE STATUS:

# time seq 1 10 |  parallel  -j 10 bash -c ' mysql   < test1.sql  > /dev/null’ 

real	85m21.374s
user	3m5.500s
sys	1m40.317s

For INFORMATION_SCHEMA:

time seq 1 10 |  parallel  -j 10 bash -c ' mysql   < test2.sql > /dev/null 


real	8m5.184s
user	1m54.587s
sys	1m11.687s

Manual testing scenarios

To see the impact our patch has, load the same set of pages on the frontend with and without the patch on any Magento instance (with real or demo data). The difference can be seen when analyzing the amount of SQL queries sent to the database and the time spent processing them. Here’s an easy way to do this:

Using Slow Query Log, you can make MySQL actually log all queries by setting the threshold to 0 seconds:

SET @@GLOBAL.long_query_time = 0; SET @@GLOBAL.slow_query_log = 1;

After you’ve opened the selected set of frontend pages, deactivate it again:

SET @@GLOBAL.slow_query_log = 0;

Now, using pt-query-digest from the Percona Toolkit on the slow query log file, you get an analysis of the logged queries, with queries grouped by type, their average and cumulative run time, and other information. Make sure to use two different log files so that they can easily be compared between M2 with and without our patch.

In the analysis, you will usually see SHOW TABLE STATUS queries appearing in the top queries (this depends a bit on the module configuration and of course on which and how many pages you accessed for this test), meaning they're taking quite a good percentage of total execution time.

Please see these extracts from the pt-query-digest output for an example, which we created when simply opening the single category page “Men > Tops” from the demo data (https://$BASEURL/men/tops-men.html?product_list_limit=36).

Analysis of SQL queries run, unpatched Magento:

# Profile
# Rank Query ID                         Response time Calls R/Call V/M   I
# ==== ================================ ============= ===== ====== ===== =
#    1 0xAADD55611E492BA965AF2F067A0...  0.0099 34.3%    22 0.0004  0.00 SELECT catalog_product_index_eav search_tmp_?ed?d?dcc?b?_? cataloginventory_stock_status
#    2 0x7009A071F49236A84A57562D461...  0.0050 17.2%     1 0.0050  0.00 SELECT catalog_product_entity catalog_category_product_index_store? catalog_product_index_price review_entity_summary review_entity cataloginventory_stock_status search_tmp_?ed?d?dccc?_?
#    3 0x8085D806F3631D0D30FE5C20326...  0.0036 12.5%     3 0.0012  0.00 SHOW TABLE STATUS
#    4 0x13262D434B03958CB4F62EC706B...  0.0011  3.9%     1 0.0011  0.00 INSERT SELECT search_tmp_?ed?d?dcc?b?_? catalog_product_index_eav cataloginventory_stock_status catalog_category_product_index_store?
#    5 0x2F9EDAB00084ED0A7EE1463DBA4...  0.0009  3.1%    72 0.0000  0.00 SELECT inventory_stock_sales_channel
[...]

You can see that queries of the type SHOW TABLE STATUS are the third most expensive queries with 12.5% of the total execution time inside the database and 3 calls in total just for this single page load.

Patched Magento, opening the same category page:

# Profile
# Rank Query ID                         Response time Calls R/Call V/M   I
# ==== ================================ ============= ===== ====== ===== =
#    1 0xAADD55611E492BA965AF2F067A0...  0.0105 29.9%    22 0.0005  0.00 SELECT catalog_product_index_eav search_tmp_?ed?cf?f?c?a?_? cataloginventory_stock_status
#    2 0x8EA4DF263A535F59EAC6ED75B20...  0.0073 20.7%    24 0.0003  0.00 SELECT catalog_product_entity catalog_product_super_link catalog_product_website catalog_product_entity_int cataloginventory_stock_status
#    3 0x7009A071F49236A84A57562D461...  0.0050 14.2%     1 0.0050  0.00 SELECT catalog_product_entity catalog_category_product_index_store? catalog_product_index_price review_entity_summary review_entity cataloginventory_stock_status search_tmp_?ed?cf?f?e?b?_?
#    4 0x13262D434B03958CB4F62EC706B...  0.0015  4.3%     1 0.0015  0.00 INSERT SELECT search_tmp_?ed?cf?f?c?a?_? catalog_product_index_eav cataloginventory_stock_status catalog_category_product_index_store?
#    5 0xCB3E7FA2C499AC0B69ACCD06C21...  0.0012  3.4%     2 0.0006  0.00 SELECT catalog_product_index_price search_tmp_?ed?cf?f?c?a?_?
#    6 0x2F9EDAB00084ED0A7EE1463DBA4...  0.0009  2.7%    72 0.0000  0.00 SELECT inventory_stock_sales_channel
#    7 0x593763BE686057BEA87DB0BC232...  0.0008  2.4%    72 0.0000  0.00 SELECT inventory_stock_sales_channel
#    8 0x23B42F57B465E32E1732E4070E9...  0.0008  2.2%    72 0.0000  0.00 SELECT inventory_stock
#    9 0x11C6E1A6D7347EEAF29EBEA1350...  0.0008  2.2%     1 0.0008  0.00 SELECT catalog_product_index_eav cataloginventory_stock_status catalog_category_product_index_store?
#   10 0xA788DCC6FE9679C3FDABA74662A...  0.0007  1.9%     1 0.0007  0.00 SELECT catalog_product_entity catalog_category_product_index_store? catalog_product_index_price cataloginventory_stock_status search_tmp_?ed?cf?f?e?b?_?
#   11 0x961084F55005407CD1C1E76C0BE...  0.0006  1.7%     2 0.0003  0.00 SELECT eav_attribute_option_swatch
#   12 0xE29FDCAD0B27AFD15D17697A415...  0.0005  1.5%     1 0.0005  0.00 SELECT catalog_product_index_price search_tmp_?ed?cf?f?c?a?_?
#   13 0x233B56CFFF59C63BF02E08D9654...  0.0005  1.4%     2 0.0002  0.00 CREATE TABLE search_tmp_?ed?cf?f?e?b?_? `search_tmp_5ed8cf6f89e2b9_26325963`
#   14 0x2F0A3214F96ED5CD4E962B1E990...  0.0005  1.3%    24 0.0000  0.00 SELECT catalog_product_super_attribute catalog_product_entity catalog_product_super_link eav_attribute catalog_product_entity catalog_product_entity_int catalog_product_super_attribute_label eav_attribute_option
#   15 0xDD45A442CF95F62A11D72D7D0F8...  0.0004  1.2%     1 0.0004  0.00 SELECT catalog_category_product_index_store? search_tmp_?ed?cf?f?c?a?_? catalog_category_entity
#   16 0x616C07D9410BE0BBA2EBCD6D77C...  0.0003  0.8%    21 0.0000  0.00 SELECT eav_attribute
#   17 0xB3E930152F4418A1C656779CE44...  0.0003  0.7%    12 0.0000  0.00 SELECT catalog_product_super_attribute
#   18 0xDFE08A11F72783E309DFA58BA8F...  0.0003  0.7%    12 0.0000  0.00 SELECT catalog_category_product
#   19 0x58EA3033F7E981C7FACDF0DE8D3...  0.0002  0.7%    20 0.0000  0.00 SELECT catalog_eav_attribute
#   20 0x13867189A41845FE8E8C4366F7B...  0.0002  0.5%    12 0.0000  0.00 SELECT catalog_product_super_attribute_label
# MISC 0xMISC                            0.0020  5.6%    74 0.0000   0.0 <38 ITEMS>

We’ve had to include the full summary in this case to show that our implementation not only eliminates the common cause for SHOW TABLE STATUS queries, but also that the new queries to INFORMATION_SCHEMA.TABLES do not occur in the top 20 queries.
This already illustrates that we’ve found a good solution to improve the performance of table existence checks. The complete impact can be estimated from observing production stores with many concurrent page views, where we see thousands of SHOW TABLE STATUS queries in 2–3 minutes.

Questions or comments

The commit in this pull request is a combined work of @jonashrem and @leeps.
We haven’t created an issue for this improvement, but can do that if it’s needed/wanted.

Contribution checklist

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds are green)

Resolved issues:

resolves [Issue] Rewrote Magento\Framework\DB\Adapter\Pdo\Mysql->isTableExists #29662: Rewrote Magento\Framework\DB\Adapter\Pdo\Mysql->isTableExists

* Use INFORMATION_SCHEMA.TABLES instead of "SHOW TABLE STATUS" * Extend DDL cache for table existence and use it in isTableExists

m2-assistant · 2020-06-04T15:42:34Z

Hi @jonashrem. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento give me test instance - deploy test instance based on PR changes
@magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

@magento run all tests - run or re-run all required tests against the PR changes
@magento run <test-build(s)> - run or re-run specific test build(s)
For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

Database Compare
Functional Tests CE
Functional Tests EE,
Functional Tests B2B
Integration Tests
Magento Health Index
Sample Data Tests CE
Sample Data Tests EE
Sample Data Tests B2B
Static Tests
Unit Tests
WebAPI Tests

You can find more information about the builds here

ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review.

For more details, please, review the Magento Contributor Guide documentation.

jonashrem · 2020-06-04T15:59:39Z

@magento run all tests

* Update Magento\Framework\DB\Adapter\Pdo\Mysql->isTableExists() to conform to code style guidelines (multi-line function calls, line length)

jonashrem · 2020-06-04T17:16:57Z

@magento run Static Tests

to conform to code style guidelines (line length)

jonashrem · 2020-06-04T18:01:25Z

@magento run all tests

jonashrem · 2020-06-04T22:14:12Z

@magento run Integration Tests

aleron75 · 2020-06-05T06:31:37Z

The failing integration test doesn't seem related to this PR.
The affected method is used in a lot of tests so to me this satisfies test coverage.
Thus I approved this PR.

magento-engcom-team · 2020-06-05T06:32:44Z

Hi @aleron75, thank you for the review.
ENGCOM-7622 has been created to process this Pull Request
✳️ @aleron75, could you please add one of the following labels to the Pull Request?

Label	Description
`Auto-Tests: Covered`	All changes in Pull Request is covered by auto-tests
`Auto-Tests: Not Covered`	Changes in Pull Request requires coverage by auto-tests
`Auto-Tests: Not Required`	Changes in Pull Request does not require coverage by auto-tests

jonashrem · 2020-06-09T16:05:00Z

We tested the failing Integration test on 2.3.5-p1 and it didn't fail there, so I agree that this isn't related to our change.

Pull Request state was updated. Re-review required.

ihor-sviziev

Integration test fails, even after restarting few times. Looks like your change caused regression in some specific case - image resize command fails in case if DB storage used.

@jonashrem could you try to run following test locally?

magento2/dev/tests/integration/testsuite/Magento/MediaStorage/Console/Command/ImageResizeCommandTest.php

Lines 120 to 124 in 40a7876

    
           public function testDatabaseStorageMissingFile() 
        
           { 
        
               $this->tester->execute([]); 
        
               $this->assertStringContainsString('Product images resized successfully', $this->tester->getDisplay()); 
        
           }

jonashrem · 2020-06-19T14:24:30Z

@magento give me 2.4-develop instance

magento-engcom-team · 2020-06-19T14:24:34Z

Hi @jonashrem. Thank you for your request. I'm working on Magento 2.4-develop instance for you

magento-engcom-team · 2020-06-19T14:26:10Z

Hi @jonashrem, here is your Magento instance.
Admin access: https://i-28516-2-4-develop.instances.magento-community.engineering/admin_6f30
Login: 0f11a9f8 Password: 4e5687cd5838
Instance will be terminated in up to 3 hours.

jonashrem · 2020-06-19T15:04:12Z

@ihor-sviziev thanks for the review.

We tested this integrationtest locally on 2.3.5-p1 before.

We now did another local test on our PR directly, but also couldn't reproduce this error.

Can you check we run the test correctly or do we need to do some special Magento settings here (for preparing DB Storage for example)?

We cloned our instance, did composer install and run the test.

dev/tests/integration # ../../../vendor/bin/phpunit --testdox testsuite/Magento/MediaStorage/Console/Command/ImageResizeCommandTest.php
PHPUnit 9.1.5 by Sebastian Bergmann and contributors.

Warning:       Using a custom test suite loader is deprecated

Image Resize Command (Magento\MediaStorage\Console\Command\ImageResizeCommand)
 ✔ Run resize with missing file
 ✔ Execute with zero byte image
 ✔ Database storage missing file

Time: 00:19.762, Memory: 111.00 MB

OK (3 tests, 3 assertions)

=== Memory Usage System Stats ===
Memory usage (OS):      141.02M (129.38% of 109.00M reported by PHP)
Estimated memory leak:  32.02M (22.71% of used memory)

dev/tests/integration # git remote -v                                                                                                          
origin  https://github.com/jonashrem/magento2.git (fetch)
origin  https://github.com/jonashrem/magento2.git (push)

dev/tests/integration # git branch
  2.4-develop
* faster-isTableExists

dev/tests/integration # cat etc/install-config-mysql.php
<?php
/**
 * Copyright © Magento, Inc. All rights reserved.
 * See COPYING.txt for license details.
 */

return [
    'db-host' => 'localhost',
    'db-user' => 'db-user-4',
    'db-password' => '*',
    'db-name' => 'db-4',
    'db-prefix' => '',
    'backend-frontname' => 'backend',
    'admin-user' => \Magento\TestFramework\Bootstrap::ADMIN_NAME,
    'admin-password' => \Magento\TestFramework\Bootstrap::ADMIN_PASSWORD,
    'admin-email' => \Magento\TestFramework\Bootstrap::ADMIN_EMAIL,
    'admin-firstname' => \Magento\TestFramework\Bootstrap::ADMIN_FIRSTNAME,
    'admin-lastname' => \Magento\TestFramework\Bootstrap::ADMIN_LASTNAME,
//    'amqp-host' => 'localhost',
//    'amqp-port' => '5672',
//    'amqp-user' => 'guest',
//    'amqp-password' => 'guest',
];

engcom-Charlie · 2020-08-28T09:11:13Z

@magento run all tests

jonashrem · 2020-08-28T10:21:19Z

@magento run Functional Tests CE

jonashrem · 2020-08-28T11:30:47Z

@magento run Functional Tests B2B

magento-engcom-team · 2020-09-09T11:02:49Z

Hi @ihor-sviziev, thank you for the review.
ENGCOM-7622 has been created to process this Pull Request

sidolov · 2020-09-11T14:43:50Z

Note for QA: the solution should be verified with cases when tables have been changed in DB.

Example:

Check table exists for the first time (cache is generated)
Remove or rename a table
Check table presence once again.

I suspect that we will get an error since the result is cached but the actual table was removed.

jonashrem · 2020-09-11T14:47:55Z

Note for QA: the solution should be verified with cases when tables have been changed in DB.

Example:
1. Check table exists for the first time (cache is generated)

2. Remove or rename a table

3. Check table presence once again.
I suspect that we will get an error since the result is cached but the actual table was removed.

One renaming and dropping tables, the cache will be cleared (

magento2/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php

Line 2721 in b597c7a

$this->resetDdlCache($oldTableName, $schemaName);

and

magento2/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php

Line 2616 in b597c7a

$this->resetDdlCache($tableName, $schemaName);

), so both should be fine.

engcom-Echo · 2020-09-28T15:06:40Z

✔️ QA Passed

Before: ✖️

After: ✔️

The request was not included in the list as the share of its participation was very small.

…xists #28516

m2-assistant · 2020-09-30T19:38:45Z

Hi @jonashrem, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

update getNextAutoincrement to not use showTableStatus for performants reasons. See also magento#28516

Rewrote Magento\Framework\DB\Adapter\Pdo\Mysql->isTableExists

2d67fdd

* Use INFORMATION_SCHEMA.TABLES instead of "SHOW TABLE STATUS" * Extend DDL cache for table existence and use it in isTableExists

ghost added the Progress: pending review label Jun 4, 2020

magento-engcom-team added Component: DB Release Line: 2.4 labels Jun 4, 2020

Conform to Codestyle Guidelines

c77a8f5

* Update Magento\Framework\DB\Adapter\Pdo\Mysql->isTableExists() to conform to code style guidelines (multi-line function calls, line length)

Update Magento\Framework\DB\Adapter\Pdo\Mysql->resetDdlCache()

4876d4e

to conform to code style guidelines (line length)

aleron75 previously approved these changes Jun 5, 2020

View reviewed changes

ghost assigned aleron75 Jun 5, 2020

ghost added Progress: ready for testing and removed Progress: pending review labels Jun 5, 2020

aleron75 added the Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests label Jun 5, 2020

ghost added Progress: pending review and removed Progress: ready for testing labels Jun 19, 2020

ihor-sviziev requested changes Jun 19, 2020

View reviewed changes

ghost assigned ihor-sviziev Jun 19, 2020

ghost added Progress: needs update and removed Progress: pending review labels Jun 19, 2020

magento deleted a comment from magento-engcom-team Sep 8, 2020

ihor-sviziev approved these changes Sep 9, 2020

View reviewed changes

ghost added Progress: testing in progress and removed Progress: ready for testing labels Sep 28, 2020

ghost added Progress: extended testing and removed Progress: testing in progress labels Sep 28, 2020

engcom-Echo added the QA: Ready to add to Regression Scope Should be analyzed and added to Regression Testing Scope(if applicable) label Sep 28, 2020

ihor-sviziev added the Area: Perf/Frontend All tickets related with improving frontend performance. label Sep 29, 2020

engcom-Echo self-assigned this Sep 30, 2020

engcom-Alfa self-assigned this Sep 30, 2020

engcom-Alfa added QA: Added to Regression Scope Scenario was analysed and added to Regression Testing Scope and removed QA: Ready to add to Regression Scope Should be analyzed and added to Regression Testing Scope(if applicable) labels Sep 30, 2020

ghost added Progress: accept and removed Progress: extended testing labels Sep 30, 2020

magento-engcom-team pushed a commit that referenced this pull request Sep 30, 2020

ENGCOM-7622: Rewrote Magento\Framework\DB\Adapter\Pdo\Mysql->isTableE…

3a6007f

…xists #28516

magento-engcom-team merged commit 6d89270 into magento:2.4-develop Sep 30, 2020

ihor-sviziev added Award: bug fix Award: category of expertise labels Sep 30, 2020

This was referenced Jul 1, 2021

backport showTableStatus from Magento2 OpenMage/magento-lts#1712

Closed

rewrite isTableExists for performance reasons OpenMage/magento-lts#1720

Merged

jonashrem added a commit to jonashrem/magento2 that referenced this pull request Jul 6, 2021

update getNextAutoincrement

66696e6

update getNextAutoincrement to not use showTableStatus for performants reasons. See also magento#28516

jonashrem mentioned this pull request Jul 6, 2021

update getNextAutoincrement #33433

Open

5 tasks

	public function testDatabaseStorageMissingFile()
	{
	$this->tester->execute([]);
	$this->assertStringContainsString('Product images resized successfully', $this->tester->getDisplay());
	}

Rewrote Magento\Framework\DB\Adapter\Pdo\Mysql->isTableExists #28516

Rewrote Magento\Framework\DB\Adapter\Pdo\Mysql->isTableExists #28516

Uh oh!

Conversation

jonashrem commented Jun 4, 2020 • edited by m2-assistant bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Implementation details

Manual testing scenarios

Questions or comments

Contribution checklist

Resolved issues:

Uh oh!

m2-assistant bot commented Jun 4, 2020

Uh oh!

jonashrem commented Jun 4, 2020

Uh oh!

jonashrem commented Jun 4, 2020

Uh oh!

jonashrem commented Jun 4, 2020

Uh oh!

jonashrem commented Jun 4, 2020

Uh oh!

aleron75 commented Jun 5, 2020

Uh oh!

magento-engcom-team commented Jun 5, 2020

Uh oh!

jonashrem commented Jun 9, 2020

Uh oh!

ihor-sviziev left a comment

Choose a reason for hiding this comment

Uh oh!

jonashrem commented Jun 19, 2020

Uh oh!

magento-engcom-team commented Jun 19, 2020

Uh oh!

magento-engcom-team commented Jun 19, 2020

Uh oh!

jonashrem commented Jun 19, 2020

Uh oh!

engcom-Charlie commented Aug 28, 2020

Uh oh!

jonashrem commented Aug 28, 2020

Uh oh!

jonashrem commented Aug 28, 2020

Uh oh!

magento-engcom-team commented Sep 9, 2020

Uh oh!

sidolov commented Sep 11, 2020

Uh oh!

jonashrem commented Sep 11, 2020

Uh oh!

engcom-Echo commented Sep 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

m2-assistant bot commented Sep 30, 2020

Uh oh!

Uh oh!

jonashrem commented Jun 4, 2020 •

edited by m2-assistant bot

Loading

engcom-Echo commented Sep 28, 2020 •

edited

Loading