-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Cache: RedisAdapter connects during cache:warmup (at least for Doctrine) #24865
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
Comments
This one is important to fix. /cc @nicolas-grekas |
Tangentially related to this: thanks to the same lack of laziness in Doctrine, the Doctrine |
@weaverryan This one is more tricky as Doctrine needs the version. So, passing a fake DATABASE_URL is kind of required to pass the DB version so that Doctrine does not attempt to connect to the database for real to get the version. If we can fix that as well, that would be FANTASTIC. |
Im facing the same issue as @weaverryan, im building a micro-service using flex and 3.3 for now till 4.0 is released to be build and deployed as docker container, i did a workaround setting fake env vars just to get composer install pass, but that is not ideal it should not depend on that variable and allow the docker image to be more clear and final allowing us to deploy the container with the correct env vars |
Yeah, these two came up while i've been trying to dockerize a sf4 install. Thanks @weaverryan for making the issue for me :D |
This was left as an exercise to the user :)
--- a/config/services.yaml
+++ b/config/services.yaml
@@ -26,3 +26,9 @@ services:
App\Controller\:
resource: '../src/Controller'
tags: ['controller.service_arguments']
+
+ cache.default_redis_provider:
+ class: Redis
+ factory: [Symfony\Component\Cache\Adapter\RedisAdapter, createConnection]
+ arguments: ['%env(REDIS_URL)%']
+ lazy: true But I guess we can make RedisAdapter accept a DSN, then create the connection lazily... I'm on it. |
It would be great to have a similar solution for doctrine @nicolas-grekas |
Doctrine already has a lazy-loading mechanism bundled, can't do much more here. |
But doctrine is requiring the environment variables to be defined at cache warmup time as @weaverryan mention in previous comment, that makes it obscure at docker build time so we need to define a fake environment variable just to complain with the cache warmup. |
@fabpot @weaverryan can the server version be provided to doctrine using a regular parameter instead? That's the only solution I can think of for this issue. |
Actually, I think the server version problem is a misconception. I believe that (at least mostly, if not entirely), the issue of Doctrine trying to establish a database connection during cache warmup has been fixed (in a few places, things have been fixed to be more lazy). The problem here is A)
B) The C) The instantiation of the I'm not sure how to break that dependency chain. The Ideas include:
|
On my case i use DoctrineMongoDBBundle and have no cache configuration that implies directly a doctrine connection and still get the exception from doctrine requiring the environment variable to be defined |
This is not solvable without setting a default value. The same issue will happen with the redis proxy cache btw. |
See #24887 for adding laziness to the redis cache. |
The same issue affects the Lock component when a cache warmer has a dependency on a LockInterface (configured with redis in the backoffice) reproduct case on top of @weaverryan 's sample project diff --git a/composer.json b/composer.json
index 87e9ddb..0cad6fc 100644
--- a/composer.json
+++ b/composer.json
@@ -9,6 +9,7 @@
"symfony/console": "^4.0",
"symfony/flex": "^1.0",
"symfony/framework-bundle": "^4.0",
+ "symfony/lock": "^4.0@beta",
"symfony/lts": "^4@dev",
"symfony/yaml": "^4.0"
},
diff --git a/config/packages/framework.yaml b/config/packages/framework.yaml
index 919fbbb..e312a1f 100644
--- a/config/packages/framework.yaml
+++ b/config/packages/framework.yaml
@@ -12,6 +12,7 @@ framework:
#fragments: ~
php_errors:
log: true
+ lock: "%env(REDIS_DSN)%"
cache:
app: cache.adapter.redis
default_redis_provider: "%env(REDIS_DSN)%"
\ No newline at end of file
diff --git a/src/CacheWarmer/Foo.php b/src/CacheWarmer/Foo.php
new file mode 100644
index 0000000..f428a8d
--- /dev/null
+++ b/src/CacheWarmer/Foo.php
@@ -0,0 +1,22 @@
+<?php
+
+namespace App\CacheWarmer;
+
+use Symfony\Component\HttpKernel\CacheWarmer\CacheWarmerInterface;
+use Symfony\Component\Lock\LockInterface;
+
+class Foo implements CacheWarmerInterface
+{
+ public function __construct(LockInterface $lock)
+ {
+ }
+
+ public function warmUp($cacheDir)
+ {
+ }
+
+ public function isOptional()
+ {
+ return false;
+ }
+} |
…s (nicolas-grekas) This PR was merged into the 3.4 branch. Discussion ---------- [Cache][Lock] Add RedisProxy for lazy Redis connections | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #24865 | License | MIT | Doc PR | - That's the only provider that is not lazy by default, leading to bad DX (see linked issue.) Best reviewed [ignoring whitespaces](https://github.com/symfony/symfony/pull/24887/files?w=1). Commits ------- 1f5e353 [Cache][Lock] Add RedisProxy for lazy Redis connections
Does that commit fix the DATABASE_URL issue too, or would you like another issue for that? Doesn't look like it does. |
Could you create a separate issue so we can track that Aaron? |
@aequasi it doesn't:
|
May be this help: |
Same issue here. Redis is used for Query/Result and Second Level Cache and it's initialized during |
Same issue. I think we need to go to Doctrine's team, as i see separation of configuration and metadata from entity manager is the only solution to build DI during Docker build and as a result get immutable docker image :( |
Still having this issue with the symfony/docker building stage: Had to remove the cache build to prevent it connecting to Redis from Doctrine. Also fails when installing assets |
Did you found a proper workaround or created a ticket for this with Doctrine? |
Is there a way to just create the symfony caché container without calling external services like redis? I'm using in my Dockerfile:
But CachePoolClearerCacheWarmer is not optional, so if the cache.pool uses redis, it breaks. One way could be use a ENV parameter to "mock" the redis adapter, but the container cache will be wrong for prod |
Steps to repeat:
git clone git@github.com:weaverryan/flex-redis-bug.git
composer install
It will explode with:
The project is a standard Flex app, but with the following changes:
A) Doctrine ORM is installed
B) The
app
cache uses redis (seeframework.yaml
)C) We're in the
prod
environmentThe error happens on
cache:clear
. This comes from the result cache configured by default inprod
for Doctrine: https://github.com/symfony/recipes/blob/93d80647f510e00a52db869251136593a2dc2182/doctrine/doctrine-bundle/1.6/config/packages/prod/doctrine.yaml#L28This is caused by a lack of laziness... somewhere. It could be in Doctrine - this whole thing starts when we try to warmup the proxies... which instantiates the entity managers (which instantiates result caches and also connection objects). Or, it could be in the Cache component, where (at least with Redis) a connection to the Redis server is established in order to create the
RedisAdapter
object (even if you never make any connections to it).Thanks!
The text was updated successfully, but these errors were encountered: