-
Improvement
-
Resolution: Unresolved
-
Minor
-
None
-
4.5
There is a SQL query written in grade_recover_history_grades:
$sql = "SELECT h.id, gi.itemtype, gi.itemmodule, gi.iteminstance as iteminstance, gi.itemnumber, h.source, h.itemid, h.userid, h.rawgrade, h.rawgrademax,
|
h.rawgrademin, h.rawscaleid, h.usermodified, h.finalgrade, h.hidden, h.locked, h.locktime, h.exported, h.overridden, h.excluded, h.feedback, h.feedbackformat, h.information, h.informationformat, h.timemodified, itemcreated.tm AS timecreated
|
FROM {grade_grades_history} h
|
JOIN (SELECT itemid, MAX(id) AS id
|
FROM {grade_grades_history}
|
WHERE userid = :userid1
|
GROUP BY itemid) maxquery ON h.id = maxquery.id AND h.itemid = maxquery.itemid
|
JOIN {grade_items} gi ON gi.id = h.itemid
|
JOIN (SELECT itemid, MAX(timemodified) AS tm
|
FROM {grade_grades_history}
|
WHERE userid = :userid2 AND action = :insertaction
|
GROUP BY itemid) itemcreated ON itemcreated.itemid = h.itemid
|
WHERE gi.courseid = :courseid";
|
The inner SELECT statements here are not limited by course, thus they look through the grade history of a given user across the entire instance of Moodle. This results in abysmal performance. Here is an EXPLAIN SELECT on the query above from one of our databases (Postgres) to illustrate that:
moodle2_otagounifom=# EXPLAIN SELECT h.id, gi.itemtype, gi.itemmodule, gi.iteminstance as iteminstance, gi.itemnumber, h.source, h.itemid, h.userid, h.rawgrade, h.rawgrademax,
|
h.rawgrademin, h.rawscaleid, h.usermodified, h.finalgrade, h.hidden, h.locked, h.locktime, h.exported, h.overridden, h.excluded, h.feedback,
|
h.feedbackformat, h.information, h.informationformat, h.timemodified, itemcreated.tm AS timecreated
|
FROM mdl_grade_grades_history h
|
JOIN (SELECT itemid, MAX(id) AS id
|
FROM mdl_grade_grades_history
|
WHERE userid = 8449
|
GROUP BY itemid) maxquery ON h.id = maxquery.id AND h.itemid = maxquery.itemid
|
JOIN mdl_grade_items gi ON gi.id = h.itemid
|
JOIN (SELECT itemid, MAX(timemodified) AS tm
|
FROM mdl_grade_grades_history
|
WHERE userid = 8449 AND action = 1
|
GROUP BY itemid) itemcreated ON itemcreated.itemid = h.itemid
|
WHERE gi.courseid = 2235;
|
QUERY PLAN
|
---------------------------------------------------------------------------------------------------------------------------------------------------------------
|
Nested Loop (cost=244941.95..245139.22 rows=1 width=480)
|
-> Nested Loop (cost=244941.38..245087.70 rows=6 width=67)
|
Join Filter: (gi.id = mdl_grade_grades_history_1.itemid)
|
-> Merge Join (cost=84155.28..84196.76 rows=1 width=51)
|
Merge Cond: (gi.id = mdl_grade_grades_history.itemid)
|
-> Sort (cost=22.64..22.67 rows=11 width=35)
|
Sort Key: gi.id
|
-> Index Scan using mdl_graditem_cou_ix on mdl_grade_items gi (cost=0.29..22.45 rows=11 width=35)
|
Index Cond: (courseid = 2235)
|
-> GroupAggregate (cost=84132.64..84157.91 rows=1291 width=16)
|
Group Key: mdl_grade_grades_history.itemid
|
-> Sort (cost=84132.64..84136.76 rows=1649 width=16)
|
Sort Key: mdl_grade_grades_history.itemid
|
-> Bitmap Heap Scan on mdl_grade_grades_history (cost=77524.87..84044.52 rows=1649 width=16)
|
Recheck Cond: ((userid = 8449) AND (action = 1))
|
-> BitmapAnd (cost=77524.87..77524.87 rows=1649 width=0)
|
-> Bitmap Index Scan on mdl_gradgradhist_use_ix (cost=0.00..487.68 rows=45215 width=0)
|
Index Cond: (userid = 8449)
|
-> Bitmap Index Scan on mdl_gradgradhist_act_ix (cost=0.00..77036.11 rows=6717272 width=0)
|
Index Cond: (action = 1)
|
-> HashAggregate (cost=160786.10..160818.36 rows=3226 width=16)
|
Group Key: mdl_grade_grades_history_1.itemid
|
-> Index Scan using mdl_gradgradhist_use_ix on mdl_grade_grades_history mdl_grade_grades_history_1 (cost=0.57..160560.02 rows=45215 width=16)
|
Index Cond: (userid = 8449)
|
-> Index Scan using mdl_gradgradhist_id_pk on mdl_grade_grades_history h (cost=0.57..8.58 rows=1 width=445)
|
Index Cond: (id = (max(mdl_grade_grades_history_1.id)))
|
Filter: (mdl_grade_grades_history_1.itemid = itemid)
|
The patch aims to improve the performance of this query by several orders of magnitude. Here is an EXPLAIN SELECT from the same database with the tweaked query:
moodle2_otagounifom=# EXPLAIN SELECT h.id, gi.itemtype, gi.itemmodule, gi.iteminstance as iteminstance, gi.itemnumber, h.source, h.itemid, h.userid, h.rawgrade, h.rawgrademax, h.rawgrademin, h.rawscaleid, h.usermodified, h.finalgrade, h.hidden, h.locked, h.locktime, h.exported, h.overridden, h.excluded, h.feedback, h.feedbackformat, h.information, h.informationformat, h.timemodified, itemcreated.tm AS timecreated
|
FROM mdl_grade_grades_history h
|
JOIN (SELECT itemid, MAX(ggh.id) AS id
|
FROM mdl_grade_grades_history ggh
|
JOIN mdl_grade_items gi ON ggh.itemid = gi.id
|
WHERE userid = 8449
|
AND courseid = 2235
|
GROUP BY itemid) maxquery ON h.id = maxquery.id AND h.itemid = maxquery.itemid
|
JOIN mdl_grade_items gi ON gi.id = h.itemid
|
JOIN (SELECT itemid, MAX(ggh.timemodified) AS tm
|
FROM mdl_grade_grades_history ggh
|
JOIN mdl_grade_items gi ON ggh.itemid = gi.id
|
WHERE userid = 8449
|
AND action = 1
|
AND courseid = 2235
|
GROUP BY itemid) itemcreated ON itemcreated.itemid = h.itemid
|
WHERE gi.courseid = 2235;
|
QUERY PLAN
|
----------------------------------------------------------------------------------------------------------------------------------------------------------
|
Nested Loop (cost=1314.06..1330.58 rows=1 width=480)
|
Join Filter: (h.itemid = gi.id)
|
-> Nested Loop (cost=1313.77..1322.26 rows=1 width=469)
|
-> Merge Join (cost=1313.20..1313.66 rows=1 width=32)
|
Merge Cond: (ggh.itemid = ggh_1.itemid)
|
-> GroupAggregate (cost=657.25..657.49 rows=14 width=16)
|
Group Key: ggh.itemid
|
-> Sort (cost=657.25..657.28 rows=14 width=16)
|
Sort Key: ggh.itemid
|
-> Nested Loop (cost=0.86..656.98 rows=14 width=16)
|
-> Index Scan using mdl_graditem_cou_ix on mdl_grade_items gi_1 (cost=0.29..22.45 rows=11 width=8)
|
Index Cond: (courseid = 2235)
|
-> Index Scan using mdl_gradgradhist_useitetim_ix on mdl_grade_grades_history ggh (cost=0.57..57.54 rows=14 width=16)
|
Index Cond: ((userid = 8449) AND (itemid = gi_1.id))
|
-> GroupAggregate (cost=655.95..655.97 rows=1 width=16)
|
Group Key: ggh_1.itemid
|
-> Sort (cost=655.95..655.95 rows=1 width=16)
|
Sort Key: ggh_1.itemid
|
-> Nested Loop (cost=0.86..655.94 rows=1 width=16)
|
-> Index Scan using mdl_graditem_cou_ix on mdl_grade_items gi_2 (cost=0.29..22.45 rows=11 width=8)
|
Index Cond: (courseid = 2235)
|
-> Index Scan using mdl_gradgradhist_useitetim_ix on mdl_grade_grades_history ggh_1 (cost=0.57..57.58 rows=1 width=16)
|
Index Cond: ((userid = 8449) AND (itemid = gi_2.id))
|
Filter: (action = 1)
|
-> Index Scan using mdl_gradgradhist_id_pk on mdl_grade_grades_history h (cost=0.57..8.59 rows=1 width=445)
|
Index Cond: (id = (max(ggh.id)))
|
Filter: (ggh.itemid = itemid)
|
-> Index Scan using mdl_graditem_id_pk on mdl_grade_items gi (cost=0.29..8.31 rows=1 width=35)
|
Index Cond: (id = ggh.itemid)
|
Filter: (courseid = 2235)
|
It is worth noting that the semantics of this query are quite muddy. This patch should have no effect on the rows that are returned since the outer query drops any rows that aren't associated with the courseid anyway. Perhaps someone braver than me would like to rewrite it entirely.
For some additional context, this performance issue popped up when our admins were uploading users to cohorts that in turn had enrollment methods registered against them. We have the "recovergradesdefault" admin setting enabled, which means the queries above are called for every single enrollment that results from this upload. It made the upload process unworkably slow for our admins.