dc-woocommerce-multi-vendor icon indicating copy to clipboard operation
dc-woocommerce-multi-vendor copied to clipboard

Memory Leak! Custom $where results to ram leak when wcmp is enabled

Open jobs-git opened this issue 6 years ago • 7 comments

As the title says, the leak happens on PHP and MYSQL, both increases in ram indefinitely after conducting a search query from the woocommerce search bar.

Here is a sample code (Works great on wcmp v. 2)

add_filter( 'posts_where' , 'posts_where_statement' );
function posts_where_statement($where1) {
    if(!is_search()){
        return $where1;
    }
    $where1 = "any where";
    return $where1;
}

jobs-git avatar Apr 13 '18 06:04 jobs-git

Hi Dominic, We have tried to reproduce the problem on our end using posts_where filter but it works fine. Moreover, WCMp is not using this filter (posts_where) anywhere. Let me know if you can shed some more light on this. Thanks.

sushobhan-pal avatar Apr 27 '18 12:04 sushobhan-pal

Retested it again on WP 4.9.5 and WC 3.3.4/3.3.5, that was just the same function code above and the memory leak in php 7.1/7.2 and mysql only happens when wcmp 3.0.5 is enabled. Retested also on different servers, same thing.

When I disabled it, woocommerce product search runs smooth. Also reverting back to wcmp 2 did not manifest this issue.

I'm pretty sure there is nothing wrong with the code above. Substituting "any" where statement guarantees reproduction of the issue.

Addtnl Info: Database is fully loaded in RAM, entries contains 1M dummy products. Moreover, no errors or unusual activity present in the logs.

jobs-git avatar Apr 28 '18 21:04 jobs-git

Hello Dominic,

The fix you have suggested by removing the action "woocommerce_shop_loop", is breaking our current logic. The purpose of the function is to return the product with minimum price, if the same product is sold by multiple vendors.

In shop or category page, WCMp shows only one instance of a product that is sold by more than one vendors. For that we hook in "woocommerce_product_query" to modify query by passing 'post_parent = 0'. Then using "woocommerce_shop_loop" hook, we return the product with the minimum price.

In future, we are thinking about using a taxonomy based approach instead. This might take some time. We will keep you updated.

Just to be sure, I'll request you to remove the action "woocommerce_product_query" as well and test with your data. Let us know what impact it has on your query timing.

Lastly, will it be possible for you to provide us the product database? We do not have test set more than several thousand. So a test set of 1M+ products will certainly help us to test more rigorously.

sushobhan-pal avatar Apr 30 '18 12:04 sushobhan-pal

Hi,

Yes, as I suspect it does something new, but returning the lowest price must not be default cause

  1. ebay, amazon, aliexpress does not implement it, I think they return results based on their algoritm
  2. it usually defaults to relevance, there is a sort to lowest price (but current algo seems to semi-superceed this)
  3. probably let developers choose their own algo

but I get it that wc devs wants to make it more user friendly so maybe improve the function so it does not cause memory leak.

I will send you the database when I get access to fast network. In case I got too long I made the dummy products using Excel. However, I dont have taxonomies for it. Its just wp_posts table.

jobs-git avatar Apr 30 '18 17:04 jobs-git

In future, we intend to provide various other vendor selection criteria like verified vendor, featured vendor, min price, max price, star ratings etc. As of now, it's only the minimum price. Though, we are providing a hook to change this default behavior. If you provide us with the DB backup then it will be of immense help for our testing team. Thank you.

sushobhan-pal avatar May 01 '18 08:05 sushobhan-pal

Here is the link for the complete database containing 1 Million product entries. The files would only be available for 2 weeks from a free service site since github could not upload files larger than 10MB.

The database were only populated in wp_posts so there would be no taxonomies for the entries except for the default products.

jobs-git avatar May 23 '18 03:05 jobs-git

Thanks, Dominic. This will certainly help our testers. 👍

sushobhan-pal avatar May 24 '18 11:05 sushobhan-pal