jackalope/jackalope-doctrine-dbal

Attempting to connect to Database during cache clear/warmup.

Closed this issue · 8 comments

eddmann commented

Since Symfony v3.4.28 and more specifically this change https://github.com/symfony/symfony/pull/31335/files#diff-8f42c88228e4781e0afb89210b210ca2L76 we have noticed that whenever we attempt to use the console i.e. clear/warmup the cache, the Doctrine event listener defined within https://github.com/doctrine/DoctrinePHPCRBundle/blob/master/src/DependencyInjection/DoctrinePHPCRExtension.php#L194 attempts to now be initialised which looking through the call graph constructs an instance of https://github.com/jackalope/jackalope-doctrine-dbal/blob/master/src/Jackalope/Transport/DoctrineDBAL/RepositorySchema.php#L37 which in-turn attempts to connect to the database.

We are unsure if this is a known change since this version or could it be due to the way we have got it configured?

dbu commented

is this causing a direct error, or just unefficient?
as we tag it as the postGenerateSchema, i would have hoped it only gets instantiated when generating a dbal schema. but indeed, seems symfony wants to always instantiate it.

https://symfony.com/doc/current/service_container/lazy_services.html could be one option, but requires an additional package.

the other option seems that you specify to doctrine dbal what version of the database you use - it would then still create a schema, but not need to talk to your database to do it: https://www.doctrine-project.org/projects/doctrine-dbal/en/2.9/reference/configuration.html#automatic-platform-version-detection

eddmann commented

Thanks for the reply, this is causing us an issue as we do not have database access when we clear/warmup the cache.
We do specify the server_version explicitly but this still results in an attempted database connection.

dbu commented

ah, i see. yeah that is annoying.

can you see what triggers the database connection and if it can somehow be avoided within dbal?

i fear that delaying what we do in the constructor to some later step is quite difficult because a lot of methods could be the entry point to detect that we need to initialize the schema. if you see a feasable way to only need 2-3 public methods call the initialization, i could see moving the initialization out of the constructor, so that we only need the db connection when the schema is actually used. but if we have to change 15 methods, the coupling is too high (and fragile, as adding a new method in the base class could silently lead to unexpected behaviour)

eddmann commented

In our case we use Postgres and I can see that this causes the database connection https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Schema/PostgreSqlSchemaManager.php#L60, there does not look to be anyway of explicitly specifying this.

I'll take a look and see if a small change is possible.

eddmann commented

After a look it doesn't seem trival to change this. Instead I have opted for overriding the schema listener with. Not ideal but delays creating the RepositorySchema until required.

class LazyJackalopeDoctrineDbalSchemaListener
{
    private $container;

    public function __construct(ContainerInterface $container)
    {
        $this->container = $container;
    }

    public function postGenerateSchema(GenerateSchemaEventArgs $args)
    {
        $schema = $args->getSchema();
        $this->container->get('doctrine_phpcr.jackalope_doctrine_dbal.schema')->addToSchema($schema);
    }
}

This could have also been achieved using symfony/proxy-manager-bridge

dbu commented

that looks reasonable. i wonder if we should change the listener in the phpcr bundle to be lazy. we would start injecting the container, but that sounds better than the current problems and overhead (even when it works, it will slow down things quite a bit)

do you want to do a pull request to change the bundle implementation?

eddmann commented

Sure thing 👍