Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-38110

Using MUC in code breaks ttl unit test

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.4.1, 2.5
    • Fix Version/s: 2.4.2, 2.5
    • Component/s: Caching
    • Labels:

      Description

      Sam, it seems like any my attempt to use MUC in code leads to breaking the ttl unit test. I will link issues being blocked by this one

      cache_phpunit_tests::test_definition_ttl
      Failed asserting that true is false.
       
      /home/marina/repositories/courserenderers/moodle/cache/tests/cache_test.php:437
      /home/marina/repositories/courserenderers/moodle/lib/phpunit/classes/advanced_testcase.php:76
       
      To re-run:
       /home/marina/pear/bin/phpunit cache_phpunit_tests cache/tests/cache_test.php
      

      If I execute just "To re-run" command, everything is ok. The error occurs only if all unittests are executed

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            samhemelryk Sam Hemelryk added a comment -

            Looking at it now thanks Marina.

            Just to confirm I am able to replicate this.

            Show
            samhemelryk Sam Hemelryk added a comment - Looking at it now thanks Marina. Just to confirm I am able to replicate this.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            I think I've identified the problem now:
            The test in question is setting up a cache with a negative time stamp of 10 seconds. It then purges the cache to be extra sure nothing is in it before running the actual tests.
            The purging of that cache is taking more than 10 seconds in the situation in which all tests are being run.
            The solution is simply to increase the negative ttl. Using something like -86400 (a day) is going to be a sure fire option. Given this is testing functionality and not performance that is probably the best solution, however there is certainly a performance issue in there as well.

            Show
            samhemelryk Sam Hemelryk added a comment - I think I've identified the problem now: The test in question is setting up a cache with a negative time stamp of 10 seconds. It then purges the cache to be extra sure nothing is in it before running the actual tests. The purging of that cache is taking more than 10 seconds in the situation in which all tests are being run. The solution is simply to increase the negative ttl. Using something like -86400 (a day) is going to be a sure fire option. Given this is testing functionality and not performance that is probably the best solution, however there is certainly a performance issue in there as well.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Marina could you please try the following patch and see if that solves the problem for you when working with your branches?

            Repo: git://github.com/samhemelryk/moodle.git
            Branch: wip-MDL-38110-m25
            Diff: http://github.com/samhemelryk/moodle/compare/master...wip-MDL-38110-m25

            Hopefully that is as simple as it is.

            Show
            samhemelryk Sam Hemelryk added a comment - Marina could you please try the following patch and see if that solves the problem for you when working with your branches? Repo: git://github.com/samhemelryk/moodle.git Branch: wip- MDL-38110 -m25 Diff: http://github.com/samhemelryk/moodle/compare/master...wip-MDL-38110-m25 Hopefully that is as simple as it is.
            Hide
            marina Marina Glancy added a comment -

            Hi Sam, I tested it with one of my issues and all tests passed all right. But the solution seems a bit strange for me.

            Can we have another ttl test with positive value - where you set ttl for, say, 2s and then sleep for 3 seconds and check the cache?

            Show
            marina Marina Glancy added a comment - Hi Sam, I tested it with one of my issues and all tests passed all right. But the solution seems a bit strange for me. Can we have another ttl test with positive value - where you set ttl for, say, 2s and then sleep for 3 seconds and check the cache?
            Hide
            samhemelryk Sam Hemelryk added a comment -

            My apologies, I was sure I had commented here and pushed this for integration.

            In summary we can't use a positive TTL as the cache API uses a timestamp taken during initialisation to avoid timing problems.
            This effectively means anything set during a request is available for the lifetime of the request. Doing this aids us in avoiding timing related issues, and will hopefully mitigate the risk of cache regeneration and syncronisation issues during long running requests (think cron etc).

            The negative ttl is works because the it sets the ttl back before that timestamp.

            Many thanks
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - My apologies, I was sure I had commented here and pushed this for integration. In summary we can't use a positive TTL as the cache API uses a timestamp taken during initialisation to avoid timing problems. This effectively means anything set during a request is available for the lifetime of the request. Doing this aids us in avoiding timing related issues, and will hopefully mitigate the risk of cache regeneration and syncronisation issues during long running requests (think cron etc). The negative ttl is works because the it sets the ttl back before that timestamp. Many thanks Sam
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Integrated (24 & master), thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Integrated (24 & master), thanks!
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            I'm passing this, tests worked ok.

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - I'm passing this, tests worked ok.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Because

            A
            MARVELOUS
            A       U
            Z  YOU  P
            I  ARE  E
            N  PPL  R
            G       B
              TNKS! 
            

            Closing, ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Because A MARVELOUS A U Z YOU P I ARE E N PPL R G B TNKS! Closing, ciao

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  11/Mar/13