launchpad-reviewers team mailing list archive

[Colin Watson <mp+435749@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:optimize-specification-search into launchpad:master.

Commit message:
Fix query plan for specification searches

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1995686 in Launchpad itself: "openstack-neutron launchpad start page timeout"
  https://bugs.launchpad.net/launchpad/+bug/1995686

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/435749

When given base clauses, `search_specifications` is intended to assume that those are more selective than other criteria: for example, starting by filtering specifications by project is likely to cut down the row count a lot more than filtering by status.  However, it constructed a query of the form:

    WITH RelevantSpecification AS (...)
    SELECT Specification.*
    FROM Specification
    WHERE
        Specification.id IN (
            SELECT RelevantSpecification.id FROM RelevantSpecification
        )
        AND ...;

This caused the PostgreSQL planner to find relevant specifications using the base clauses, then filter all specifications according to the other clauses, then join the results of those two searches together: the worst of both worlds.  Instead, we want a query of the form:

    WITH RelevantSpecification AS (...)
    SELECT Specification.*
    FROM
        Specification
        JOIN RelevantSpecification
             ON Specification.id = RelevantSpecification.id
    WHERE ...;

This results in a query plan that finds relevant specifications using the base clauses and then filters the result of that search using the other clauses.  Much more sensible, and takes milliseconds rather than seconds.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:optimize-specification-search into launchpad:master.

diff --git a/lib/lp/blueprints/model/specificationsearch.py b/lib/lp/blueprints/model/specificationsearch.py
index f505628..243e4c9 100644
--- a/lib/lp/blueprints/model/specificationsearch.py
+++ b/lib/lp/blueprints/model/specificationsearch.py
@@ -111,12 +111,14 @@ def search_specifications(
             Select(Specification.id, And(clauses), tables=tables),
         )
         store = store.with_(relevant_specification_cte)
-        tables = [Specification]
-        clauses = [
-            Specification.id.is_in(
-                Select(Column("id", RelevantSpecification))
+        tables = [
+            Specification,
+            Join(
+                RelevantSpecification,
+                Specification.id == Column("id", RelevantSpecification),
             ),
         ]
+        clauses = []
     clauses.extend(get_specification_privacy_filter(user))
     clauses.extend(get_specification_filters(spec_filter))