Skip to content
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

Null in fields value in Cached Entity several times on day on high-load project. #7735

Closed
AlexSmerw opened this issue Jun 5, 2019 · 4 comments
Assignees
Labels
Milestone

Comments

@AlexSmerw
Copy link

Bug Report

Q A
BC Break yes
Version 2.6.3

Summary

We have a problem with the cache on a high-load project. Several times a day, the entity is retrieved from the cache with the field value = null, and these fields cannot be null

/ **
    * @var curator
    *
    * @ManyToOne (targetEntity = "SD \ Domain \ PersistModel \ Curator \ Curator")
    * @JoinColumn (name = "curator_id", referencedColumnName = "id")
    * /
   private $ curator ;

Current behavior

This situation arises from related entities. When the parent entity gets out of the cache, and the related is already expired.
See DefaultEntityHydrator function loadCacheEntry
Then
BasicEntityPersister function load will return Entity with null fields value.
Because AbstractEntityPersister
$entity = $this->persister->loadById($identifier, $entity);
It put $entity = null to function loadById($identifier, $entity) Instead Entity.

How to reproduce

Set $assocEntry = null on DefaultEntityHydrator line180 for child entity

Expected behavior

I think this can be corrected by editing the code in function loadById(array $identifier, $entity = null)
loadById

Replace $ entity with $ cachedEntity
https://github.com/doctrine/orm/blob/master/lib/Doctrine/ORM/Cache/Persister/Entity/AbstractEntityPersister.php#L458

$cachedEntity = $this->persister->loadById($identifier, $entity);
        if ($cachedEntity === null) {
            return null;
         }
@Ocramius
Copy link
Member

Ocramius commented Jun 5, 2019

We'd need a test case for this: as the issue is presented right now, this isn't really usable by maintainers.

@AlexSmerw
Copy link
Author

AlexSmerw commented Jun 19, 2019

Look. For example. We have 3 classes :
Car
Engine
Power
Link on code

Init data

$engine = new Car\Engine('turbo');
$power = new Car\Power(100, 110, $engine);
$car = new Car\Car('white', $engine);

$entityManger->persist($car);
$entityManger->persist($power);
$entityManger->persist($engine);

$entityManger->flush();

When entities have already set to cache, we try to get data from cache


$carRepository = $entityManger->getRepository(Car\Car::class);
$car = $carRepository->find(1);
$engine = $car->getEngine();

$engine short dump
isInitialized = false
model = null

$power = $engine->getPower(); // In this moment $assocRegion->get($assocKey) in Doctrine/ORM/Cache/DefaultEntityHydrator.php it can return null, because key 'power_hash' can be expired in hi load project.
It very rare situation, but we have this problem every days.

$engine short dump
isInitialized = true
model = null

$model = $engine->getModel(); - Exception 'Car\Engine::getModel() must be of th
e type string, null returned'

For problem reproduction when started code $power = $engine->getPower(); set debug breakpoint there $assocRegion->get($assocKey) in Doctrine/ORM/Cache/DefaultEntityHydrator.php. And wait while cache time is over. Then go ahead. As for me, I set regionLifeTime = 20 seconds.

@Ocramius
Copy link
Member

Thanks for the more detailed explanation, but we'd still need an (automated) integration test that verifies the issue. See https://github.com/doctrine/orm/tree/master/tests/Doctrine/Tests/ORM/Functional for examples.

lcobucci pushed a commit to AlexSmerw/orm that referenced this issue Aug 11, 2019
The same variable name is used below, and that causes a bug etc.
Fixes doctrine#7735
lcobucci pushed a commit to AlexSmerw/orm that referenced this issue Aug 11, 2019
The same variable name is used below, and that causes a bug etc.
Fixes doctrine#7735
lcobucci pushed a commit to AlexSmerw/orm that referenced this issue Aug 11, 2019
The same variable name is used below, and that causes a bug etc.
Fixes doctrine#7735
@lcobucci lcobucci added this to the 2.6.4 milestone Aug 11, 2019
lcobucci pushed a commit to AlexSmerw/orm that referenced this issue Aug 11, 2019
The same variable name is used below, and that causes a bug etc.
Fixes doctrine#7735
lcobucci pushed a commit to AlexSmerw/orm that referenced this issue Aug 11, 2019
The same variable name is used below, and that causes a bug etc.
Fixes doctrine#7735
lcobucci pushed a commit to AlexSmerw/orm that referenced this issue Aug 11, 2019
The same variable name is used below, and that causes a bug etc.
Fixes doctrine#7735
@lcobucci
Copy link
Member

Handled by #7750

Voziv added a commit to ratehub/doctrine2 that referenced this issue Oct 22, 2019
v2.6.4

[![Build Status](https://travis-ci.org/doctrine/orm.svg?branch=v2.6.4)](https://travis-ci.org/doctrine/orm)

In this release we've fixes many bugs (including a performance regression) and
made the v2.x series compatible with PHP 7.4.

--------------------------------------------

- Total issues resolved: **11**
- Total pull requests resolved: **32**
- Total contributors: **30**

Improvement
-----------

 - [7785: Fix "access array offset on value of type null" PHP 7.4 notices](doctrine#7785) thanks to @mlocati
 - [7142: Rename this repository to doctrine/orm](doctrine#7142) thanks to @greg0ire

Bug
------------------

 - [7821: Bug: doctrine#7820 paginator ignores dbal type conversions in identifiers](doctrine#7821) thanks to @Ocramius
 - [7778: Guard L2C regions against corrupted data](doctrine#7778) thanks to @umpirsky
 - [7767: PersistentCollection::matching() does not respect the collections native sorting](doctrine#7767) thanks to @stephanschuler
 - [7766: Respect collection orderBy meta when matching()](doctrine#7766) thanks to @stephanschuler
 - [7761: Do not modify UOW on PersistentCollection::clear() when owner has DEFFERED&doctrine#95;EXPLICIT change tracking policy](doctrine#7761) thanks to @paxal
 - [7750: Fix incorrect return of null values in L2C](doctrine#7750) thanks to @AlexSmerw
 - [7737: Fix MEMBER&doctrine#95;OF comparison when using criteria in query builder](doctrine#7737) thanks to @Smartel1
 - [7735: Null in fields value in Cached Entity several times on day on high-load project.](doctrine#7735) thanks to @AlexSmerw
 - [7630: Fix doctrine#7629 - `scheduledForSynchronization` leaks memory when using `@ORM\ChangeTrackingPolicy("DEFERRED&doctrine#95;EXPLICIT")`](doctrine#7630) thanks to @yethee
 - [7528: Prevent `UnitOfWork` lookup for DBAL types specified in `Doctrine\ORM\Query#setParameter()`](doctrine#7528) thanks to @Ocramius
 - [7322: JoinedSubclassPersister pass identifier types on delete](doctrine#7322) thanks to @dennisenderink and @fred-jan
 - [7266: Call to a member function resolveAssociationEntries() on boolean {"detail":"&doctrine#91;object&doctrine#93; (Symfony\\Component\\Debug\\Exception\\FatalThrowableError(code: 0): Call to a member function resolveAssociationEntries() on boolean at /www/vendor/doctrine/orm/lib/Doctrine/ORM/Cache/DefaultQueryCache.php:140)"}](doctrine#7266) thanks to @mingmingxianseng
 - [4632: DDC-3789: Paginator does not convert entity ids if they are value objects](doctrine#4632) thanks to @doctrinebot

Documentation
-------------

 - [7818: Add note into docs about not using SimpleAnnotationReader](doctrine#7818) thanks to @SenseException
 - [7791: Fix preFlush event documentation stating incorrectly that flush can be called safely](doctrine#7791) thanks to @Steveb-p
 - [7753: Add ORM annotations in getting-started docs](doctrine#7753) thanks to @SenseException and @wajdijurry
 - [7744: Fixed a typo-error](doctrine#7744) thanks to @noobshow
 - [7732: &doctrine#91;Documentation&doctrine#93; Missing comma fix](doctrine#7732) thanks to @lchrusciel
 - [7729: Update DATE&doctrine#95;ADD and DATE&doctrine#95;SUB docs](doctrine#7729) thanks to @JoppeDC
 - [7672: Added cross-links to relevant documentation](doctrine#7672) thanks to @jschaedl
 - [7612: Update ordered-associations.rst](doctrine#7612) thanks to @spirlici
 - [7610: Change APC to OPcache in improving-performance.rst ](doctrine#7610) thanks to @smtchahal
 - [7596: Correct method names and broken link in docs](doctrine#7596) thanks to @mbessolov
 - [7577: Fix of single link to dbal docs in advanced-configuration.rst](doctrine#7577) thanks to @SenseException
 - [7572: Remove codeigniter Framework example](doctrine#7572) thanks to @SenseException
 - [7571: Fix typo in inheritance mappings docs](doctrine#7571) thanks to @batwolf
 - [7557: Change Stackoverflow tag to doctrine-orm](doctrine#7557) thanks to @malarzm
 - [7551: &doctrine#91;2.6&doctrine#93; Migrate repository name doctrine/doctrine2 -> doctrine/orm](doctrine#7551) thanks to @Majkl578
 - [7530: Documentation error typo fix: s/Used-defined/User-Defined](doctrine#7530) thanks to @vladyslavstartsev
 - [7519: doctrine#7518 Fixed type mismatch between `EntityRepository#&doctrine#95;&doctrine#95;construct()` and its documented constructor arguments](doctrine#7519) thanks to @koftikes
 - [7518: `EntityRepository::&doctrine#95;&doctrine#95;construct()` expects `Doctrine\ORM\EntityManager` instead of actual required `EntityManagerInterface`](doctrine#7518) thanks to @koftikes
 - [7490: Fix broken link](doctrine#7490) thanks to @vladyslavstartsev
 - [7483: Fixed a minor syntax issue](doctrine#7483) thanks to @javiereguiluz

CI
-----------------

 - [7794: Fix test compatibility with DBAL 2.10.x-dev](doctrine#7794) thanks to @lcobucci
 - [7731: Replace custom install script with add-on](doctrine#7731) thanks to @greg0ire
 - [7473: Incremental CS checks in 2.x branches](doctrine#7473) thanks to @Majkl578
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants