8000 Cache: RedisAdapter connects during cache:warmup (at least for Doctrine) · Issue #24865 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

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

Closed
weaverryan opened this issue Nov 7, 2017 · 24 comments
Closed

Comments

@weaverryan
Copy link
Member
Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? no
Symfony version 3.2?

Steps to repeat:

  1. git clone git@github.com:weaverryan/flex-redis-bug.git

  2. composer install

It will explode with:

screen shot 2017-11-07 at 3 15 49 pm

The project is a standard Flex app, but with the following changes:

A) Doctrine ORM is installed

B) The app cache uses redis (see framework.yaml)

C) We're in the prod environment

The error happens on cache:clear. This comes from the result cache configured by default in prod for Doctrine: https://github.com/symfony/recipes/blob/93d80647f510e00a52db869251136593a2dc2182/doctrine/doctrine-bundle/1.6/config/packages/prod/doctrine.yaml#L28

This 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!

@fabpot
Copy link
Member
fabpot commented Nov 7, 2017

This one is important to fix. /cc @nicolas-grekas

@weaverryan
Copy link
Member Author

Tangentially related to this: thanks to the same lack of laziness in Doctrine, the Doctrine Connection object is also instantiated. This means that a DATABASE_URL environment variable must be available at cache:warmup time. It doesn't need to be valid (no connection is make), but it does need to be defined:

screen shot 2017-11-07 at 3 35 32 pm

@fabpot
Copy link
Member
fabpot commented Nov 7, 2017

@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.

@bitgandtter
Copy link
Contributor

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

@cryptiklemur
Copy link
Contributor

Yeah, these two came up while i've been trying to dockerize a sf4 install. Thanks @weaverryan for making the issue for me :D

@nicolas-grekas
Copy link
Member
8000

This was left as an exercise to the user :)
Here is the solution:

  • composer req proxy-manager-bridge
  • apply
--- 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.

@bitgandtter
Copy link
Contributor

It would be great to have a similar solution for doctrine @nicolas-grekas

@nicolas-grekas
Copy link
Member

Doctrine already has a lazy-loading mechanism bundled, can't do much more here.

@bitgandtter
Copy link
Contributor

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.

@nicolas-grekas
Copy link
Member

@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.

@weaverryan
Copy link
Member Author

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 ProxyCacheWarmer. If I return immediately, then I cache warmup the cache without specifying a DATABASE_URL. The dependency chain is:

A) ProxyCacheWarmer instantiates all the EntityManagers (

foreach ($this->registry->getManagers() as $em) {
)

B) The EntityManager requires the Connection

C) The instantiation of the Connection triggers the error (from the built container) that the DATABASE_URL environment variable is not present.

I'm not sure how to break that dependency chain. The Connection should not be needed in order to get the metadata/proxy information. But that's buried in Doctrine. At the same time, creating a Connection object is not really a problem...since no queries are actually made. But since the container checks for the existence of the environment parameter, we hit the issue.

Ideas include:

  1. Adding something to make environment variables optional? Kinda doesn't really address the problem (the Connection being instantiated).

  2. Create duplicated, "fake" entity manager services, where the Connection is some dummy, not-real connection object. Then inject these "fake" entity manager services into ProxyCacheWarmer and use them to get the metadata needed. This seems a little nuts, but would indeed work...

@bitgandtter
Copy link
Contributor

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

@nicolas-grekas
Copy link
Member

This is not solvable without setting a default value. The same issue will happen with the redis proxy cache btw.
The only solution to now have this is a lazy proxy from proxy-manager.
Which means there is no solution by default.

@nicolas-grekas
Copy link
Member
nicolas-grekas commented Nov 9, 2017

See #24887 for adding laziness to the redis cache.
But as stated, the env var will have to be defined at build time, even if a dummy value is used.
The only way around is using a real lazy proxy from proxy-manager, which has tighter integration with the DI component and is able to call a factory lazily.

@jderusse
Copy link
Member

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;
+    }
+}

fabpot added a commit that referenced this issue Nov 10, 2017
…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
@fabpot fabpot closed this as completed Nov 10, 2017
@cryptiklemur
Copy link
Contributor
cryptiklemur commented Nov 10, 2017

Does that commit fix the DATABASE_URL issue too, or would you like another issue for that?

Doesn't look like it does.

@weaverryan
Copy link
Member Author

Could you create a separate issue so we can track that Aaron?

@nicolas-grekas
Copy link
Member

@aequasi it doesn't:

The only way around is using a real lazy proxy from proxy-manager, which has tighter integration with the DI component and is able to call a factory lazily.

@tecnocat
Copy link
Contributor

May be this help:

#25157 (comment)

@Warxcell
Copy link
Contributor

Same issue here. Redis is used for Query/Result and Second Level Cache and it's initialized during cache:clear which in our case happens during docker build and breaks the build stage.

@sokil
Copy link
sokil commented May 27, 2022

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 :(

@WebDaMa
Copy link
WebDaMa commented Feb 22, 2023
AAE1

Still having this issue with the symfony/docker building stage:

Had to remove the cache build to prevent it connecting to Redis from Doctrine.
Running the cache:clear on deploy, not ideal.

Also fails when installing assets

@WebDaMa
Copy link
WebDaMa commented Mar 3, 2023

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 :(

Did you found a proper workaround or created a ticket for this with Doctrine?

@JoniJnm
Copy link
JoniJnm commented Aug 22, 2024

Is there a way to just create the symfony caché container without calling external services like redis?

I'm using in my Dockerfile:

bin/console cache:warmup --no-optional-warmers --env=prod

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

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

No branches or pull requests

0