xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Ian Jackson <ian.jackson@eu.citrix.com>
To: xen-devel@lists.xenproject.org
Cc: Ian Jackson <ian.jackson@eu.citrix.com>,
	George Dunlap <George.Dunlap@citrix.com>
Subject: [OSSTEST PATCH v2 12/41] Executive: Use index for report__find_test
Date: Fri, 31 Jul 2020 12:37:51 +0100	[thread overview]
Message-ID: <20200731113820.5765-13-ian.jackson@eu.citrix.com> (raw)
In-Reply-To: <20200731113820.5765-1-ian.jackson@eu.citrix.com>

After we refactor this query then we can enable the index use.
(Both of these things together in this commit because I haven't perf
tested the version with just the refactoring.)

(We have provided an index that can answer this question really
quickly if a version is specified.  But the query planner couldn't see
that because it works without seeing the bind variables, so doesn't
know that the value of name is going to be suitable for this index.)

* Convert the two EXISTS subqueries into JOIN/AND with a DISTINCT
  clause naming the fields on flights, so as to replicate the previous
  result rows.  Then do $selection field last.  The subquery is a
  convenient way to let this do the previous thing for all the values
  of $selection (including, notably, *).

* Add the additional AND clause for r.name, which has no logical
  effect given the actual values of name, enabling the query planner
  to use this index.

Perf: In my test case the sg-report-flight runtime is now ~8s.  I am
reasonably confident that this will not make other use cases of this
code worse.

Perf: runtime of my test case now ~11s

Example query before (from the Perl DBI trace):

        SELECT *
         FROM flights f
        WHERE
                EXISTS (
                   SELECT 1
                    FROM runvars r
                   WHERE name=?
                     AND val=?
                     AND r.flight=f.flight
                     AND (      (CASE
       WHEN (r.job) LIKE 'build-%-prev' THEN 'xprev'
       WHEN ((r.job) LIKE 'build-%-freebsd'
             AND 'x' = 'freebsdbuildjob') THEN 'DISCARD'
       ELSE                                      ''
       END)
 = '')
                 )
          AND ( (TRUE AND flight <= 151903) AND (blessing='real') )
          AND (branch=?)
        ORDER BY flight DESC
        LIMIT 1

After:

        SELECT *
          FROM ( SELECT DISTINCT
                      flight, started, blessing, branch, intended
                 FROM flights f
                    JOIN runvars r USING (flight)
                   WHERE name=?
                     AND name LIKE 'revision\_%'
                     AND val=?
                     AND r.flight=f.flight
                     AND (      (CASE
       WHEN (r.job) LIKE 'build-%-prev' THEN 'xprev'
       WHEN ((r.job) LIKE 'build-%-freebsd'
             AND 'x' = 'freebsdbuildjob') THEN 'DISCARD'
       ELSE                                      ''
       END)
 = '')
          AND ( (TRUE AND flight <= 151903) AND (blessing='real') )
          AND (branch=?)
) AS sub WHERE TRUE
        ORDER BY flight DESC
        LIMIT 1

In both cases with bind vars:

   'revision_xen'
   '165f3afbfc3db70fcfdccad07085cde0a03c858b'
   "xen-unstable"

Diff to the example query:

@@ -1,10 +1,10 @@
         SELECT *
+          FROM ( SELECT DISTINCT
+                      flight, started, blessing, branch, intended
          FROM flights f
-        WHERE
-                EXISTS (
-                   SELECT 1
-                    FROM runvars r
+                    JOIN runvars r USING (flight)
                    WHERE name=?
+                     AND name LIKE 'revision\_%'
                      AND val=?
                      AND r.flight=f.flight
                      AND (      (CASE
@@ -14,8 +14,8 @@
        ELSE                                      ''
        END)
  = '')
-                 )
           AND ( (TRUE AND flight <= 151903) AND (blessing='real') )
           AND (branch=?)
+) AS sub WHERE TRUE
         ORDER BY flight DESC
         LIMIT 1

Reviewed-by: George Dunlap <George.Dunlap@citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
v2: Use proper \ escaping for underscores in LIKE
---
 Osstest/Executive.pm              | 20 ++++++++------------
 schema/runvars-revision-index.sql |  2 +-
 2 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index c3dc1261..9208d8af 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -415,37 +415,32 @@ sub report__find_test ($$$$$$$) {
 
     my $querytext = <<END;
         SELECT $selection
-	 FROM flights f
-	WHERE
+          FROM ( SELECT DISTINCT
+                      flight, started, blessing, branch, intended
+   	         FROM flights f
 END
 
     if (defined $revision) {
 	if ($tree eq 'osstest') {
 	    $querytext .= <<END;
-		EXISTS (
-		   SELECT 1
-		    FROM flights_harness_touched t
+		    JOIN flights_harness_touched t USING (flight)
 		   WHERE t.harness=?
-		     AND t.flight=f.flight
-		 )
 END
             push @params, $revision;
 	} else {
 	    $querytext .= <<END;
-		EXISTS (
-		   SELECT 1
-		    FROM runvars r
+		    JOIN runvars r USING (flight)
 		   WHERE name=?
+                     AND name LIKE 'revision\_%'
 		     AND val=?
 		     AND r.flight=f.flight
                      AND ${\ main_revision_job_cond('r.job') }
-		 )
 END
             push @params, "revision_$tree", $revision;
         }
     } else {
 	$querytext .= <<END;
-	    TRUE
+	    WHERE TRUE
 END
     }
 
@@ -460,6 +455,7 @@ END
 END
     push @params, @$branches;
 
+    $querytext .= ") AS sub WHERE TRUE\n";
     $querytext .= $extracond;
     $querytext .= $sortlimit;
 
diff --git a/schema/runvars-revision-index.sql b/schema/runvars-revision-index.sql
index 8871b528..25306354 100644
--- a/schema/runvars-revision-index.sql
+++ b/schema/runvars-revision-index.sql
@@ -1,4 +1,4 @@
--- ##OSSTEST## 008 Preparatory
+-- ##OSSTEST## 008 Needed
 --
 -- This index helps Executive::report__find_test find relevant flights.
 
-- 
2.20.1



  parent reply	other threads:[~2020-07-31 11:56 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 01/41] Add cperl-indent-level to .dir-locals.el Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 02/41] SQL: Use "LIKE" rather than "like", etc Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 03/41] SQL: Fix incorrect LIKE pattern syntax (literals) Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 04/41] SQL: Fix incorrect LIKE pattern syntax (program variables) Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 05/41] sg-report-flight: Add a comment re same-flight search narrowing Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 06/41] sg-report-flight: Sort failures by job name as last resort Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 07/41] schema: Provide indices for sg-report-flight Ian Jackson
2020-07-31 14:21   ` George Dunlap
2020-07-31 14:55     ` Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 08/41] sg-report-flight: Ask the db for flights of interest Ian Jackson
2020-07-31 14:17   ` George Dunlap
2020-07-31 15:43     ` Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 09/41] sg-report-flight: Use WITH to use best index use for $flightsq Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 10/41] sg-report-flight: Use WITH clause to use index for $anypassq Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 11/41] sg-report-flight: Use the job row from the intitial query Ian Jackson
2020-07-31 11:37 ` Ian Jackson [this message]
2020-07-31 11:37 ` [OSSTEST PATCH v2 13/41] duration_estimator: Ignore truncated jobs unless we know the step Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 14/41] duration_estimator: Introduce some _qtxt variables Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 15/41] duration_estimator: Explicitly provide null in general host q Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 16/41] duration_estimator: Return job column in first query Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 17/41] duration_estimator: Move $uptincl_testid to separate @x_params Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 18/41] duration_estimator: Move duration query loop into database Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 19/41] Executive: Drop redundant AND clause Ian Jackson
2020-07-31 14:17   ` George Dunlap
2020-07-31 11:37 ` [OSSTEST PATCH v2 20/41] schema: Add index for quick lookup by host Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 21/41] sg-report-host-history: Find flight limit by flight start date Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 22/41] sg-report-host-history: Drop per-job debug etc Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 23/41] Executive: Export opendb_tests Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 24/41] sg-report-host-history: Add a debug print after sorting jobs Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 25/41] sg-report-host-history: Do the main query per host Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 26/41] sg-report-host-history: Rerganisation: Make mainquery per-host Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 27/41] sg-report-host-history: Rerganisation: Read old logs later Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 28/41] sg-report-host-history: Rerganisation: Change loops Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 29/41] sg-report-host-history: Drop a redundznt AND clause Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 30/41] sg-report-host-history: Fork Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 31/41] schema: Add index to help cs-bisection-step Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 32/41] adhoc-revtuple-generator: Fix an undef warning in a debug print Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 33/41] cs-bisection-step: Generalise qtxt_common_rev_ok Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 34/41] cs-bisection-step: Move an AND Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 35/41] cs-bisection-step: Break out qtxt_common_ok Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 36/41] cs-bisection-step: Use db_prepare a few times instead of ->do Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 37/41] cs-bisection-step: temporary table: Insert only rows we care about Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 38/41] SQL: Change LIKE E'...\\_...' to LIKE '...\_...' Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 39/41] cs-bisection-step: Add a debug print when we run dot(1) Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 40/41] cs-bisection-step: Lay out the revision tuple graph once Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 41/41] duration_estimator: Clarify recentflights query a bit Ian Jackson
2020-07-31 14:04   ` George Dunlap

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200731113820.5765-13-ian.jackson@eu.citrix.com \
    --to=ian.jackson@eu.citrix.com \
    --cc=George.Dunlap@citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).