Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ps_crossselling : Smarty cache clearing can break payment flow #24338

Open
Matt75 opened this issue May 4, 2021 · 4 comments · May be fixed by PrestaShop/ps_crossselling#43
Open

ps_crossselling : Smarty cache clearing can break payment flow #24338

Matt75 opened this issue May 4, 2021 · 4 comments · May be fixed by PrestaShop/ps_crossselling#43
Labels
Checkout Cross selling Module: ps_crossselling Feature Type: New Feature FO Category: Front Office Module Module Needs Specs Status: issue needs to be specified Performance Label: Which BO under menu is concerned Waiting for PM Status: action required, waiting for product feedback

Comments

@Matt75
Copy link
Contributor

Matt75 commented May 4, 2021

Describe the bug

The module https://github.com/PrestaShop/ps_crossselling generate a lot of cache files on filesystem and clear cache at every order status change causing poor performances and issues during checkout.

It generate a lot of cache files due to cache key used, that's contains every id_product in current cart https://github.com/PrestaShop/ps_crossselling/blob/v2.0.1/ps_crossselling.php#L190

It clear cache at every order status changes
https://github.com/PrestaShop/ps_crossselling/blob/v2.0.1/ps_crossselling.php#L108

That's cause poor performances and sometime issues during checkout when PrestaShop doing the order create process.

Expected behavior

Avoid clear cache on Order Status change and stop generate too much cache files on filesystem.

Steps to Reproduce

Steps to reproduce the behavior:

Very hard to reproduce so don't loose time to try, it require a shop with many users at same time to create reproductible conditions

  1. Install https://github.com/PrestaShop/ps_crossselling
  2. Have a lot of files in cache on filesystem (Add product to cart, go to cart page, repeat multiple times)
  3. Have some concurrent process on filesystem
  4. Make a payment
  5. If you're unlucky you will encountered an error 500

Stack trace

UnexpectedValueException: RecursiveDirectoryIterator::__construct(/app/cache/prod/smarty/cache/ps_crossselling/19469/1/1/1/1/1/10/4e/42): failed to open dir: No such file or directory
#17 vendor/prestashop/smarty/sysplugins/smarty_internal_cacheresource_file.php(196): __construct
#16 [Anonymous function](0): getChildren
#15 vendor/prestashop/smarty/sysplugins/smarty_internal_cacheresource_file.php(196): clear
#14 vendor/prestashop/smarty/Smarty.class.php(845): clearCache
#13 classes/Tools.php(3021): clearCache
#12 classes/module/Module.php(2391): _clearCache
#11 modules/ps_crossselling/ps_crossselling.php(115): _clearCache
#10 modules/ps_crossselling/ps_crossselling.php(110): hookActionOrderStatusPostUpdate
#9 classes/Hook.php(924): coreCallHook
#8 classes/Hook.php(328): callHookOn
#7 classes/Hook.php(860): exec
#6 classes/order/OrderHistory.php(403): changeIdOrderState
#5 classes/PaymentModule.php(737): validateOrder
#4 modules/ps_checkout/src/ValidateOrder.php(195): validateOrder
#3 modules/ps_checkout/controllers/front/validate.php(121): postProcess
#2 classes/controller/Controller.php(242): run
#1 classes/Dispatcher.php(428): dispatch
#0 index.php(28): null

Additional information

Reported by Sentry

  • PrestaShop version: 1.7.3.3
  • PHP version: 7.1.33
@hibatallahAouadni
Copy link
Contributor

Hello @Matt75

It seems your issue is similar to this one #20874
And your PS version is 1.7.3.3 or 1.7.7.3?

If you're unlucky you will encountered an error 500

Well, I tried a few times but couldn't reproduce it (I think I'm lucky 😁 )
Please check and feedback.

Thanks!

@hibatallahAouadni hibatallahAouadni added 1.7.3.3 1.7.7.3 Affects versions Bug Type: Bug Checkout Cross selling Module: ps_crossselling FO Category: Front Office NMI Status: issue needs more information labels May 5, 2021
@Matt75
Copy link
Contributor Author

Matt75 commented May 5, 2021

Hi @hibatallahAouadni

Both issues has common issue with performances but my issue is the module break the order process after payment.
So customer has paid but no order created in PrestaShop or order has been created but with inconsistancies...

@Matt75 Matt75 changed the title ps_crossselling : Smarty cache management cause poor performances ps_crossselling : Smarty cache clearing can break payment flow May 5, 2021
@hibatallahAouadni hibatallahAouadni added Improvement Type: Improvement Performance Label: Which BO under menu is concerned Needs Specs Status: issue needs to be specified Waiting for PM Status: action required, waiting for product feedback and removed Bug Type: Bug NMI Status: issue needs more information labels May 11, 2021
@Matt75 Matt75 mentioned this issue Aug 13, 2021
Closed
@Matt75
Copy link
Contributor Author

Matt75 commented Aug 13, 2021

We have a merchant with more 50 orders not created after payments every days !
So I made a Pull Request on this module because Sentry reports keep going.

@hibatallahAouadni hibatallahAouadni added the PR available Solution: issue is being addressed label Aug 15, 2021
@marionf marionf added Feature Type: New Feature and removed Improvement Type: Improvement labels Dec 28, 2021
@MatShir MatShir added the Module Module label Dec 28, 2021
@hibatallahAouadni hibatallahAouadni removed 1.7.3.3 1.7.7.3 Affects versions labels May 23, 2022
@hibatallahAouadni hibatallahAouadni removed the PR available Solution: issue is being addressed label Jun 6, 2022
@lmeyer1
Copy link
Contributor

lmeyer1 commented Jul 22, 2022

@kpodemski
I think there are two flaws that combined give this issue:

  • The cache key is not well calculated. Currently all product ids are concatenated, not even in a specific order. This can indeed lead to an explosion of the number of cached templates. Given 1000 articles, a max of 3 articles per cart, and the max number of templates is 1000 + 2 * 1000 ^2 + 3 ! * 1000 ^3 = 6002001000 files !
  • There is no need to empty the cache on every order state update.

The solution to the latter is what @Matt75 implemented (remove actionOrderStatusPostUpdate hook and replace with actionObjectProductUpdateAfter and actionObjectProductDeleteAfter hook).
The solution to the former is to never cache a template, if there are more than one product_id. This allows to cache the results on the product detail page, while avoiding the issue stated above.

Do you think there are other things to take into account about this performance issue?

This was referenced Jul 22, 2022
Closed
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Checkout Cross selling Module: ps_crossselling Feature Type: New Feature FO Category: Front Office Module Module Needs Specs Status: issue needs to be specified Performance Label: Which BO under menu is concerned Waiting for PM Status: action required, waiting for product feedback
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Cache management with lifetime lmeyer1/ps_crossselling
Cache management lmeyer1/ps_crossselling
5 participants
@Matt75 @lmeyer1 @marionf @hibatallahAouadni @MatShir