Moodle
  1. Moodle
  2. MDL-25837

disasterous caching bug in course_overview block

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.1
    • Fix Version/s: 2.0.2
    • Component/s: Performance
    • Labels:
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE
    • Rank:
      15706

      Description

      The course_overview block has a major performance flaw which is causing it to call rebuild_course_cache() for each and every course it displays. For a normal student with 4 courses on their page, this resulted in over 3000 database queries! This does not just happen once - it happens every time My Moodle page loads for any user.

      I have attached a patch with a very simple fix. The course_overview block was not passing the modinfo attribute with the course objects when it passes them to print_overview(). This causes print_overview() to think the course has no modinfo cache and thus it goes off an rebuilds it all.

      In my test case, the number of DB queries went from 3474 down to 90.

        Activity

        Hide
        Eloy Lafuente (stronk7) added a comment -

        Moving this up in the stable backlog...

        Show
        Eloy Lafuente (stronk7) added a comment - Moving this up in the stable backlog...
        Hide
        Petr Škoda added a comment -

        Hmm, I can not reproduce this problem - no standard activity is using modinfo to print overview.

        I am adding a debug warning to modinfo processing so that it clearly explains why this happens next time.
        I am also adding your patch because it can be used in 3rd party modules and maybe even official ones in the future.

        Show
        Petr Škoda added a comment - Hmm, I can not reproduce this problem - no standard activity is using modinfo to print overview. I am adding a debug warning to modinfo processing so that it clearly explains why this happens next time. I am also adding your patch because it can be used in 3rd party modules and maybe even official ones in the future.
        Hide
        Petr Škoda added a comment -

        grrrrr

        Show
        Petr Škoda added a comment - grrrrr
        Hide
        Petr Škoda added a comment -

        sorry, I can reproduce it now - I was editing files in different checkout - I am going to use a bit different code, I think it is better to add the modinfo as mandatory field directly into enrol_get_my_courses().

        Show
        Petr Škoda added a comment - sorry, I can reproduce it now - I was editing files in different checkout - I am going to use a bit different code, I think it is better to add the modinfo as mandatory field directly into enrol_get_my_courses().
        Hide
        Petr Škoda added a comment -

        hmm, in the end I have used your patch, if more places are found we may add the modinfo field as mandator\.

        thanks a lot for the report and patch!

        Show
        Petr Škoda added a comment - hmm, in the end I have used your patch, if more places are found we may add the modinfo field as mandator\. thanks a lot for the report and patch!
        Hide
        Ashley Holman added a comment -

        I'm pretty sure this is caused by the standard quiz module:

        1. line 3042 of /lib/moodlelib.php: call to rebuild_course_cache()
        2. line 1481 of /lib/datalib.php: call to get_fast_modinfo()
        3. line 1488 of /mod/quiz/lib.php: call to get_all_instances_in_courses()
        4. line 856 of /course/lib.php: call to quiz_print_overview()
        5. line 107 of /blocks/course_overview/block_course_overview.php: call to print_overview()
        6. line 279 of /blocks/moodleblock.class.php: call to block_course_overview->get_content()
        7. line 232 of /blocks/moodleblock.class.php: call to block_base->formatted_contents()
        8. line 882 of /lib/blocklib.php: call to block_base->get_content_for_output()
        9. line 934 of /lib/blocklib.php: call to block_manager->create_block_contents()
        10. line 304 of /lib/blocklib.php: call to block_manager->ensure_content_created()
        11. line 899 of /lib/outputrenderers.php: call to block_manager->get_content_for_region()
        12. line 150 of /my/index.php: call to core_renderer->blocks_for_region()
        Show
        Ashley Holman added a comment - I'm pretty sure this is caused by the standard quiz module: line 3042 of /lib/moodlelib.php: call to rebuild_course_cache() line 1481 of /lib/datalib.php: call to get_fast_modinfo() line 1488 of /mod/quiz/lib.php: call to get_all_instances_in_courses() line 856 of /course/lib.php: call to quiz_print_overview() line 107 of /blocks/course_overview/block_course_overview.php: call to print_overview() line 279 of /blocks/moodleblock.class.php: call to block_course_overview->get_content() line 232 of /blocks/moodleblock.class.php: call to block_base->formatted_contents() line 882 of /lib/blocklib.php: call to block_base->get_content_for_output() line 934 of /lib/blocklib.php: call to block_manager->create_block_contents() line 304 of /lib/blocklib.php: call to block_manager->ensure_content_created() line 899 of /lib/outputrenderers.php: call to block_manager->get_content_for_region() line 150 of /my/index.php: call to core_renderer->blocks_for_region()
        Hide
        David Mudrak added a comment - - edited

        Tested. In my simple test case with just two almost empty courses, the difference was 86 vs 108 queries. The debugging message is displayed when the modinfo field is not fetched by enrol_get_my_courses().

        Good catch Ashley! Thanks!

        Show
        David Mudrak added a comment - - edited Tested. In my simple test case with just two almost empty courses, the difference was 86 vs 108 queries. The debugging message is displayed when the modinfo field is not fetched by enrol_get_my_courses(). Good catch Ashley! Thanks!

          People

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

            Dates

            • Created:
              Updated:
              Resolved: