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

DDC-2248: Expire result cache functionality not implemented #2947

Closed
doctrinebot opened this issue Jan 19, 2013 · 13 comments
Closed

DDC-2248: Expire result cache functionality not implemented #2947

doctrinebot opened this issue Jan 19, 2013 · 13 comments
Assignees
Labels
Projects
Milestone

Comments

@doctrinebot
Copy link

Jira issue originally created by user nazin:

According to documentation expireResultCache, should force cache to update but it's not working... Why? Because functionality is not implemented. You can set _expireResultCache variable, but there is no place where this variable is being checked.

@doctrinebot
Copy link
Author

Comment created by @Ocramius:

A cache profile can be set and cleaned. I suppose that expireResultCache is an old piece of code that survived the refactoring. Should just be removed and documented accordingly

@doctrinebot
Copy link
Author

Comment created by mdwyer415:

My understanding was that expireResultCache would ignore the cache, execute the query, and then save the result to cache. I'm not sure how accomplish that same thing by manipulating the cache profile, unless I created a result cache driver that always returned false on fetch, and used it for these occasions.

Am I missing something or is that a use case that shouldn't happen?

Thanks,
Mike

@bruno-ds
Copy link

bruno-ds commented Apr 7, 2017

Hello,

I was reading the source code, and I've got the feeling that this problem is not yet solved.

This is a documented functionality :

edit : do not confond it with expireQueryCache who is well implemented, it is used here :

// Doctrine\ORM\Query
/**
     * Parses the DQL query, if necessary, and stores the parser result.
     *
     * Note: Populates $this->_parserResult as a side-effect.
     *
     * @return \Doctrine\ORM\Query\ParserResult
     */
    private function _parse()
    {
//...
        $cached = $this->_expireQueryCache ? false : $queryCache->fetch($hash);
//...
    }

it seems that the implementation could be as simple as :

    /**
     * Load from second level cache or executes the query and put into cache.
     *
     * @param ArrayCollection|array|null $parameters
     * @param integer|null               $hydrationMode
     *
     * @return mixed
     */
    private function executeUsingQueryCache($parameters = null, $hydrationMode = null)
    {
    //...
if ($this->getExpireResultCache()) {
        $result     = null;
} else {
        $result     = $queryCache->get($queryKey, $rsm, $this->_hints);
}
    //...
    }

note I replace the $result = $queryCache->get($queryKey, $rsm, $this->_hints); by the if

@lcobucci
Copy link
Member

lcobucci commented Apr 7, 2017

@bruno-ds can you please send a test that confirms that this is not really working?

@bruno-ds
Copy link

bruno-ds commented Apr 7, 2017

Hello,

I'm rather new to symfony, so I don't know how to write the requested test.

But looking at the code, it is obvious : as said in the original issue, getExpireResultCache (and the protected $_expireResultCache) are not used in the code except in the getter/setter.

@theofidry
Copy link

@bruno-ds lucky you it's not Symfony but Doctrine :P Jokes aside, maybe look at the concerned class and look for a test case using it. You may find a test case related to it where you may find some lead to implement the requested test :)

@bruno-ds
Copy link

I just forked, cloned and search in the tests... but I'm lost... I think that the test should be Doctrine\Tests\ORM\Cache\*. But I don't even understand those tests, and similar function are not tested (ie : setResultCacheLifetime, setResultCacheId, ...)

@lcobucci
Copy link
Member

@bruno-ds those tests are related to L2-cache (second level cache). You can create a funcional test (based on https://github.com/doctrine/doctrine2/blob/2c5e76c96187b512b3b51c14b68ec64c8c2dd857/tests/Doctrine/Tests/ORM/Functional/Ticket/GH5762Test.php for example).

@bruno-ds
Copy link

thank you, I tried to create a test, but I don't know how to run it, so I created a PR : #6390

@lcobucci
Copy link
Member

@bruno-ds thanks for the PR I'll review it soon and find a way to have it properly fixed.

Just one thing, based on the PR I'd recommend you to check L2-cache since it handles cache invalidation automatically http://docs.doctrine-project.org/projects/doctrine-orm/en/latest/reference/second-level-cache.html

@lcobucci lcobucci added the Bug label Apr 11, 2017
@lcobucci lcobucci added this to the 2.6.0 milestone Apr 11, 2017
@lcobucci lcobucci assigned lcobucci and unassigned beberlei Apr 11, 2017
@bruno-ds
Copy link

thank you,

just for information, my use case is : I have to count the unread notifications for each user, I compute this only by advance & store it in a lifetime 0 cache.
since expireResultCache(true) do not work, when this counter has to change, I ended up creating myself the id, it permits me to delete it in redis, but the code is obviously more complex.

Since the result I cache is a count, I don't think that the second level cache would help.

@lcobucci
Copy link
Member

You're right, L2C won't help you with counts but theoretically the timestamp region of an entity might help to know when you need to invalidate that result set

@lcobucci lcobucci added this to Planning & Design in ORM v2.6.x Apr 13, 2017
@lcobucci lcobucci moved this from Planning & Design to In Progress in ORM v2.6.x Apr 30, 2017
@lcobucci lcobucci moved this from In Progress to Review in ORM v2.6.x Apr 30, 2017
@Ocramius
Copy link
Member

Ocramius commented May 3, 2017

Implemented in #6417 - will land in 2.6

@Ocramius Ocramius closed this as completed May 3, 2017
@lcobucci lcobucci moved this from Review to Done in ORM v2.6.x May 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

No branches or pull requests

6 participants