-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Cache] fix sharing cache between apps #35723
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
Conversation
@@ -52,8 +52,8 @@ public function process(ContainerBuilder $container) | |||
$seed = '.'.$container->getParameterBag()->resolveValue($container->getParameter('cache.prefix.seed')); | |||
} else { | |||
$seed = '_'.$container->getParameter('kernel.project_dir'); | |||
$seed .= '.'.$container->getParameter('kernel.container_class'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also think we should set this combined seed as default in
symfony/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
Lines 872 to 874 in f46ab58
->scalarNode('prefix_seed') | |
->info('Used to namespace cache keys when using several apps with the same shared backend') | |
->example('my-application-name') |
Then the default behavior is obvious and people understand why they might want to change this value.
The configurable cache prefix seed does not give full control over the cache prefix because the container class is added to the prefix in any case. This is a problem because the container class contains the app env name. We use different app environments for different deployment targets (dev and test). Dev and test should use the same redis cache. But this is impossible to achieve because even setting the cache prefix seed does not accomplish this.
I fear removing this parameter from the seed might create other kinds of problems. |
Dev and test are meant to share some data caches and updating the cache on one should be reflected on the other. But this does not work currently. I think giving people the possibily to do something like this is important. |
Removing this parameter from the seed not only means dev and test will share the same storage, but also prod will. |
Yes having it separated by default is fine. And it still is. This PR is about when people set the cache prefix seed manually so they actually have control over cache separation. |
Then this should be for master to me - we don't want to force-invalidate existing caches in a bug fix release. We should also change https://github.com/symfony/recipes/blob/master/symfony/framework-bundle/3.3/config/packages/cache.yaml also: Note that I'm not getting the point of the change from a conceptual pov, but I'm still trying to be solution-oriented :) |
Thanks for your consideration. I think it's important to be able to have full control over the cache prefix. E.g. if you try to mix symfony and a legacy app you likely also want to share caches. I think there are alot of valid use-cases. I will do these changes in master then. |
PR for master: #35890 |
This PR was merged into the 5.2-dev branch. Discussion ---------- [Cache] give control over cache prefix seed | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tickets | | License | MIT | Doc PR | Reopened #35723 for master. The configurable cache prefix seed does not give full control over the cache prefix because the container class is added to the prefix in any case. This is a problem because the container class contains the app env name. We use different app environments for different deployment targets (dev and test). We want dev and test to use the same redis cache. But this is impossible to achieve because even setting the cache prefix seed does not accomplish this. Commits ------- 6681b92 [Cache] give control over cache prefix seed
The configurable cache prefix seed (#20610) does not give full control over the cache prefix because the container class is added to the prefix in any case. This is a problem because the container class contains the app env name. We use different app environments for different deployment targets (dev and test). We want dev and test to use the same redis cache. But this is impossible to achieve because even setting the cache prefix seed does not accomplish this.