Bug 1573081

(In reply to Dan Callaghan from comment #0)
> MySQL [beaker_dcallagh]> SELECT recipe_set.id AS recipe_set_id 
>     -> FROM recipe_set INNER JOIN job ON job.id = recipe_set.job_id 
>     -> WHERE job.deleted IS NULL AND NOT (EXISTS (SELECT 1 
>     -> FROM recipe 
>     -> WHERE recipe_set.id = recipe.recipe_set_id AND recipe.status !=
> 'Scheduled'));
> Empty set (24.99 sec)

This is the query from provision_scheduled_recipesets(). The backwards-looking double negative with NOT EXISTS ... != is actually saying, "give me all recipe sets where all the recipes in the set are Scheduled".

> MySQL [beaker_dcallagh]> SELECT recipe_set.id AS recipe_set_id  FROM
> recipe_set INNER JOIN job ON job.id = recipe_set.job_id  WHERE job.deleted
> IS NULL AND NOT (EXISTS (SELECT 1  FROM recipe  WHERE recipe_set.id =
> recipe.recipe_set_id AND recipe.status != 'Processed')) ORDER BY
> recipe_set.priority DESC, recipe_set.id;
> Empty set (27.57 sec)

This is the query from queue_processed_recipesets(). Its structure is the same as above, "give me all recipe sets where all the recipes in the set are Processed".

Interestingly, even though the EXPLAIN output indicates the condition on job.deleted is problematic (and we could potentially get rid of it), taking it out of the queries actually makes them about *three times slower* even when they are still matching 0 rows.

The problem is, as always, the NOT EXISTS clause. The EXPLAIN shows that it is scanning all non-deleted rows in the job table (2 million or so). It doesn't pick the index on recipe.status.

The usual trick to avoid problematic dependent subqueries is to convert them to a join. In this case, we are only selecting back recipe_set IDs (not the whole rows) which gives us a lot of leeway to do GROUP BY/DISTINCT hackery.

And indeed, I came up with the following which works nicely.

The point of the NOT EXISTS is that we want to select only recipe *sets* where all recipes in the set are Processed. We can express that with some COUNT()s and GROUP BY recipe_set.id:

SELECT recipe_set.id
FROM recipe_set
INNER JOIN recipe all_recipes on recipe_set.id = all_recipes.recipe_set_id
INNER JOIN recipe processed_recipes on recipe_set.id = processed_recipes.recipe_set_id and processed_recipes.status = 'Processed'
GROUP BY recipe_set.id
HAVING COUNT(DISTINCT all_recipes.id) = COUNT(DISTINCT processed_recipes.id);

I think this gives us the right answer, and it comes back in 0.25 seconds. It tricks MySQL into using the recipe.status index first (which is efficient because there is always a very small number of Processed recipes).

+------+-------------+-------------------+--------+----------------------+---------------+---------+----------------------------------------+------+----------------------------------------------------+
| id   | select_type | table             | type   | possible_keys        | key           | key_len | ref                                    | rows | Extra                                              |
+------+-------------+-------------------+--------+----------------------+---------------+---------+----------------------------------------+------+----------------------------------------------------+
|    1 | SIMPLE      | processed_recipes | ref    | recipe_set_id,status | status        | 1       | const                                  |    5 | Using index condition; Using where; Using filesort |
|    1 | SIMPLE      | recipe_set        | eq_ref | PRIMARY              | PRIMARY       | 4       | beaker.processed_recipes.recipe_set_id |    1 | Using index                                        |
|    1 | SIMPLE      | all_recipes       | ref    | recipe_set_id        | recipe_set_id | 4       | beaker.processed_recipes.recipe_set_id |    1 | Using index                                        |
+------+-------------+-------------------+--------+----------------------+---------------+---------+----------------------------------------+------+----------------------------------------------------+
3 rows in set (0.26 sec)

We can safely drop the condition on Job.is_deleted as well. It was only added as an optimisation hack (incidentally, in a previous attempt at fixing this same problem) back in bug 1062480.

All the code paths for marking a job as deleted correctly enforce that the job is already in a finished state. And I confirmed in our production database that all deleted jobs (and their recipe sets and recipes) are Completed, Cancelled, or Aborted.

MariaDB [beaker]> select count(*) from job where deleted is not null and status not in ('Completed', 'Aborted', 'Cancelled');
+----------+
| count(*) |
+----------+
|        0 |
+----------+
1 row in set (2.13 sec)

MariaDB [beaker]> select count(*) from job inner join recipe_set on job.id = recipe_set.job_id where job.deleted is not null and recipe_set.status not in ('Completed', 'Aborted', 'Cancelled');
+----------+
| count(*) |
+----------+
|        0 |
+----------+
1 row in set (26.47 sec)

MariaDB [beaker]> select count(*) from job inner join recipe_set on job.id = recipe_set.job_id inner join recipe on recipe_set.id = recipe.recipe_set_id where job.deleted is not null and recipe.status not in ('Completed', 'Aborted', 'Cancelled');
+----------+
| count(*) |
+----------+
|        0 |
+----------+
1 row in set (1 min 8.50 sec)

https://gerrit.beaker-project.org/6096