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.

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

[BUG] Cache storage issue #20

Open
Nuranto opened this issue Oct 26, 2023 · 4 comments
Open

[BUG] Cache storage issue #20

Nuranto opened this issue Oct 26, 2023 · 4 comments

Comments

@Nuranto
Copy link

Nuranto commented Oct 26, 2023

Describe the bug
A clear and concise description of what the bug is.

Preconditions

  • Magento 2 version: 2.4.6-p3
  • CrowdSec_Bouncer module version: 2.0.0

To Reproduce
Steps to reproduce the behavior:

  1. Go to module's configuration, cache config section

Expected result
A clear and concise description of what you expected to happen.

  • Be able to configure redis with native M2 redis classes.
  • Be able to use filesystem on multi-nodes infrastructure

Actual result
A clear and concise description of the actual result.

  • We're forced to install phpredis to use redis
  • On filesystem mode, files are stored locally, only on the pod running the cron. So others pod don't have any decisions. Would have been better to store this in a remote storage like import_export for example.
@julienloizelet
Copy link
Collaborator

Hi @Nuranto ,
Thanks for your message.

Regarding Redis, I only found documentation that's say that the PHP Redis extension is required anyway (https://www.cloudways.com/blog/magento-redis/#Magento-Redis-Requirements for example); But maybe it's not ?

Regarding the filesystem mode, can you tell me more about what you mean by "a remote storage like import_export" ?
I found the remote storage feature https://experienceleague.adobe.com/docs/commerce-operations/configuration-guide/storage/remote-storage/remote-storage.html?lang=en but I'm not sure this is related.

Thanks again

@Nuranto
Copy link
Author

Nuranto commented Oct 26, 2023

Hi @julienloizelet

Thanks for your quick answer !

For redis, Magento is using https://github.com/colinmollenhour/credis lib, which supports phpredis, but also propose a custom implementation. A lot of Magento users are using this custom implementation, because a lot of bugs occured with phpredis (Although I don't know the current state of this, may be it is stable now.)

For the filesystem, Magento propose a remote storage adapter (S3 or S3-like) which, as redis, prevents the need of shared volumes. Magento comes with two remote folders : media folder, and var/import_export. If you can access it through $filesystem->getDirectoryWrite(DirectoryList::VAR_IMPORT_EXPORT)

Actually, why don't use Magento's cache system directly, by adding a new cache type ? https://developer.adobe.com/commerce/php/development/cache/partial/cache-type/
With that approach, we would'nt have to bother with cache setup anymore when installing your extension. It would also simplify your extension's architecture.

@julienloizelet
Copy link
Collaborator

@Nuranto , thank you for this clarification.

Using a new type of Magento cache is certainly a good option. We didn't do it because the Magento module is built on top of a more generic CrowdSec PHP library that provides 3 Symfony "ready-to-use" cache systems (Redis, Filesystem and Memcached). This more generic lib is aimed to be used as a quick starter for implementing a bouncer in various PHP frameworks (WordPress, Drupal, etc). It should be extendable enough to handle a new cache type but it will require some work.

Regarding the filesystem issue, the /var/cache/crowdsec path value is hard-coded . Do you think it could resolve the problem to let a user choose a different path (maybe relative to the var folder) ? If yes, it could be a quick win to add a new setting for this.

What do you think ?

@Nuranto
Copy link
Author

Nuranto commented Oct 27, 2023

Ok, I see.

No changing the path won't solve the issue if you're not using Magento's filesystem classes (without it, it won't use the remote adapter, and still write files locally).
I guess we'll have to either disable stream mode, or give another try to phpredis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants