Smile-SA / elasticsuite

Smile ElasticSuite - Magento 2 merchandising and search engine built on ElasticSearch
https://elasticsuite.io
Open Software License 3.0
762 stars 341 forks source link

Enhance performance in admin category page for categories with many children #1798

Open dambrogia opened 4 years ago

dambrogia commented 4 years ago

Is your feature request related to a problem? Please describe.

We are facing performance issues in the admin when editing/saving categories with more than just a couple children. My understanding is that this stems from loading categories for the preview (new relic trace provided). All child categories of the current category are loaded recursively and this causes large performance degradation when loading more than 10+ categories for us. We're experience 40+ second load times and 504's (with a 60s TTL) when trying to save a category.

Describe the solution you'd like

Describe alternatives you've considered To work around this issue, for now we've considered disabling previews for categories with more than $x children. This is "ok", since we prefer to use optimizers to sort our catalog rather than manual sort, but we do feel like we're missing an opportunity to use provided functionality.

We have also already opted to disable previews for categories that have a static block display, we believe the preview in this situation is pointless no matter what. As a rule of thumb, our categories with the most children have static block views, so this especially has made an immediate impact.

Additional context Here is our new relic trace. This is even on a less-extreme example (~25 children total in this category).

This is in our future production environment so we have a very large and (intentionally) over-sized database via aws's RDS. Our Elastic is aws' elasticsearch service running a cluster size of 2 r5.xl's.

Edit: Also, I'm using 2.8.3, I will be updating to 2.8.6 sometime this week (we have an open PR that has other dependent changes before it gets merged). I know that in 2.8.4 there was a relevent change to the Rule.php model, however after looking at it I don't think this will make a large difference in performance for what i have mentioned, but none the less I will do my due diligence and keep an eye on it.

image

romainruaud commented 4 years ago

Hi @dambrogia

Just for us to know, how much virtual categories are involved in the category tree you are showing ?

dambrogia commented 4 years ago

Hi @romainruaud, I don't recall exactly which category. I've implemented a workaround to remove previews for categories with children for the time being to work around it. So I no longer get these stack traces in our APM. But i can say that after about 10-15 ancestors of the parent category we can start to notice the performance issues of the extra calls.

It seems that rather than recursively running through parent -> children, it would be better to look at the path attribute of the parent and know that anytime the id exists but is not the last number, that is an child/grandchild/ancestor of the parent category. Admittedly, i do not know how feasible this query is in elasticsearch but it is somewhat common in MySQL at least.

For example:

SELECT 
    entity_id, 
    path,
    REPLACE(path, '/', ',') as path_csv
FROM 
    catalog_category_entity 
WHERE 
    entity_id != 47
HAVING 
    find_in_set(47, path_csv) <> 0

This returns a set in one fetch command like so:

    entity_id   path                        path_csv
    .....
    '1354',     '1/2/47/48/1354',           '1,2,47,48,1354'
    '2980',     '1/2/47/113/2980',          '1,2,47,113,2980'
    '3523',     '1/2/47/3523',              '1,2,47,3523'
    '3524',     '1/2/47/3523/429/3524',     '1,2,47,3523,429,3524'
    '3525',     '1/2/47/3523/429/3525',     '1,2,47,3523,429,3525'
    '3526',     '1/2/47/3523/429/3526',     '1,2,47,3523,429,3526'
    '3527',     '1/2/47/3523/429/3527',     '1,2,47,3523,429,3527'
    '3528',     '1/2/47/3523/429/3528',     '1,2,47,3523,429,3528'
    '3529',     '1/2/47/3523/429/3529',     '1,2,47,3523,429,3529'
    '3530',     '1/2/47/3523/3530',         '1,2,47,3523,3530'
    '3531',     '1/2/47/3523/3531',         '1,2,47,3523,3531'
    .....
dverkade commented 4 years ago

I can confirm this issue is still present in 2.8.9. It's loading all the children categories. We have load times of over 1 minute and up. See screenshot from New Relic. I don't think this qualifies as a "Feature" but should be handled as a "bug" because it's such a common use case.

image

dambrogia commented 1 year ago

I would like to bump this as we had some issues on black friday regarding a similar situation. However this time the issue was in the frontend when compiling a virtual rule rather than in the backend previewing a category.

image

If continued into the stack trace, we can see further looping/recursion going on:

image

We are now on Magento 2.4.4 and ElasticSuite version 2.10.10.

Edit: I would agree this should not be tagged as a feature, but rather an enhancement if anything.