Moodle
  1. Moodle
  2. MDL-20578

Performance issues with forum_print_overview()

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor 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
    • Rank:
      36478

      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.

      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
        Ashley Holman added a comment -

        Attaching patch for 19_STABLE.

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

        Patch for HEAD.

        Show
        Ashley Holman added a comment - Patch for HEAD.
        Hide
        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
        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
        Martin Dougiamas added a comment -

        +1000 for 1.9.6 . Thanks Ash!

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

        Fixed in CVS as per attached patches.

        Show
        Ashley Holman added a comment - Fixed in CVS as per attached patches.
        Hide
        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
        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: