magento2: Bug on category count (performance issue)

In src/vendor/magento/module-catalog/Model/ResourceModel/Category/Collection.php

Line 330-333 :

                $productsCount = isset($categoriesProductsCount[$item->getId()])
                    ? (int)$categoryProductsCount[$item->getId()]
                    : $this->getProductsCountFromCategoryTable($item, $websiteId);

should be

                $productsCount = isset($categoryProductsCount[$item->getId()])
                    ? (int)$categoryProductsCount[$item->getId()]
                    : $this->getProductsCountFromCategoryTable($item, $websiteId);

( categoriesProductsCount instead of categoryProductsCount )

This is causing performance issue on backend category page especially when having a big amount of categories/products.

About this issue

Original URL
State: open
Created 3 years ago
Reactions: 2
Comments: 17 (13 by maintainers)

Most upvoted comments

@damienwebdev Thank you, I feel less lonely on this one :p

In my case, I ended up removing product counts on that page, we didn’t need it after all…

I guess some reviewers did not see the issue correctly because they had their catalog_category_product_index filled by previous releases where it was still in use… So fixing the typo did fixed a part of the perf issue for them, if their category tree didn’t change much since then.

Nuranto on Apr 10, 2023

Additionally, in all likelihood, the original indexer here should be re-worked to use common table expressions as it is now supported in all major versions of db’s currently supported by Magento and there is no longer a need for the temporary index table.

Unfortunate for us however, Laminas doesn’t support it.

with recursive category_tree (entity_id, parent_id) as (
  select     entity_id,
             parent_id
  from       catalog_category_entity
  where      parent_id = 1
  union all
  select     c.entity_id,
             c.parent_id
  from       catalog_category_entity c
  inner join category_tree
          on c.parent_id = category_tree.entity_id
)
select parent_id as 'category_id', entity_id as 'child_id' from category_tree where parent_id > 1;

Notably, however, that parent_id should be carefully selected per root catalog that you’re trying to show in the admin UI.

For store0 it should 1 For store* it should be the rootCategory of that store.

damienwebdev on Apr 8, 2023

Hi,

internal team started working on this issue.

Thanks.

engcom-Bravo on Dec 15, 2023

@Nuranto’s original analysis here is correct. There was a bug fixed by https://github.com/magento/magento2/pull/35216

However, the actual problem here is not fully corrected by https://github.com/magento/magento2/pull/35216 but #35216 is part of the fix.

@Nuranto goes on to explain here: https://github.com/magento/magento2/pull/35216#issuecomment-1371194515 the actual issue in v2.4.3+ - including current 2.4-develop).

To be even more clear he was even kind enough to provide a PR here (that was closed): https://github.com/magento/magento2/pull/34111

In essence, for triage, the bug is as follows:

When Magento goes to compute the “edit category” screen for the root category, it loads a list of all of the categories to fulfill the tree UI structure. Importantly, part of the information retrieved for those categories is the number of products in that category.

Currently, because of two separate bugs, this computation is prohibitively expensive and (for me) results in the entire “Edit Categories” UI to be completely unusable.

The bugs are as follows:

A typo in the code remedied by https://github.com/magento/magento2/pull/35216
A completely inaccurate table name catalog_category_product_index that should be catalog_category_product_index_{WEBSITE_ID} - catalog_category_product_index is always empty leading to a net-negative load time because the call is always extraneous.

@Nuranto was kind enough to fix the second one here: https://github.com/magento/magento2/pull/34111 but it looks like it was closed. @sidolov it looks like (as part of the community upvotes, the first PR was merged, however it doesn’t actually fix the root issue, so please also focus on this issue AS IF it was as high priority as the now-merged PR).

damienwebdev on Apr 7, 2023

@engcom-Bravo, would be great if they will just port https://github.com/mage-os/mageos-magento2/pull/25 + https://github.com/mage-os/mageos-magento2/pull/47, so it won’t have 2 different implementation between the 2 forks

ihor-sviziev on Dec 18, 2023

@Nuranto that’s a really interesting insight that I had not considered. There’s likely a decent number of individuals with older databases where this table was never truncated and, as a result, the data probably looks somewhat correct (although in reality it is completely nonsensical).

Thanks for this realization, it will probably help along the way.

damienwebdev on Apr 10, 2023

✅ Confirmed by @engcom-Hotel. Thank you for verifying the issue.
Issue Available: @engcom-Hotel, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

m2-assistant[bot] on Aug 29, 2022