High Database Load for Sales Rule Validation #19117

rauberdaniel · 2018-11-08T14:04:18Z

Preconditions (*)

Magento 2.2.5
PHP 7.1 / MySQL 5.6.41

Steps to reproduce (*)

Create multiple Sales Rules with multiple coupons each
Get a lot of customers to redeem codes

Expected result (*)

High Performance, as finding sales rules is fast using indexes on Database

Actual result (*)

Incredibly Low Performance, as SQL query is badly constructed so it cannot use indexes

Description

We’re currently having about 400 sales rules (most of them are inactive or expired) as well as about 600.000 coupons. The query, that is currently being used by the sales-rule module is extremely inefficient on the database and therefore results in massive database CPU consumption when having several orders with coupons per minute. However, this query can greatly be optimized by splitting it into two queries.

The query that is generated by the setValidationFilter method in magento/vendor/magento/module-sales-rule/Model/ResourceModel/Rule/Collection.php currently looks like this:

SELECT `main_table`.*, `rule_coupons`.`code`
FROM `salesrule` AS `main_table`
INNER JOIN `salesrule_website` AS `website`
ON website.website_id IN ('1') AND main_table.rule_id = website.rule_id
INNER JOIN `salesrule_customer_group` AS `customer_group_ids`
ON main_table.rule_id = customer_group_ids.rule_id AND customer_group_ids.customer_group_id = 0
LEFT JOIN `salesrule_coupon` AS `rule_coupons` ON main_table.rule_id = rule_coupons.rule_id AND main_table.coupon_type != 1

WHERE (from_date is null or from_date <= '2018-11-01')
AND (to_date is null or to_date >= '2018-11-01')
AND (`is_active` = '1')
AND
(
    main_table.coupon_type = 1
    OR
    (
        (
            (main_table.coupon_type = 3 AND rule_coupons.type = 1)
            OR
            (main_table.coupon_type = 2 AND main_table.use_auto_generation = 1 AND rule_coupons.type = 1)
            OR
            (main_table.coupon_type = 2 AND main_table.use_auto_generation = 0 AND rule_coupons.type = 0)
        )
        AND
        rule_coupons.code = 'COUPONCODE'
        AND
        (rule_coupons.expiration_date IS NULL OR rule_coupons.expiration_date >= '2018-11-01')
    )
)
AND (`is_active` = '1') ORDER BY sort_order ASC;

The problematic part in this query is line 14 main_table.coupon_type = 1 OR. This part forces the database to filter the joined tables based on coupon_type. By simply removing that line, the rule_coupons.code can be used as an index to find exactly the one matching sales rule without having to join and filter a massive amount of rows. The line 14 is a completely separate case than the rest in that scope (line 13 to 29) and therefore should be treated separately e.g. by a second query in a UNION operation.

Some details using the MySQL EXPLAIN function:

Original Query:

Query without line 14/15 (not added in a second query in this example, therefore ignoring sales rules without coupons):

This is a major performance improvement especially for larger amounts of sales rules and coupons and absolutely critical.

The text was updated successfully, but these errors were encountered:

magento-engcom-team · 2018-11-08T14:04:22Z

Hi @rauberdaniel. Thank you for your report.
To help us process this issue please make sure that you provided the following information:

Summary of the issue
Information on your environment
Steps to reproduce
Expected and actual results

Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:

@magento-engcom-team give me $VERSION instance

where $VERSION is version tags (starting from 2.2.0+) or develop branches (for example: 2.3-develop).
For more details, please, review the Magento Contributor Assistant documentation.

@rauberdaniel do you confirm that you was able to reproduce the issue on vanilla Magento instance following steps to reproduce?

yes
no

rauberdaniel · 2018-11-08T14:17:46Z

To be a bit more precise, the alternative recommended query that has the equal result but split into two queries that are very fast because of index usage is:

SELECT `main_table`.*, `rule_coupons`.`code`
FROM `salesrule` AS `main_table`
INNER JOIN `salesrule_website` AS `website`
ON website.website_id IN ('1') AND main_table.rule_id = website.rule_id
INNER JOIN `salesrule_customer_group` AS `customer_group_ids`
ON main_table.rule_id = customer_group_ids.rule_id AND customer_group_ids.customer_group_id = 0
LEFT JOIN `salesrule_coupon` AS `rule_coupons` ON main_table.rule_id = rule_coupons.rule_id AND main_table.coupon_type != 1

WHERE (from_date is null or from_date <= '2018-11-08')
AND (to_date is null or to_date >= '2018-11-08')
AND (`is_active` = '1')
AND
   (
        (
            (main_table.coupon_type = 3 AND rule_coupons.type = 1)
            OR
            (main_table.coupon_type = 2 AND main_table.use_auto_generation = 1 AND rule_coupons.type = 1)
            OR
            (main_table.coupon_type = 2 AND main_table.use_auto_generation = 0 AND rule_coupons.type = 0)
        )
        AND
        rule_coupons.code = 'COUPONCODE'
        AND
        (rule_coupons.expiration_date IS NULL OR rule_coupons.expiration_date >= '2018-11-08')
    )
AND (`is_active` = '1')

UNION

SELECT `main_table`.*, NULL as code
FROM `salesrule` AS `main_table`
INNER JOIN `salesrule_website` AS `website`
ON website.website_id IN ('1') AND main_table.rule_id = website.rule_id
INNER JOIN `salesrule_customer_group` AS `customer_group_ids`
ON main_table.rule_id = customer_group_ids.rule_id AND customer_group_ids.customer_group_id = 0
WHERE (from_date is null or from_date <= '2018-11-08')
AND (to_date is null or to_date >= '2018-11-08')
AND (`is_active` = '1')
AND (main_table.coupon_type = 1)

ORDER BY sort_order ASC;

The first query will then use rule_coupons.code as the index to retrieve that one coupon as well as the respective sales rule, the second query has to filter the sales rules table which however is smaller than the coupon table (on any joined combination). In our case by a very big factor as one sales rule can have multiple thousands of individual coupons.

orlangur · 2018-11-08T14:58:41Z

@rauberdaniel great finding and analysis! Are you interested in preparing a pull request maybe?

rauberdaniel · 2018-11-08T15:01:06Z

@orlangur I absolutely would, however, I’m not very into Magento code (or PHP at all) and the class that defines this query as well as the abstract are pretty confusing to me.

orlangur · 2018-11-08T15:08:09Z

@rauberdaniel got it, no problem. Let's wait for a volunteer then.

magento-engcom-team · 2018-11-09T08:36:02Z

Hi @engcom-backlog-nazar. Thank you for working on this issue.
In order to make sure that issue has enough information and ready for development, please read and check the following instruction: 👇

1. Verify that issue has all the required information. (Preconditions, Steps to reproduce, Expected result, Actual result).
Details
If the issue has a valid description, the label Issue: Format is valid will be added to the issue automatically. Please, edit issue description if needed, until label Issue: Format is valid appears.
2. Verify that issue has a meaningful description and provides enough information to reproduce the issue. If the report is valid, add Issue: Clear Description label to the issue by yourself.
3. Add Component: XXXXX label(s) to the ticket, indicating the components it may be related to.
4. Verify that the issue is reproducible on 2.3-develop branch
Details
- Add the comment @magento-engcom-team give me 2.3-develop instance to deploy test instance on Magento infrastructure.
- If the issue is reproducible on 2.3-develop branch, please, add the label Reproduced on 2.3.x.
- If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!

5. Verify that the issue is reproducible on 2.2-develop branch.
Details
- Add the comment @magento-engcom-team give me 2.2-develop instance to deploy test instance on Magento infrastructure.
- If the issue is reproducible on 2.2-develop branch, please add the label Reproduced on 2.2.x
6. Add label Issue: Confirmed once verification is complete.
7. Make sure that automatic system confirms that report has been added to the backlog.

magento-engcom-team · 2018-11-09T09:18:12Z

@engcom-backlog-nazar Thank you for verifying the issue. Based on the provided information internal tickets MAGETWO-96241, MAGETWO-96242 were created

magento-engcom-team · 2018-11-16T12:47:02Z

Hi @progreg. Thank you for working on this issue.
Looks like this issue is already verified and confirmed. But if your want to validate it one more time, please, go though the following instruction:

1. Add/Edit Component: XXXXX label(s) to the ticket, indicating the components it may be related to.
2. Verify that the issue is reproducible on 2.3-develop branch
Details
- Add the comment @magento-engcom-team give me 2.3-develop instance to deploy test instance on Magento infrastructure.
- If the issue is reproducible on 2.3-develop branch, please, add the label Reproduced on 2.3.x.
- If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!

3. Verify that the issue is reproducible on 2.2-develop branch.
Details
- Add the comment @magento-engcom-team give me 2.2-develop instance to deploy test instance on Magento infrastructure.
- If the issue is reproducible on 2.2-develop branch, please add the label Reproduced on 2.2.x
4. If the issue is not relevant or is not reproducible any more, feel free to close it.

progreg · 2018-11-20T14:37:32Z

Hey @orlangur,

I already have some working solution, but I'm not sure about an extension point that I've used.
My solution is to clone select in beforeLoad method and filter all no needed conditions to prepare select for the union. The reason why I choose such a strategy is not crash custom conditions that can be added manually to origin select.

All conditions that should be deleted in cloned selectwould be marked by /*@OPTIONAL*/ comment, for example:

$select->where(
    '/*@OPTIONAL*/ (' . $orWhereCondition . ') AND ' . $andWhereCondition,
    null,
    Select::TYPE_CONDITION
);

So in result i have such beforeLoad method in Magento\SalesRule\Model\ResourceModel\Rule\Collection.

protected function _beforeLoad()
{
    if ($this->getFlag('validation_filter')) {
        $connection = $this->getConnection();
        $noCouponWhereCondition = $connection->quoteInto(
            'main_table.coupon_type = ? ',
            \Magento\SalesRule\Model\Rule::COUPON_TYPE_NO_COUPON
        );

        $origin = $this->_select;
        $clone = clone $this->_select;
        $clone->where($noCouponWhereCondition);

        // Remove coupon join
        $fromPart = array_filter(
            $clone->getPart(Select::FROM),
            function ($item) {
                if ($item['joinType'] == 'left join' && $item['tableName'] == 'salesrule_coupon') {
                    return false;
                }
                return $item;
            }
        );
        $clone->setPart(Select::FROM, $fromPart);

        // Replace coupon code column with NULL value
        $columnsPart = array_map(
            function ($item) use ($connection) {
                if ($item[0] == 'rule_coupons' && $item[1] == 'code') {
                    $item[1] = new Zend_Db_Expr('NULL as ' . $connection->quote('code'));
                }
                return $item;
            },
            $clone->getPart(Select::COLUMNS)
        );
        $clone->setPart(Select::COLUMNS, $columnsPart);

        // Remove where conditions related to coupon_type
        $wherePart = array_filter(
            $clone->getPart(Select::WHERE),
            function ($item) {
                if (preg_match('/\*@OPTIONAL\*/', $item)) {
                    return false;
                }
                return $item;
            }
        );
        $clone->setPart(Select::WHERE, $wherePart);

        // Apply UNION
        $this->_select = $this->getConnection()->select();
        $this->_select->union([$origin, $clone]);
    }
    return parent::_beforeLoad();
}

This is just an idea of concrete realization, that's why I first wanna ask you what is the best solution for this issue in your opinion. Also please check maybe I miss something.

Query Before:

SELECT `main_table`.*, `rule_coupons`.`code`
FROM `salesrule` AS `main_table`
       INNER JOIN `salesrule_website` AS `website` ON website.website_id IN (1) AND main_table.rule_id = website.rule_id
       INNER JOIN `salesrule_customer_group` AS `customer_group_ids`
         ON main_table.rule_id = customer_group_ids.rule_id AND customer_group_ids.customer_group_id = 1
       LEFT JOIN `salesrule_coupon` AS `rule_coupons`
         ON main_table.rule_id = rule_coupons.rule_id AND main_table.coupon_type != 1
WHERE (from_date is null or from_date <= '2018-11-20')
  AND (to_date is null or to_date >= '2018-11-20')
  AND (`is_active` = '1')
  AND (/*@OPTIONAL*/
          (
              (main_table.coupon_type = 3 AND rule_coupons.type = 1)
                OR
              (main_table.coupon_type = 2 AND main_table.use_auto_generation = 1 AND rule_coupons.type = 1)
                OR
              (main_table.coupon_type = 2 AND main_table.use_auto_generation = 0 AND rule_coupons.type = 0)
              )
            AND
          rule_coupons.code = 'asdasdasdasd'
            AND
          (rule_coupons.expiration_date IS NULL OR rule_coupons.expiration_date >= '2018-11-20')
          )

Query After:

SELECT `main_table`.*, `rule_coupons`.`code`
FROM `salesrule` AS `main_table`
       INNER JOIN `salesrule_website` AS `website` ON website.website_id IN (1) AND main_table.rule_id = website.rule_id
       INNER JOIN `salesrule_customer_group` AS `customer_group_ids`
         ON main_table.rule_id = customer_group_ids.rule_id AND customer_group_ids.customer_group_id = 1
       LEFT JOIN `salesrule_coupon` AS `rule_coupons`
         ON main_table.rule_id = rule_coupons.rule_id AND main_table.coupon_type != 1
WHERE (from_date is null or from_date <= '2018-11-20')
  AND (to_date is null or to_date >= '2018-11-20')
  AND (`is_active` = '1')
  AND (/*@OPTIONAL*/
          (
              (main_table.coupon_type = 3 AND rule_coupons.type = 1) 
                 OR
              (main_table.coupon_type = 2 AND main_table.use_auto_generation = 1 AND rule_coupons.type = 1) 
                 OR
              (main_table.coupon_type = 2 AND main_table.use_auto_generation = 0 AND rule_coupons.type = 0)
          ) 
          AND
          rule_coupons.code = 'asdasdasdasd' 
          AND
         (rule_coupons.expiration_date IS NULL OR rule_coupons.expiration_date >= '2018-11-20')
   )
UNION
SELECT `main_table`.*, NULL as 'code'
FROM `salesrule` AS `main_table`
       INNER JOIN `salesrule_website` AS `website` ON website.website_id IN (1) AND main_table.rule_id = website.rule_id
       INNER JOIN `salesrule_customer_group` AS `customer_group_ids`
         ON main_table.rule_id = customer_group_ids.rule_id AND customer_group_ids.customer_group_id = 1
WHERE (from_date is null or from_date <= '2018-11-20')
  AND (to_date is null or to_date >= '2018-11-20')
  AND (`is_active` = '1')
  AND (main_table.coupon_type = 1)

progreg · 2018-12-18T09:17:53Z

Hey @orlangur. Can you give me any feedback, is my solution may be good? Or I should add PR first?

Github Issue: magento#19117 Refactored sql query that created a huge temporary table for each request, when a greater amount of salesrules and coupon codes exists in database. The sorting of this table took a lot of cpu time. The statement now consists of two subselects that drill down the remaining lines as far as possible, so that the remaining temporary table is minimal and easily sorted. example: for 2,000 salesrules and 3,000,000 coupon codes the original query took about 2.4 seconds (mbp, server, aws). the optimized query takes about 5ms (about 100ms on aws).

rauberdaniel · 2019-01-23T10:56:35Z

@david-fuehr Thank you very much for your work on this issue!

rauberdaniel · 2019-01-23T10:58:03Z

@progreg Thank you for your work as well! Sorry to see you didn’t get any feedback on your work (which I’m unable to give due to little knowledge over Magento).

progreg · 2019-01-23T11:04:16Z

@rauberdaniel, At least I've tried) Anyway, if a solution of @david-fuehr will be better then all is good.

david-fuehr · 2019-01-23T13:26:36Z

@rauberdaniel @progreg Thanks for the report and your thoughts on this issue. With that, we were able to quickly solve that issue as we encountered it.
And as we have a working solution, I found a contribution appropriate ;)

magento-engcom-team · 2019-03-08T11:59:24Z

Hi @rauberdaniel. Thank you for your report.
The issue has been fixed in #20484 by @david-fuehr in 2.3-develop branch
Related commit(s):

The fix will be available with the upcoming 2.3.2 release.

Github Issue: magento#19117 Refactored sql query that created a huge temporary table for each request, when a greater amount of salesrules and coupon codes exists in database. The sorting of this table took a lot of cpu time. The statement now consists of two subselects that drill down the remaining lines as far as possible, so that the remaining temporary table is minimal and easily sorted. example: for 2,000 salesrules and 3,000,000 coupon codes the original query took about 2.4 seconds (mbp, server, aws). the optimized query takes about 5ms (about 100ms on aws).

magento-engcom-team · 2019-03-14T15:30:17Z

Hi @rauberdaniel. Thank you for your report.
The issue has been fixed in #21699 by @david-fuehr in 2.2-develop branch
Related commit(s):

The fix will be available with the upcoming 2.2.9 release.

cesmendez · 2019-05-15T21:05:43Z

Hello, I realize that this post is now closed. But do you all know if there is a similar solution for M1 EE?

david-fuehr · 2019-05-16T10:00:30Z

Hello @cesmendez,

I don't believe that someone implemented this (or a similar) fix for M1. But as the code hardly changed, you could try to apply these changes: https://github.com/magento/magento2/pull/20484/files to magento/app/code/core/Mage/SalesRule/Model/Resource/Rule/Collection.php

magento-engcom-team added the Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed label Nov 8, 2018

orlangur added the Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed label Nov 8, 2018

orlangur added help wanted Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development and removed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development labels Nov 8, 2018

ghost self-assigned this Nov 9, 2018

ghost added Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release Component: SalesRule Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed labels Nov 9, 2018

magento-engcom-team added the Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development label Nov 9, 2018

ghost removed their assignment Nov 9, 2018

progreg self-assigned this Nov 16, 2018

progreg pinned this issue Jan 21, 2019

ajay2jcommerce unpinned this issue Jan 21, 2019

david-fuehr mentioned this issue Jan 22, 2019

Fix performance leak in salesrule collection #20484

Merged

4 tasks

progreg removed their assignment Jan 23, 2019

magento-engcom-team added the Fixed in 2.3.x The issue has been fixed in 2.3 release line label Mar 8, 2019

magento-engcom-team closed this as completed Mar 8, 2019

david-fuehr mentioned this issue Mar 12, 2019

[Backport] 20484 - Fix performance leak in salesrule collection #21699

Merged

4 tasks

magento-engcom-team added the Fixed in 2.2.x The issue has been fixed in 2.2 release line label Mar 14, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

High Database Load for Sales Rule Validation #19117

High Database Load for Sales Rule Validation #19117

rauberdaniel commented Nov 8, 2018 •

edited

Loading

magento-engcom-team commented Nov 8, 2018

rauberdaniel commented Nov 8, 2018 •

edited

Loading

orlangur commented Nov 8, 2018

rauberdaniel commented Nov 8, 2018

orlangur commented Nov 8, 2018

magento-engcom-team commented Nov 9, 2018 •

edited by ghost

Loading

magento-engcom-team commented Nov 9, 2018

magento-engcom-team commented Nov 16, 2018 •

edited by progreg

Loading

progreg commented Nov 20, 2018

progreg commented Dec 18, 2018

rauberdaniel commented Jan 23, 2019

rauberdaniel commented Jan 23, 2019

progreg commented Jan 23, 2019

david-fuehr commented Jan 23, 2019

magento-engcom-team commented Mar 8, 2019

magento-engcom-team commented Mar 14, 2019

cesmendez commented May 15, 2019

david-fuehr commented May 16, 2019

High Database Load for Sales Rule Validation #19117

High Database Load for Sales Rule Validation #19117

Comments

rauberdaniel commented Nov 8, 2018 • edited Loading

Preconditions (*)

Steps to reproduce (*)

Expected result (*)

Actual result (*)

Description

magento-engcom-team commented Nov 8, 2018

rauberdaniel commented Nov 8, 2018 • edited Loading

orlangur commented Nov 8, 2018

rauberdaniel commented Nov 8, 2018

orlangur commented Nov 8, 2018

magento-engcom-team commented Nov 9, 2018 • edited by ghost Loading

magento-engcom-team commented Nov 9, 2018

magento-engcom-team commented Nov 16, 2018 • edited by progreg Loading

progreg commented Nov 20, 2018

progreg commented Dec 18, 2018

rauberdaniel commented Jan 23, 2019

rauberdaniel commented Jan 23, 2019

progreg commented Jan 23, 2019

david-fuehr commented Jan 23, 2019

magento-engcom-team commented Mar 8, 2019

magento-engcom-team commented Mar 14, 2019

cesmendez commented May 15, 2019

david-fuehr commented May 16, 2019

rauberdaniel commented Nov 8, 2018 •

edited

Loading

rauberdaniel commented Nov 8, 2018 •

edited

Loading

magento-engcom-team commented Nov 9, 2018 •

edited by ghost

Loading

magento-engcom-team commented Nov 16, 2018 •

edited by progreg

Loading