Uploaded image for project: '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

          Attachments

            Issue Links

              Activity

              Hide
              poltawski Dan Poltawski added a comment -

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

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

              looks good to me too, need testing instructions

              Show
              marina Marina Glancy added a comment - looks good to me too, need testing instructions
              Hide
              tlevi 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
              tlevi 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
              poltawski 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
              poltawski 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 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 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 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 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 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 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
              tlevi 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
              tlevi 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 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 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 Marina Glancy added a comment -

              Integrated to 2.3, 2.4, 2.5, master

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

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

              Show
              dmonllao 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 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 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_frenz Ankit Agarwal added a comment -

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

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

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

              Thanks for your contribution!

              Show
              damyon 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:
                    Fix Release Date:
                    9/Sep/13