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

      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?

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Petr Skoda added a comment -

            thanks for the report

            Show
            Petr Skoda added a comment - thanks for the report
            Hide
            Petr Skoda 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 Skoda 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 Skoda added a comment -

            sure, no problem!

            Show
            Petr Skoda added a comment - sure, no problem!
            Hide
            Petr Skoda 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 Skoda 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:
                1 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: