Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-20578

Performance issues with forum_print_overview()

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9.5, 2.0
    • Fix Version/s: 1.9.6
    • Component/s: Forum, My home
    • Labels:
      None
    • Database:
      Any, PostgreSQL
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE

      Description

      The forum_print_overview() function, used by My Moodle, can be extremely slow for high usage sites. It queries the mdl_log table which can have millions of entries per day. We were finding on some systems that the function was actually taking more than 30 seconds and sometimes minutes. See attached text file for a postgres explain analyze output. The performance can be improved by changing the ILIKE 'add post%' condition to be = 'add post', so that the query can use the log_coumodact_ix index to filter the log.action value.

      My attached patch changes the "ILIKE" to an "=" which is fine because there are never any cases where there is text following the 'add post' (confirmed by code audit + log audit).

      Although this change speeds up the query and reduces DB load, it still is a very expensive query. I think there needs to be a way to determine number of new forum posts without having to scan the log table. Note that the query can contain conditions like "WHERE lastaccess > 0" which causes a requires a lengthy scan of the log table.

        Gliffy Diagrams

        1. forumoverview.txt
          15 kB
          Ashley Holman
        2. forumoverview19.patch
          0.7 kB
          Ashley Holman
        3. forumoverview20.patch
          0.5 kB
          Ashley Holman

          Activity

          Hide
          ashleyholman Ashley Holman added a comment -

          Attaching patch for 19_STABLE.

          Show
          ashleyholman Ashley Holman added a comment - Attaching patch for 19_STABLE.
          Hide
          ashleyholman Ashley Holman added a comment -

          Patch for HEAD.

          Show
          ashleyholman Ashley Holman added a comment - Patch for HEAD.
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          +1000

          That "LIKE" there doesn't seem to have sense at all as far as "add post" is the only one action having the "add post" substring in the forum module. So it's safe to change it to stricter (and quicker) "=". Must cause a HUGE difference.

          About changing the query to avoid using logs, I don't think it will cause big benefits as far as you will be changing it by 2-3 joins and will continue needing the time inequality. So I'm not sure if that will cause any improvement (depends of number of records and so on).

          Really that query mixes some of the worst (from an efficiency pov) things that one SQL can have: inequalities, partial matching and OR-ed conditions. Let's fix at least the partial matching as suggested by Ashley.

          Well spotted! Ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - +1000 That "LIKE" there doesn't seem to have sense at all as far as "add post" is the only one action having the "add post" substring in the forum module. So it's safe to change it to stricter (and quicker) "=". Must cause a HUGE difference. About changing the query to avoid using logs, I don't think it will cause big benefits as far as you will be changing it by 2-3 joins and will continue needing the time inequality. So I'm not sure if that will cause any improvement (depends of number of records and so on). Really that query mixes some of the worst (from an efficiency pov) things that one SQL can have: inequalities, partial matching and OR-ed conditions. Let's fix at least the partial matching as suggested by Ashley. Well spotted! Ciao
          Hide
          dougiamas Martin Dougiamas added a comment -

          +1000 for 1.9.6 . Thanks Ash!

          Show
          dougiamas Martin Dougiamas added a comment - +1000 for 1.9.6 . Thanks Ash!
          Hide
          ashleyholman Ashley Holman added a comment -

          Fixed in CVS as per attached patches.

          Show
          ashleyholman Ashley Holman added a comment - Fixed in CVS as per attached patches.
          Hide
          andyjdavis Andrew Davis added a comment - - edited

          Verified. While there I commented out this
          $LIKE = sql_ilike();
          as the variable was no longer being used. Not that this change will be in 1.9.7

          Show
          andyjdavis Andrew Davis added a comment - - edited Verified. While there I commented out this $LIKE = sql_ilike(); as the variable was no longer being used. Not that this change will be in 1.9.7

            People

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

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                21/Oct/09