Price Indexer Performance Issue With Out of Stock Products #24415

brucemead · 2019-09-02T16:46:28Z

Description (*)

Within the reindexing process there is a bug which results in a delete query to be called for every out of stock product during a full or partial reindex.

Steps to reproduce (*)

Ensure that display out of stock products option is disabled
php bin/magento indexer:reindex catalog_product_price
php bin/magento cron:run --group=index

Fixed Issues (if relevant)

Price Indexer Performance Issue With Out of Stock Products #24414: Price Indexer Performance Issue With Out of Stock Products

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)

- Relates to issue magento#24414

m2-assistant · 2019-09-02T16:46:31Z

Hi @brucemead. 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.3-develop instance - deploy vanilla Magento instance

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

aleron75 · 2019-09-04T06:44:00Z

app/code/Magento/CatalogInventory/Model/Indexer/ProductPriceIndexFilter.php

            $select->where('stock_item.use_config_manage_stock = 0 AND stock_item.manage_stock = 1');
        }

+        if ($entityIds !== null) {


I think the null check is not needed and can be removed, simplifying code: the $entityIds parameter has declared type so in case the $entityIds parameter of modifyPrice() function is null, PHP would throw a fatal error.

sidolov

@brucemead please, apply changes suggested by @aleron75

sidolov · 2019-09-05T17:01:10Z

app/code/Magento/CatalogInventory/Model/Indexer/ProductPriceIndexFilter.php

        }

+        if ($entityIds !== null) {
+            if (count($entityIds) > 1) {


Why we should count elements here? $entityIds always array, let's use $select->where('stock_item.product_id in (?)', $entityIds) in all cases if array is not empty

Yeah fair point I copied the implementation from here Magento\Catalog\Model\ResourceModel\Product\Indexer\Price\Query\BaseFinalPrice::getQuery

southerncomputer · 2019-09-11T18:47:43Z

might be faster if you intval $entityids too!

also pull against the same thing in vendor/magento/module-catalog/Model/ResourceModel/Product/Indexer/Price/Query/BaseFinalPrice.php as it uses this same routine but with a between min/max(entityIds) which is horribly wrong to do for a list of 100 entityids (think 2000 and 2Million) over and over again!

sidolov · 2019-09-19T17:24:08Z

@brucemead , I am closing this PR now due to inactivity.
Please reopen and update if you wish to continue.
Thank you for the collaboration!

m2-assistant · 2019-09-19T17:24:10Z

Hi @brucemead, 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.

- Relates to issue magento#24414 - Made changes based on feedback on PR

brucemead · 2019-09-19T19:35:42Z

@sidolov I've pushed the suggested changes now. Sorry for the slow turn around I've been on leave recently.

Thanks,
Bruce

app/code/Magento/CatalogInventory/Model/Indexer/ProductPriceIndexFilter.php

magento-engcom-team · 2019-09-21T05:54:43Z

Hi @aleron75, thank you for the review.
ENGCOM-5910 has been created to process this Pull Request

- Relates to issue magento#24414 - Made changes based on feedback on PR - Resolving whitespace issue https://testing-service.magento-community.engineering/reports/magento/magento2/pull/24415/56e6453d1da3a1546212c6c35f78d77d/Statics/allure-report-ce/index.html#categories/479362f77c9377ec388bccba15830ad3/997602107dd59e40/

…xer-performance-fix' into fix/catalog-inventory-price-indexer-performance-fix # Conflicts: # app/code/Magento/CatalogInventory/Model/Indexer/ProductPriceIndexFilter.php

- Relates to issue magento#24414 - Resolving whitespace issue https://testing-service.magento-community.engineering/reports/magento/magento2/pull/24415/56e6453d1da3a1546212c6c35f78d77d/Statics/allure-report-ce/index.html#categories/479362f77c9377ec388bccba15830ad3/997602107dd59e40/

brucemead · 2019-09-21T08:49:07Z

@sidolov Just pushed a fix for the white space issue that was flagged here https://testing-service.magento-community.engineering/reports/magento/magento2/pull/24415/56e6453d1da3a1546212c6c35f78d77d/Statics/allure-report-ce/index.html#categories/479362f77c9377ec388bccba15830ad3/997602107dd59e40/retries

engcom-Alfa · 2019-09-30T11:38:52Z

✔️ QA Passed

#24415

m2-assistant · 2019-10-03T12:59:55Z

Hi @brucemead, 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.

Price Indexer Performance Issue With Out of Stock Products

7563129

- Relates to issue magento#24414

magento-engcom-team added Component: CatalogInventory Release Line: 2.3 Partner: AYKO partners-contribution Pull Request is created by Magento Partner labels Sep 2, 2019

brucemead mentioned this pull request Sep 2, 2019

Price Indexer Performance Issue With Out of Stock Products #24414

Closed

brucemead added the Component: Indexer label Sep 2, 2019

aleron75 reviewed Sep 4, 2019

View reviewed changes

aleron75 self-assigned this Sep 4, 2019

sidolov requested changes Sep 4, 2019

View reviewed changes

ghost assigned sidolov Sep 4, 2019

ghost added the Progress: needs update label Sep 4, 2019

sidolov requested changes Sep 5, 2019

View reviewed changes

sidolov closed this Sep 19, 2019

Price Indexer Performance Issue With Out of Stock Products

4b1b1dc

- Relates to issue magento#24414 - Made changes based on feedback on PR

brucemead reopened this Sep 19, 2019

brucemead requested review from maghamed and tariqjawed83 as code owners September 19, 2019 19:35

ghost unassigned aleron75 and sidolov Sep 19, 2019

aleron75 reviewed Sep 20, 2019

View reviewed changes

app/code/Magento/CatalogInventory/Model/Indexer/ProductPriceIndexFilter.php Outdated Show resolved Hide resolved

Added check for empty array

6677a49

sidolov approved these changes Sep 20, 2019

View reviewed changes

ghost assigned sidolov Sep 20, 2019

ghost removed the Progress: needs update label Sep 20, 2019

sidolov added the Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests label Sep 20, 2019

aleron75 approved these changes Sep 21, 2019

View reviewed changes

ghost assigned aleron75 Sep 21, 2019

brucemead added 3 commits September 21, 2019 09:38

Merge remote-tracking branch 'origin/fix/catalog-inventory-price-inde…

9c9d286

…xer-performance-fix' into fix/catalog-inventory-price-indexer-performance-fix # Conflicts: # app/code/Magento/CatalogInventory/Model/Indexer/ProductPriceIndexFilter.php

engcom-Alfa self-assigned this Sep 23, 2019

ghost added Progress: testing in progress Progress: extended testing and removed Progress: ready for testing Progress: testing in progress Progress: extended testing labels Sep 23, 2019

engcom-Golf self-assigned this Sep 30, 2019

ghost added Progress: extended testing and removed Progress: testing in progress labels Sep 30, 2019

Cover changes with unit test.

a73b832

ghost added Progress: accept and removed Progress: extended testing labels Oct 1, 2019

sidolov added Auto-Tests: Covered All changes in Pull Request is covered by auto-tests and removed Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests labels Oct 1, 2019

magento-engcom-team added Award: test coverage Award: bug fix labels Oct 1, 2019

magento-engcom-team pushed a commit that referenced this pull request Oct 3, 2019

ENGCOM-5910: Price Indexer Performance Issue With Out of Stock Products

882ea8f

#24415

magento-engcom-team merged commit a73b832 into magento:2.3-develop Oct 3, 2019

magento-engcom-team added this to the Release: 2.3.4 milestone Oct 3, 2019

Price Indexer Performance Issue With Out of Stock Products #24415

Price Indexer Performance Issue With Out of Stock Products #24415

Uh oh!

Conversation

brucemead commented Sep 2, 2019

Description (*)

Steps to reproduce (*)

Fixed Issues (if relevant)

Contribution checklist (*)

Uh oh!

m2-assistant bot commented Sep 2, 2019

Uh oh!

aleron75 Sep 4, 2019

Choose a reason for hiding this comment

Uh oh!

sidolov left a comment

Choose a reason for hiding this comment

Uh oh!

sidolov Sep 5, 2019

Choose a reason for hiding this comment

Uh oh!

brucemead Sep 19, 2019

Choose a reason for hiding this comment

Uh oh!

southerncomputer commented Sep 11, 2019

Uh oh!

sidolov commented Sep 19, 2019

Uh oh!

m2-assistant bot commented Sep 19, 2019

Uh oh!

brucemead commented Sep 19, 2019

Uh oh!

Uh oh!

magento-engcom-team commented Sep 21, 2019

Uh oh!

brucemead commented Sep 21, 2019

Uh oh!

engcom-Alfa commented Sep 30, 2019

Uh oh!

m2-assistant bot commented Oct 3, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants