Moodle
  1. Moodle
  2. MDL-32659

Unusually costly mdl_course query on every page load under Moodle 2.2 using PostgreSQL

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2
    • Fix Version/s: 2.3
    • Component/s: Navigation
    • Testing Instructions:
      Hide

      1/ create more than 20 courses
      2/ login as admin and verify the course are properly loaded in navigation
      3/ enrol student to a few course
      4/ login as the student and verify navigation loads courses properly

      Show
      1/ create more than 20 courses 2/ login as admin and verify the course are properly loaded in navigation 3/ enrol student to a few course 4/ login as the student and verify navigation loads courses properly
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      w20_MDL-32659_m23_navcourses
    • Rank:
      39599

      Description

      See the discussion at http://moodle.org/mod/forum/discuss.php?d=201214#p878277 for more context. Here are some stats for the moodle database I'm working with:

      mdl_course records: approx. 4000
      mdl_context records: approx. 300,000
      mdl_course_categories: approx. 200

      All but a couple of queries on a basic page load (http://moodle_site/login/index.php, for example) are of the sub-millisecond variety. The following query, however, takes about 200ms every time (even worse when things aren't in the database and/or os cache):

      moodle=# EXPLAIN ANALYZE SELECT c.id, c.sortorder, c.visible, c.fullname, c.shortname, c.category, cat.path AS categorypath , ctx.id AS ctxid, ctx.path AS ctxpath, ctx.depth AS ctxdepth, ctx.contextlevel AS ctxlevel, ctx.instanceid AS ctxinstance
                        FROM mdl_course c
                             LEFT JOIN mdl_context ctx ON (ctx.instanceid = c.id AND ctx.contextlevel = 50)
                   LEFT JOIN mdl_course_categories cat ON cat.id=c.category
                       WHERE c.id <> 1 
                    ORDER BY c.sortorder;
      
                                                                                 QUERY PLAN                                                                           
      ----------------------------------------------------------------------------------------------------------------------------------------------------------------
       Sort  (cost=2681.31..2691.70 rows=4157 width=210) (actual time=195.667..196.797 rows=4157 loops=1)
         Sort Key: c.sortorder
         Sort Method: quicksort  Memory: 1657kB
         ->  Hash Left Join  (cost=922.22..2431.45 rows=4157 width=210) (actual time=9.922..190.237 rows=4157 loops=1)
               Hash Cond: (c.category = cat.id)
               ->  Merge Right Join  (cost=902.84..2354.91 rows=4157 width=142) (actual time=9.605..187.454 rows=4157 loops=1)
                     Merge Cond: (ctx.instanceid = c.id)
                     ->  Index Scan using mdl_cont_ins_ix on mdl_context ctx  (cost=0.00..32361.58 rows=4074 width=47) (actual time=0.024..172.762 rows=4158 loops=1)
                           Filter: (contextlevel = 50)
                     ->  Sort  (cost=902.84..913.23 rows=4157 width=95) (actual time=9.565..10.645 rows=4157 loops=1)
                           Sort Key: c.id
                           Sort Method: quicksort  Memory: 866kB
                           ->  Seq Scan on mdl_course c  (cost=0.00..652.98 rows=4157 width=95) (actual time=0.005..4.394 rows=4157 loops=1)
                                 Filter: (id <> 1)
               ->  Hash  (cost=16.39..16.39 rows=239 width=76) (actual time=0.302..0.302 rows=239 loops=1)
                     Buckets: 1024  Batches: 1  Memory Usage: 27kB
                     ->  Seq Scan on mdl_course_categories cat  (cost=0.00..16.39 rows=239 width=76) (actual time=0.003..0.164 rows=239 loops=1)
       Total runtime: 197.911 ms
      (18 rows)
      

      Is there a way to make this query more efficient or to cache it so that it doesn't make quite as large a hit as it does now?

        Issue Links

          Activity

          Hide
          Petr Škoda added a comment -

          thanks for the report

          Show
          Petr Škoda added a comment - thanks for the report
          Hide
          Petr Škoda added a comment -

          Hello, I have tried to simplify the query, hopefully that together with the new sortorder index will resolve this. Thanks for the report.

          Show
          Petr Škoda added a comment - Hello, I have tried to simplify the query, hopefully that together with the new sortorder index will resolve this. Thanks for the report.
          Hide
          Sam Hemelryk added a comment -

          Hi guys,

          Sorry about this but that load_all_courses method has been recently rewritten as it was horribly buggy.
          The new code is largely different and the SQL statement broken up so this is no longer a clean merge and having reviewed the code here and the code now in that function it looks like these changes are no longer applicable.

          Petr have a look and see what you think. Ping me if you have any questions.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi guys, Sorry about this but that load_all_courses method has been recently rewritten as it was horribly buggy. The new code is largely different and the SQL statement broken up so this is no longer a clean merge and having reviewed the code here and the code now in that function it looks like these changes are no longer applicable. Petr have a look and see what you think. Ping me if you have any questions. Cheers Sam
          Hide
          Petr Škoda added a comment -

          sure, no problem!

          Show
          Petr Škoda added a comment - sure, no problem!
          Hide
          Petr Škoda added a comment -

          resubmitted, I have removed the inequalities in SQL queries again, unfortunately I do not have any test data to actually measure the performance, sorry.

          Sam: feel free to change or reject it, I guess you are the only one who full understands the code, to me it seems that the exclusion of already loaded courses is not used much and might be an over-optimisation worth removing.

          Show
          Petr Škoda added a comment - resubmitted, I have removed the inequalities in SQL queries again, unfortunately I do not have any test data to actually measure the performance, sorry. Sam: feel free to change or reject it, I guess you are the only one who full understands the code, to me it seems that the exclusion of already loaded courses is not used much and might be an over-optimisation worth removing.
          Hide
          Sam Hemelryk added a comment -

          Thanks Petr, I've looked the changes over, everything appears a good improvement so it it went

          The already included sites wouldn't come up very ofter at all, in fact only in situations where navigation had already generated but didn't find the active node. So really only a problem in pages that have edge case navigation needs. And your comment about query caching was interesting as perhaps that would negate it anyway.
          Certainly it is probably a good over-optimisation to remove.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Thanks Petr, I've looked the changes over, everything appears a good improvement so it it went The already included sites wouldn't come up very ofter at all, in fact only in situations where navigation had already generated but didn't find the active node. So really only a problem in pages that have edge case navigation needs. And your comment about query caching was interesting as perhaps that would negate it anyway. Certainly it is probably a good over-optimisation to remove. Cheers Sam
          Hide
          Jason Fowler added a comment -

          Testing on the big4

          Show
          Jason Fowler added a comment - Testing on the big4
          Hide
          Jason Fowler added a comment -

          Works fine under Oracle, next, MS SQL

          Show
          Jason Fowler added a comment - Works fine under Oracle, next, MS SQL
          Hide
          Jason Fowler added a comment -

          MS SQL works fine, next, Postgre SQL

          Show
          Jason Fowler added a comment - MS SQL works fine, next, Postgre SQL
          Hide
          Jason Fowler added a comment -

          PGSQL is fine, next, MySQL

          Show
          Jason Fowler added a comment - PGSQL is fine, next, MySQL
          Hide
          Jason Fowler added a comment -

          MySQL Works fine too, passing

          Show
          Jason Fowler added a comment - MySQL Works fine too, passing
          Hide
          Eloy Lafuente (stronk7) added a comment -

          U P S T R E A M I Z E D !

          Many thanks for the hard work, closing this as fixed.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - U P S T R E A M I Z E D ! Many thanks for the hard work, closing this as fixed. Ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: