Moodle
  1. Moodle
  2. MDL-40035

rebuild_course_cache runs over and over for empty course

    Details

    • Testing Instructions:
      Hide
      1. create empty site
      2. enable the DB query logging
      3. refresh the frontpage twice, second time make sure there is no writing to course.modinfo and course.sectioncache fields
      4. add a module on frontpage, make sure it is displayed immediately
      Show
      create empty site enable the DB query logging refresh the frontpage twice, second time make sure there is no writing to course.modinfo and course.sectioncache fields add a module on frontpage, make sure it is displayed immediately
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull from Repository:

      Description

      I've just done a fresh vanilla 2.5 install, and every page load is calling rebuild_course_cache for course 1.

      This appears to be a bug from sectioncache, where it can be empty and this triggers the rebuild from course_modinfo::__construct

      I think the simple answer is to remove the empty() check there.

      May relate to MDL-38448.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Dan Poltawski added a comment -

            Patch looks good to me, thanks Tony - ping Marina Glancy

            Show
            Dan Poltawski added a comment - Patch looks good to me, thanks Tony - ping Marina Glancy
            Hide
            Marina Glancy added a comment -

            looks good to me too, need testing instructions

            Show
            Marina Glancy added a comment - looks good to me too, need testing instructions
            Hide
            Tony Levi added a comment -

            Does it suffice to enable perfinfo in fresh install and verify db writes on login/index.php goes from 1 to 0 ? Otherwise it is kinda invisible that this is going on.

            Show
            Tony Levi added a comment - Does it suffice to enable perfinfo in fresh install and verify db writes on login/index.php goes from 1 to 0 ? Otherwise it is kinda invisible that this is going on.
            Hide
            Dan Poltawski added a comment -

            We just need something that runs the code that demonstrate there is not a regression.

            Trying to think of something, I hoped there would be unit tests for it, but there are not, so I think that now might be the time to add them.

            Show
            Dan Poltawski added a comment - We just need something that runs the code that demonstrate there is not a regression. Trying to think of something, I hoped there would be unit tests for it, but there are not, so I think that now might be the time to add them.
            Hide
            Marina Glancy added a comment -

            yes just a test to check for regressions - create empty site and then add some module on frontpage, make sure it is displayed immediately

            Show
            Marina Glancy added a comment - yes just a test to check for regressions - create empty site and then add some module on frontpage, make sure it is displayed immediately
            Hide
            Damyon Wiese added a comment -

            Thanks Tony - but these test instructions fail for me.

            I still get "UPDATE mdl_integration_2course SET modinfo = $1,sectioncache = $2 WHERE id=$3 [array ( 'modinfo' => 'a:0:{}', 'sectioncache' => 'a:0:{}', 0 => '1', )]" on the front page with this patch installed.

            Show
            Damyon Wiese added a comment - Thanks Tony - but these test instructions fail for me. I still get "UPDATE mdl_integration_2course SET modinfo = $1,sectioncache = $2 WHERE id=$3 [array ( 'modinfo' => 'a:0:{}', 'sectioncache' => 'a:0:{}', 0 => '1', )] " on the front page with this patch installed.
            Hide
            Marina Glancy added a comment -

            Damyon, this query should appear only once and after that on frontpage refresh it should not repeat itself. Are you saying that it is repeating every time the frontpage is requested?

            Show
            Marina Glancy added a comment - Damyon, this query should appear only once and after that on frontpage refresh it should not repeat itself. Are you saying that it is repeating every time the frontpage is requested?
            Hide
            Tony Levi added a comment -

            I've just retested and confirmed this patch solves the problem; I've tweaked the test instructions to include a second refresh which should not be running the update...

            Show
            Tony Levi added a comment - I've just retested and confirmed this patch solves the problem; I've tweaked the test instructions to include a second refresh which should not be running the update...
            Hide
            Marina Glancy added a comment - - edited

            I'm taking over the integration as Damyon asked.

            TO TESTERS: I tested on 2.4, so you can skip it. Please test on 2.3, 2.5 and master.

            Show
            Marina Glancy added a comment - - edited I'm taking over the integration as Damyon asked. TO TESTERS: I tested on 2.4, so you can skip it. Please test on 2.3, 2.5 and master.
            Hide
            Marina Glancy added a comment -

            Integrated to 2.3, 2.4, 2.5, master

            Show
            Marina Glancy added a comment - Integrated to 2.3, 2.4, 2.5, master
            Hide
            David Monllaó added a comment -

            Note that there is no assignee, I can guess is Tony but you will now better than me

            Show
            David Monllaó added a comment - Note that there is no assignee, I can guess is Tony but you will now better than me
            Hide
            Marina Glancy added a comment -

            Yes it's Toni, I could not put him as an assignee because he's not in the developers group. I can put myself but then I'll be both assignee and integrator - not good

            Show
            Marina Glancy added a comment - Yes it's Toni, I could not put him as an assignee because he's not in the developers group. I can put myself but then I'll be both assignee and integrator - not good
            Hide
            Ankit Agarwal added a comment -

            works as described. Second call didn't trigger a query.
            Thanks

            Show
            Ankit Agarwal added a comment - works as described. Second call didn't trigger a query. Thanks
            Hide
            Damyon Wiese added a comment -

            a single bug fix
            a drop in a waterfall
            hear the mighty roar

            Thanks for your contribution!

            Show
            Damyon Wiese added a comment - a single bug fix a drop in a waterfall hear the mighty roar Thanks for your contribution!

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: