Moodle
  1. Moodle
  2. MDL-34783

course_overview block doesn't retrieve all required fields for get_fast_modinfo

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.3, 2.4
    • Fix Version/s: 2.3.2
    • Component/s: Blocks, Course
    • Labels:
    • Testing Instructions:
      Hide

      Preparation:

      • to be enrolled on a couple of courses
      • for those courses to have some activities including some of the following
        • chat
        • assign (2.3 version)
        • forum
        • lesson
        • quiz
        • scorm

      Testing

      I'd advise looking at the db read/write count both before and after applying this patch

      • Open /my
      • look at the db read/write count
      • ensure that all expected overview items are seen both before and after applying
      Show
      Preparation: to be enrolled on a couple of courses for those courses to have some activities including some of the following chat assign (2.3 version) forum lesson quiz scorm Testing I'd advise looking at the db read/write count both before and after applying this patch Open /my look at the db read/write count ensure that all expected overview items are seen both before and after applying
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-34783-master-1
    • Rank:
      43274

      Description

      We've been seeing some massive speed issues on the /my page and they largely seem to be a result of get_fast_modinfo repeatedly resetting.
      Having tracked this issue back:

      • blocks/course_overview/block_course_overview.php::get_content()
        • calls enrol_get_my_courses() to retrieve a list of courses
        • passes this list to print_overview($courses, $remote_courses)
      • course/lib.php::print_overview()
        • loops through each module, and calls its {$mod->name})print_overview() function
      • {$mod->name})print_overview() calls get_all_instances_in_courses()
      • lib/datalib.php::get_all_instances_in_courses() calls get_fast_modinfo()
      • lib/modinfolib.php::get_fast_modinfo()
        • checks whether the cache is filled and a relevant entry is found for the specified course
        • if found returns
        • if not inserts a new course_modinfo object into the cache
      • lib/modinfolib.php::course_modinfo
        • checks whether the supplied course has a valid modinfo and sectioncache - if not, it calls rebuild_course_cache which resets the get_fast_modinfo cache

      Unfortunately though, the course_overview block doesn't retrieve the sectioncache field for each course from the DB, and as a result, every time a course is retrieved, the entire get_fast_modinfo cache is emptied.

        Issue Links

          Activity

          Hide
          Andrew Nicols added a comment -

          Raising this to critical as it makes the /my page almost painfully slow.

          We're seeing in excess of 3000 db reads for standard users accessing the /my page if they're on several courses with several modules defining a _print_overview() function.

          The intended fix brings this down to ~100

          Show
          Andrew Nicols added a comment - Raising this to critical as it makes the /my page almost painfully slow. We're seeing in excess of 3000 db reads for standard users accessing the /my page if they're on several courses with several modules defining a _print_overview() function. The intended fix brings this down to ~100
          Hide
          Dan Poltawski added a comment -

          +1, pushing for integration. We should git grep for 'modinfo' uses to see if there are any other places missed like this.

          Show
          Dan Poltawski added a comment - +1, pushing for integration. We should git grep for 'modinfo' uses to see if there are any other places missed like this.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated (23 & master), thanks!

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

          Tested pre and post patch.
          DB read / writes pre: 249/15
          DB read / writes post: 90/0

          I would say a remarkable improvement.
          Test passed.

          Show
          Adrian Greeve added a comment - Tested pre and post patch. DB read / writes pre: 249/15 DB read / writes post: 90/0 I would say a remarkable improvement. Test passed.
          Hide
          Gregor McNish added a comment -

          Tested pre and post (large site)
          DB reads/writes pre: 15222/215 took 30-50sec!
          DB reads/writes post: 239/4

          Logged in as admin, 18 courses in the list.
          Thanks for this, it would have blocked migration to 2.3

          Show
          Gregor McNish added a comment - Tested pre and post (large site) DB reads/writes pre: 15222/215 took 30-50sec! DB reads/writes post: 239/4 Logged in as admin, 18 courses in the list. Thanks for this, it would have blocked migration to 2.3
          Hide
          Eloy Lafuente (stronk7) added a comment -

          For the good and the bad... this is now part of Moodle and people around the world will start using it immediately, what a responsibility!

          Many thanks for your collaboration, yay!

          Closing, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - For the good and the bad... this is now part of Moodle and people around the world will start using it immediately, what a responsibility! Many thanks for your collaboration, yay! Closing, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: