[2.15.0] Call to a member function removeQueryCacheProfile() on null

[PhilETaylor]

BC Break Report

Q	A
BC Break	yes
Version	2.15.0

Summary

Previously fully working live Symfony 6.3.x-dev/6.2 application working fine, until upgrade to doctrine/orm 2.15.0 this morning.

Previous behavior

The following code worked without issue in a standard Symfony repository

  public function getChangeLogSinceLastLogin($date): array
    {
        $query = $this->createQueryBuilder('o')
            ->orderBy('o.posted', Criteria::DESC)
            ->where('o.published = 1')
            ->andWhere('o.posted > :date and o.posted < :now')
            ->setParameter('date', $date)
            ->setParameter('now', date('Y-m-d H:i:s'))
            ->getQuery();

        return $query->getResult() ?? [];
    }

Current behavior

After updating to 2.15.0 the getResult method call fatal errors with

Call to a member function removeQueryCacheProfile() on null

How to reproduce

I can work on seeing if I can provide a reproducer after lunch, but posting here in case its a known issue?

[PhilETaylor]

Nevermind, removing the query cache drivers from the symfony configuration seems to fix this... not sure if this is Doctrine or Symfony now.

[derrabus]

But those settings should be fine. I'd still be interested in a reproducer.

[PhilETaylor]

I cannot reproduce it in a clean reproducer app :(

[PhilETaylor]

pin pointed it to a configuration of default_redis_provider in my app (https://symfony.com/doc/current/reference/configuration/framework.html#default-redis-provider)

$cache->defaultRedisProvider(env('REDIS_URL'))

if I remove this then it works as expected, trying to replicate that in a reproducer now

[PhilETaylor]

ok this was the thing that made me finally get to the root issue

note that "something" was still trying to set the old property with an underscore (which has changed in this version of orm)

This was in the Symfony Cache Marshaller, unserialising something it had retrieved from redis cache (that obviously is not cleaned/cleared with a normal symfony cache clear for some reason!!!)

As soon as I flushed the whole redis db (in dev, not prod!!!!!) I was then able to get the broken page to load, now checking redis I can see a cached "sqlExecutor"

So now I need to work out how to clean this up when deploying top production :-(

[flack]

Just chiming in to say that I'm seeing the same error, too in 2.15. I'm not using Redis (AFAIK), and not the full Symfony framework either. I don't have a good backtrace currently, but I can see if I can get one

[PhilETaylor]

Looking on my production redis server (that has not yet been deployed this new version of orm to) I can see the underscore property cached... so when this is unserialised, that is what is causing the problem.

[derrabus]

Okay, so it's a deserialization issue. Serialize a parsed ORM query with 2.14 and try to deserialize it with 2.15 and it bombs, did I get this right?

[PhilETaylor]

Correct.

So I guess the reason I could not reproduce it, is that the new version creates the correct cache which is then correctly deserialised.

I guess (as Im out of time and have a dentist appointment to run to) the reproducer needs to generate a cache with the old version of orm, and then you need to upgrade to the latest version, and have the latest version try to deserialise the cached ParserResult, which will then set the wrong property (the underscored one) and thus leaving the correct property (which no longer has an underscore in the latest version) null.

[flack]

Yes, was the same in my case: I use Apcu, and as soon as I restarted Apache (which clears the apcu cache) the error disappeared

[fezfez]

I got the same issue with mezzio using acpu cache

[curry684]

Can confirm upgrade issue, I'm using the Redis adapter for the query cache. Manually invoking flushdb on the relevant Redis DB fixed the issue.

Brings back to that it surprises me that Symfony DoctrineBundle doesn't automatically flush the Doctrine cache pools during bin/console cache:clear, why not?

[greg0ire]

Looking at the release notes, nothing stands out.
Possible next steps:

• try while upgrading doctrine/orm and only that. version 2.15.0 allows upgrading doctrine/instantiator, and maybe more packages if that has rippling effects
• try using git bisect to pinpoint what commit exactly is responsible for the issue.
• figure out why/how it works with 2.14: does it actually empty Redis, or is there a new namespace created (apparently that's how doctrine/cache used to work) That should answer @curry684 's question. Also, it would be great to figure out if you are using doctrine/cache or psr/cache, and if that somehow changes while upgrading.

[PhilETaylor]

Looking at the release notes, nothing stands out.

Apart from the fact that the private properties were changed from having underscores in them, so when the PRE-EXISTING PRE-UPDATE cached data is unserialised, it attempts to set the underscored private properties, and they dont exist ,hence the logged PHP 8 deprecations in my screenshot.

This is the root problem here. https://github.com/doctrine/orm/pull/10477/files#diff-9f60c320ee22b4779babc2248523b6792ff07b7dba0005a645dc76262b9e2eb3

[curry684]

figure out why/how it works with 2.14: does it actually empty Redis

Semi-offtopic but I did notice multiple times before, at least for a year or so now, that Doctrine cache pools were not flushed automatically. Just never bothered to escalate as it never gave issues before. As @PhilETaylor points out it's simply only an issue now because property paths changed, before this the caches were (accidentally?) always backwarde compatible.

[flack]

would be nice if the serialized objects had some sort of version number, then the cached data could be ignored by newer releases

[curry684]

would be nice if the serialized objects had some sort of version number, then the cached data could be ignored by newer releases

That's pretty much impossible as you'd need to deserialize the object to get to the version number. There's a reason default Composer config automatically runs cache:clear after install/update, because caches are in general not expected or required to be version-compatible. It's also implicitly useless as you'd be consciously polluting the cache with objects that will never be used again. Caches are by their definition meant to be expendable and software upgrades are the designated moment to expend them.

[flack]

or a cache namespace, then

[greg0ire]

Yes. Flushing seems sub-optimal since it would mean a performance loss during a deploy. And indeed, it could have happened with any previous release. I would not say the root issue is that underscores were removed, I'd say the root cause is expecting the cache to always stay backwards-compatible, and not bumping a version number in the namespace (if there is still one).

Possible next steps:

• figure out what tools doctrine/orm exposes to deal with this. Apparently, you can delete, but you can also invalidate:
  • https://github.com/doctrine/orm/blob/60c625be17668f905cd2c1f73b48ab9b7a105755/lib/Doctrine/ORM/Tools/Console/Command/ClearCache/MetadataCommand.php#L27
  • https://github.com/doctrine/orm/blob/60c625be17668f905cd2c1f73b48ab9b7a105755/lib/Doctrine/ORM/Tools/Console/Command/ClearCache/QueryCommand.php#L36
  • https://github.com/doctrine/orm/blob/60c625be17668f905cd2c1f73b48ab9b7a105755/lib/Doctrine/ORM/Tools/Console/Command/ClearCache/ResultCommand.php#L38
• try using these tools instead of directly flushing redis / restarting apache. Does it fix the issue?
• figure out how these tools are integrated in the bundles
• if there is an integration, figure out why it does not kick in.

[curry684]

Flushing seems sub-optimal since it would mean a performance loss during a deploy.

If the caches are possibly incompatible that's unavoidable anyway. The only way to prevent the performance hit is to prewarm the caches and ship them with the deployment. Symfony has full support for this (you can ship applications with a precompiled container and warmed filesystem cache) but it's pretty much moot point as for those application that do that AND depend on filesystem caches the Doctrine caches will by definition be ABSENT during deployment. They always take the performance hit anyway.

I think the root issue is just that the Doctrine Bundle should automatically invoke those 3 cache clear commands during Symfony built in cache clear. It will out of the box solve all portability issues in exchange for sometimes innecessarily rebuilding a cache, and the only way to prevent that is to implement a cache warmer which would be incredibly complex as it would need to be compatible with Redis, APCU, Memcached et al adapters.

or a cache namespace, then

So changing the namespace would cause the cache to effectively be empy, forcing a rebuild. How does this improve on just trashing it?

try using these tools instead of directly flushing redis / restarting apache. Does it fix the issue?

I ran into this on a live environment as query and result caches are only enabled in prod. I did the quick fix 😉

[PhilETaylor]

For me, understanding what the problem is, and now knowing that once I deploy after upgrading doctrine/orm I need to address the caching issue, makes this whole issue moot. It was the fact that this was not known, and not documented in the release notes that forced me to raise this as an issue while investigating it.

The issue only affects those people that have existing caches, and who upgrade without clearing those caches manually somehow. Like you said, caches are not part of the b\c promises.

Knowing this, I would say there is nothing to "resolve" here, other than having the knowledge that this update will break if there are existing caches.

Maybe we should just close this then as there is not really anything to fix?

[curry684]

Maybe we should just close this then as there is not really anything to fix?

The bug just moved. There is no issue whatsoever with the 2.15.0 release, the fact that the issue popped up now is indeed just an accidental symptom of a perfectly fine B/C change.

There's still a bug though that causes this, because as you say:

The issue only affects those people that have existing caches, and who upgrade without clearing those caches manually somehow.

I always run bin/console cache:clear between deployments. Manually or automatically. But DoctrineBundle is (apparently) not respecting that. That's the actual root cause bug that causes these deployment issues.

other than having the knowledge that this update will break if there are existing caches.

You do realize that this is the 99.9% use case of deployments: overwriting an existing older codebase with shiny new stuff? 😉

[greg0ire]

If there is indeed an issue, I agree that it is in the bundle. Transferring.

I ran into this on a live environment as query and result caches are only enabled in prod. I did the quick fix wink

I would have done the same, but if anybody can reproduce this locally, or even in prod, and fix it by running the doctrine commands to clear the cache, let us know.

[flack]

maybe I'm misremebering, but would that cache clearing from shell even work with apcu? I always thought apcu needs to be cleared from the same SAPI

[greg0ire]

I think you're correct about that.

[curry684]

@flack That's the cache pool adapter's responsibility, not Doctrine's. As long as Doctine sends out the 'trash my cache' request correctly the APCu adapter should take care of the rest.

[PhilETaylor]

I always run bin/console cache:clear between deployments. Manually or automatically. But DoctrineBundle is (apparently) not respecting that. That's the actual root cause bug that causes these deployment issues.

Unfortunately in a multi server large app in multiple docker containers, we rarely clear the whole cache - which is centrally stored in redis and shared by all containers. The local filesystem caches are warmed by building the container in the docker containers, but the main app caches are all stored in a centralised redis server. I dont think thats unusual in a larger distributed app.

deploying a single instance on one server in this set up, will not, and should not clear the the whole cache for the whole app.

[flack]

If there is indeed an issue, I agree that it is in the bundle. Transferring.

@greg0ire I don't have DoctrineBundle installed, still the problem occurred. Not sure if changing something in DoctrineBundle can fix it

[greg0ire]

@flack well maybe there are 2 issues:

• one with the bundle
• one with the ORM (or with the cache implem, as @curry684 mentioned?), in the particular case of APCu

[flack]

@greg0ire ok, but on a high level, only ORM knows when the cache needs to be cleared, no? It just seems a bit backwards to make DoctrinBundle (and by extension all alternative integration layers) keep track of when ORM's internal serialization format changes

[greg0ire]

I wasn't suggesting that. What I was suggesting is that the cache should not be considered reusable between deploys, at any time. There could be a strategy to bump the version number / prefix whenever you deploy, or whenever the composer.lock changes. At that point, I'm wondering if this should really be the responsibility of the bundle either.

Maybe a good next step would be to decide whether the cache should be considered reusable between deploys or not, document that, and then document what should be done by users when deploying (what commands to call, etc.).

[mbabker]

Maybe a good next step would be to decide whether the cache should be considered reusable between deploys or not, document that, and then document what should be done by users when deploying (what commands to call, etc.).

The Flex recipe configures the result cache to persist across deployments and the query cache to be build-specific by default for the production environment. To me that seems like a sane set of defaults; I'd expect the result cache to be persistent across deployments but the other caches (metadata and query) seem coupled to the code so those should be build-specific.

Maybe the ORM or bundle docs should suggest not using a persistent cache for metadata and query caches (if they don't already, haven't been in that part of the documentation in a hot minute), or if one is used, to use some kind of prefix or namespace or something unique for each deployment.

[stof]

Well, the system caches are cleared automatically on cache:clear, so metadata and query caches should be cleared at each deployment already (unless doctrine.system_cache_pool does not actually map to the system cache of FrameworkBundle, whcih would be a bad naming convention).

[curry684]

I don't have DoctrineBundle installed

In that case your own code configured the caches somewhere and therefore assumes responsibility for clearing it.

[flack]

@curry684 actually, I'm using ORMSetup, so caches get automatically configured by ORM depending on what's installed on the host machine

[curry684]

Doesn't change anything - if ORMSetup configures the caches it assumes responsibility for how and when to clear them 😄 In Symfony's case it's the DoctrineBundle that assumes that task, hence why it should hook into the Symfony kernel cache clear mechanisms.

In either case, Doctrine ORM cannot magically run code itself without some outside trigger.

[flack]

Yeah, exactly. And ORMSetup is part of doctrine/orm, so it needs to be dealt with there:

https://github.com/doctrine/orm/blob/60c625be17668f905cd2c1f73b48ab9b7a105755/lib/Doctrine/ORM/ORMSetup.php#L179-L217

[derrabus]

Please see doctrine/orm#10684 for a possible fix.

[nCrazed]

Still seeing this exact error with doctrine/doctrine-bundle: 2.7.2 and doctrine/orm: 2.15.4 regardless of how fresh/stale the cache is.

Downgrading to doctrine/orm: 2.14.* fixes the error