catalyst / moodle-tool_excimer

A Moodle tool to find bottlenecks in your code safely in production
https://moodle.org/plugins/tool_excimer
GNU General Public License v3.0
13 stars 9 forks source link

perf: adjust handling based on real data where single DELETE was bottleneck #360

Closed keevan closed 2 months ago

keevan commented 2 months ago

Replace 'DELETE FROM where id NOT IN' with a SELECT and DELETE statement where id is in a known scope, better utilising indexes

Here is the explain for the DELETE prior to this patch:

+----+--------------------+------------------------------+------------+------+---------------+------+---------+------+-------+----------+---------------------------------+
| id | select_type        | table                        | partitions | type | possible_keys | key  | key_len | ref  | rows  | filtered | Extra                           |
+----+--------------------+------------------------------+------------+------+---------------+------+---------+------+-------+----------+---------------------------------+
|  1 | DELETE             | mdl_tool_excimer_page_groups | NULL       | ALL  | NULL          | NULL | NULL    | NULL | 50341 |   100.00 | Using where                     |
|  2 | DEPENDENT SUBQUERY | <derived3>                   | NULL       | ALL  | NULL          | NULL | NULL    | NULL | 50341 |   100.00 | Using temporary; Using filesort |
|  3 | DERIVED            | mdl_tool_excimer_page_groups | NULL       | ALL  | NULL          | NULL | NULL    | NULL | 50341 |   100.00 | NULL                            |
+----+--------------------+------------------------------+------------+------+---------------+------+---------+------+-------+----------+---------------------------------+

Here's an example of the difference between having a NOT IN vs an IN check, with dummy data:

mysql> explain      DELETE FROM mdl_tool_excimer_page_groups              WHERE id in (1,2,3);
+----+-------------+------------------------------+------------+-------+---------------+---------+---------+-------+------+----------+-------------+
| id | select_type | table                        | partitions | type  | possible_keys | key     | key_len | ref   | rows | filtered | Extra       |
+----+-------------+------------------------------+------------+-------+---------------+---------+---------+-------+------+----------+-------------+
|  1 | DELETE      | mdl_tool_excimer_page_groups | NULL       | range | PRIMARY       | PRIMARY | 8       | const |    3 |   100.00 | Using where |
+----+-------------+------------------------------+------------+-------+---------------+---------+---------+-------+------+----------+-------------+
1 row in set (0.00 sec)

mysql> explain      DELETE FROM mdl_tool_excimer_page_groups              WHERE id NOT in (1,2,3);
+----+-------------+------------------------------+------------+-------+---------------+---------+---------+-------+-------+----------+-------------+
| id | select_type | table                        | partitions | type  | possible_keys | key     | key_len | ref   | rows  | filtered | Extra       |
+----+-------------+------------------------------+------------+-------+---------------+---------+---------+-------+-------+----------+-------------+
|  1 | DELETE      | mdl_tool_excimer_page_groups | NULL       | range | PRIMARY       | PRIMARY | 8       | const | 24167 |   100.00 | Using where |
+----+-------------+------------------------------+------------+-------+---------------+---------+---------+-------+-------+----------+-------------+
1 row in set (0.00 sec)

There's substantially less checks if we know which specific ids we need to remove.

jwalits commented 2 months ago

Patch looks good to me

danmarsden commented 1 month ago

Fyi - Looks like a hardcoded mdl prefix snuck into this pr.