magento-lts icon indicating copy to clipboard operation
magento-lts copied to clipboard

Improving query performance for coupon code check

Open lemundo-team opened this issue 5 years ago • 2 comments

Adding coupon code to where clause in order to improve query performance

Description (*)

This was one of the major bottle necks over the last few Black Fridays! Thousands of customers applying coupon codes in checkout and the checkout is causing causing more and more load.

It might be that this query is only leading to bad performance when using certain MySQL versions like https://github.com/OpenMage/magento-lts/issues/295 does...

Related Pull Requests

https://github.com/OpenMage/magento-lts/pull/303

Manual testing scenarios (*)

  1. Let 1000 customers apply coupon codes every minute
  2. Checkout the slow query log

Contribution checklist (*)

  • [x] Pull request has a meaningful description of its purpose
  • [x] All commits are accompanied by meaningful commit messages
  • [x] All automated tests passed successfully (all builds are green)

lemundo-team avatar Jul 13 '20 18:07 lemundo-team

Hi Can you give more context? Eg how the original query looks like and why adding the where constraint on coupon speeds things up. Do you have any numbers before/after optimization? What mysql version are you using?

tmotyl avatar Jul 13 '20 20:07 tmotyl

pinging @lemundo-team to see if we can get some info although this PR is quite old

fballiano avatar Jun 09 '22 13:06 fballiano

I discover this PR today, I also found a dramatic performance loss with setValidationFilter, I try to fix the problem by rewriting left joins. I will also try the proposed change. An image from Blackfire (before any changes):

image

luigifab avatar Jan 17 '23 10:01 luigifab

fixed conflict

fballiano avatar Jan 17 '23 10:01 fballiano

@Flyingmana can we setup demo-instances and connect them to blackfire.io?

  • latest v19 release
  • latest v20 release
  • dev-main

sreichel avatar Jan 17 '23 10:01 sreichel

@Flyingmana can we setup demo-instances and connect them to blackfire.io?

  • latest v19 release
  • latest v20 release
  • dev-main

we have our demo on nexess which is configured here: https://github.com/OpenMage/OpenMage-demo-deployment (and where we have also newRelic configured)

although this looks more like something which fits into the gitpod thing Or you want a permanent one for all 3 branches?

Flyingmana avatar Jan 17 '23 11:01 Flyingmana

Before any changes from checkout/cart with a coupon applied:

image

With the current PR:

image

With another approach (© me, dev in progress):

image

With another approach and with the current PR (© me, dev in progress):

image

luigifab avatar Jan 17 '23 11:01 luigifab

I think we should have both the fixes

fballiano avatar Jan 17 '23 11:01 fballiano

I haven't finished the crash test for my version.

luigifab avatar Jan 17 '23 11:01 luigifab

Or you want a permanent one for all 3 branches?

this would be nice, but should be discussed somewhere else.

sreichel avatar Jan 18 '23 00:01 sreichel

$result = Mage::getResourceModel('salesrule/rule_collection')
            ->setValidationFilter(1, 10, 'abc')
            ->getSelect()->__toString();

$result is

SELECT`main_table`.*, `rule_coupons`.`code` 
FROM `salesrule` AS `main_table` 
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 = 10 
LEFT JOIN `salesrule_coupon` AS `rule_coupons` 
  ON main_table.rule_id = rule_coupons.rule_id 
    AND main_table.coupon_type != 1 #cond1
    AND rule_coupons.code = 'abc' #cond2 added by PR
WHERE 
  (
    EXISTS (
      SELECT 1 
      FROM `salesrule_website` AS `website` 
      WHERE 
        (website.website_id IN (1)) AND (main_table.rule_id = website.rule_id)
    )
  ) 
  AND (from_date is null or from_date <= '2023-01-19') 
  AND (to_date is null or to_date >= '2023-01-19') 
  AND (`is_active` = '1') 
  AND (
    main_table.coupon_type = 1 #see cond1, this is always false
    OR (
      (
        (
          main_table.coupon_type = 3  AND rule_coupons.type = 0
        ) 
        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 = 'abc'
    )
  )

Can be simplified to

SELECT`main_table`.*, `rule_coupons`.`code` 
FROM `salesrule` AS `main_table` 
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 = 10 
LEFT JOIN `salesrule_coupon` AS `rule_coupons` 
  ON main_table.rule_id = rule_coupons.rule_id 
    AND main_table.coupon_type != 1 
    AND rule_coupons.code = 'abc' #cond2 added by PR
WHERE 
  (
    EXISTS (
      SELECT 1 
      FROM `salesrule_website` AS `website` 
      WHERE 
        (website.website_id IN (1)) AND (main_table.rule_id = website.rule_id)
    )
  ) 
  AND (from_date is null or from_date <= '2023-01-19') 
  AND (to_date is null or to_date >= '2023-01-19') 
  AND (`is_active` = '1') 
  AND rule_coupons.code = 'abc' #cond3
  AND   
      (
        (
          main_table.coupon_type = 3 AND rule_coupons.type = 0
        ) 
        OR (
          main_table.coupon_type = 2 AND main_table.use_auto_generation <= 1 AND rule_coupons.type <= 1
        ) 
      ) 

I think either cond2 or cond3 can be removed, but not sure which one is faster.

kiatng avatar Jan 19 '23 02:01 kiatng

@kiatng let's merge this in the meanwhile and see if we can improve it ;-)

fballiano avatar Jan 19 '23 09:01 fballiano

merged and cherrypicked to v20

fballiano avatar Jan 19 '23 09:01 fballiano

Just FYI We've backported this to our magento installation (we're slowly updating openmage to its newer versions, we don't want to break anything and carefully examine each release) and we got a HUUUGE performance increase. We are talking 10s of millions records here. (Down from ~9s to ~1s)

Thank you!

pquerner avatar Jun 15 '23 14:06 pquerner