Ipstenu / varnish-http-purge

Proxy Cache Purge
Apache License 2.0
46 stars 47 forks source link

WooCommerce checkout causes 6 calls to VarnishPurger->purge_post per cart item #69

Closed gerdneuman closed 4 years ago

gerdneuman commented 6 years ago

This is a spin-off of #63.

This "bug" applies to simple as well as variable products. This happens whenever an item has a restricted stock / inventory (see https://docs.woocommerce.com/document/managing-products/#section-6 and look for "stock management"), so they you sell physical goods (coffee, T-Shirts) then whenever an item is sold, then the inventory is reduced by that number. This causes to post to be saved.

Not sure why this happens but for every item in a cart the purge functionality (VarnishPurger->purge_post) is called 6 times. So when someone has an order of say 5 products, then it would be called 6 x 5 = 30 times. Each time for the $urls the purge response is done. It's about 20 URLs each time. This makes 30 x 20 = 600 purge requests in each checkout.

On our server the varnish backend is nearby, that is, a response is about 50-100ms. Still this makes 50-100ms x 600 requests = 30000-60000ms, that is 30 to 60 seconds delay just because of the varnish http purge plugin :-(

gerdneuman commented 6 years ago

I looked into it and WooCommerce only triggers the purge_post and consequently the vhp_purge_urls twice. Only when adding Polylang it is called 6 times. Nonetheless even twice per cart item is a lot, especially since most URLs are the same each time (feed URLs, author, blog).

@Ipstenu But I think I've came up with an idea based on how it is outlined in https://github.com/Ipstenu/varnish-http-purge/issues/5#issuecomment-414261460. This would also fix #65. The idea is as follows:

Have a static (kind a global) array for the $purge_urls. On each call to vhp_purge_urls just do a static/global $purge_urls = array_unique( array_merge (static/global $purge_urls, $newUrls). Then using the shutdown action to just execute on these pilled up URLs. This would also greatly help with bulk editing (I guess very similar to checkouts: edits on multiple pages) where you also just have a list of almost equal URLs (feeds, author, blog) and it would be fine to accumulate these in an array and only do the PURGE request and the very end.

What you think? I'd be happy to add a PR if that's the way to go?

...10 min later... :)

Hmm, looking through the code it seems like it already uses shutdown:

https://github.com/Ipstenu/varnish-http-purge/blob/b2c13647cd56dfbc23212d8615e78046075fad3e/varnish-http-purge.php#L170

But I do not understand how this can work to accumulate (merge) the incoming URLs into it. The code just seems to assign the new value each time but not to merge:

https://github.com/Ipstenu/varnish-http-purge/blob/b2c13647cd56dfbc23212d8615e78046075fad3e/varnish-http-purge.php#L844

So I guess only for the last round the purges are executed (that is the last item/product in the cart) because $purge_urls is always re-assigned / overwritten with the newest lates array.

Also, not sure but I think that the variable should be static so that it only exists once per request (might also work if the VarnishPurger is only once instantiated, but this might be a source for bugs):

https://github.com/Ipstenu/varnish-http-purge/blob/b2c13647cd56dfbc23212d8615e78046075fad3e/varnish-http-purge.php#L42

gerdneuman commented 6 years ago

Side note: Interestingly I just saw now that vhp_purge_urls is also called 6 times when saving a normal blog post (like for products). For a page it is only called once (correct). So it looks like a blog post might be the best way to start debugging because there it is also called way to often.

Ipstenu commented 6 years ago

I can't figure out why it's calling it 6 times for you, since I can't reproduce that EXCEPT on a site where I've heavily integrated multiple post types. So saving one type triggers a save on another page, which causes it to be called twice. But that's it.