diff mbox series

[OSSTEST,06/14] sg-report-flight: Use WITH clause to use index for $anypassq

Message ID 20200721184205.15232-7-ian.jackson@eu.citrix.com (mailing list archive)
State New, archived
Headers show
Series Flight report performance improvements | expand

Commit Message

Ian Jackson July 21, 2020, 6:41 p.m. UTC 
Perf: runtime of my test case now ~11s

Example query before (from the Perl DBI trace):

        SELECT * FROM flights JOIN steps USING (flight)
            WHERE (branch='xen-unstable')
              AND job=? and testid=? and status='pass'
              AND ( (TRUE AND flight <= 151903) AND (blessing='real') )
            LIMIT 1

After:

        WITH s AS
        (
        SELECT * FROM steps
         WHERE job=? and testid=? and status='pass'
        )
        SELECT * FROM flights JOIN s USING (flight)
            WHERE (branch='xen-unstable')
              AND ( (TRUE AND flight <= 151903) AND (blessing='real') )
            LIMIT 1

In both cases with bind vars:

   "test-amd64-i386-xl-pvshim"
   "guest-start"

Diff to the query:

-        SELECT * FROM flights JOIN steps USING (flight)
+        WITH s AS
+        (
+        SELECT * FROM steps
+         WHERE job=? and testid=? and status='pass'
+        )
+        SELECT * FROM flights JOIN s USING (flight)
             WHERE (branch='xen-unstable')
-              AND job=? and testid=? and status='pass'
               AND ( (TRUE AND flight <= 151903) AND (blessing='real') )
             LIMIT 1

CC: George Dunlap <George.Dunlap@citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 schema/steps-job-index.sql |  2 +-
 sg-report-flight           | 14 ++++++++++++--
 2 files changed, 13 insertions(+), 3 deletions(-)

Comments

George Dunlap July 27, 2020, 4:15 p.m. UTC | #1 
> On Jul 21, 2020, at 7:41 PM, Ian Jackson <ian.jackson@eu.citrix.com> wrote:
> 
> Perf: runtime of my test case now ~11s
> 
> Example query before (from the Perl DBI trace):
> 
>        SELECT * FROM flights JOIN steps USING (flight)
>            WHERE (branch='xen-unstable')
>              AND job=? and testid=? and status='pass'
>              AND ( (TRUE AND flight <= 151903) AND (blessing='real') )
>            LIMIT 1
> 
> After:
> 
>        WITH s AS
>        (
>        SELECT * FROM steps
>         WHERE job=? and testid=? and status='pass'
>        )
>        SELECT * FROM flights JOIN s USING (flight)
>            WHERE (branch='xen-unstable')
>              AND ( (TRUE AND flight <= 151903) AND (blessing='real') )
>            LIMIT 1
> 
> In both cases with bind vars:
> 
>   "test-amd64-i386-xl-pvshim"
>   "guest-start"
> 
> Diff to the query:
> 
> -        SELECT * FROM flights JOIN steps USING (flight)
> +        WITH s AS
> +        (
> +        SELECT * FROM steps
> +         WHERE job=? and testid=? and status='pass'
> +        )
> +        SELECT * FROM flights JOIN s USING (flight)
>             WHERE (branch='xen-unstable')
> -              AND job=? and testid=? and status='pass'
>               AND ( (TRUE AND flight <= 151903) AND (blessing='real') )
>             LIMIT 1
> 
> CC: George Dunlap <George.Dunlap@citrix.com>
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
> ---
> schema/steps-job-index.sql |  2 +-
> sg-report-flight           | 14 ++++++++++++--
> 2 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/schema/steps-job-index.sql b/schema/steps-job-index.sql
> index 07dc5a30..2c33af72 100644
> --- a/schema/steps-job-index.sql
> +++ b/schema/steps-job-index.sql
> @@ -1,4 +1,4 @@
> --- ##OSSTEST## 006 Preparatory
> +-- ##OSSTEST## 006 Needed
> --
> -- This index helps sg-report-flight find if a test ever passed.
> 
> diff --git a/sg-report-flight b/sg-report-flight
> index b5398573..b8d948da 100755
> --- a/sg-report-flight
> +++ b/sg-report-flight
> @@ -849,10 +849,20 @@ sub justifyfailures ($;$) {
> 
>     my @failures= values %{ $fi->{Failures} };
> 
> +    # In psql 9.6 this WITH clause makes postgresql do the steps query
> +    # first.  This is good because if this test never passed we can
> +    # determine that really quickly using the new index, without
> +    # having to scan the flights table.  (If the test passed we will
> +    # probably not have to look at many flights to find one, so in
> +    # that case this is not much worse.)

Seems a bit weird, but OK.  The SQL looks the same, so:

Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Ian Jackson July 31, 2020, 10:41 a.m. UTC | #2 
George Dunlap writes ("Re: [OSSTEST PATCH 06/14] sg-report-flight: Use WITH clause to use index for $anypassq"):
> > On Jul 21, 2020, at 7:41 PM, Ian Jackson <ian.jackson@eu.citrix.com> wrote:
> > +    # In psql 9.6 this WITH clause makes postgresql do the steps query
> > +    # first.  This is good because if this test never passed we can
> > +    # determine that really quickly using the new index, without
> > +    # having to scan the flights table.  (If the test passed we will
> > +    # probably not have to look at many flights to find one, so in
> > +    # that case this is not much worse.)
> 
> Seems a bit weird, but OK.  The SQL looks the same, so:
> 
> Reviewed-by: George Dunlap <george.dunlap@citrix.com>

Thanks.  This business with the WITH clause as an optimisation fence
is well-known in Postgres circles, it seems.

Ian.
diff mbox series

Patch

diff --git a/schema/steps-job-index.sql b/schema/steps-job-index.sql
index 07dc5a30..2c33af72 100644
--- a/schema/steps-job-index.sql
+++ b/schema/steps-job-index.sql
@@ -1,4 +1,4 @@ 
--- ##OSSTEST## 006 Preparatory
+-- ##OSSTEST## 006 Needed
 --
 -- This index helps sg-report-flight find if a test ever passed.
 
diff --git a/sg-report-flight b/sg-report-flight
index b5398573..b8d948da 100755
--- a/sg-report-flight
+++ b/sg-report-flight
@@ -849,10 +849,20 @@  sub justifyfailures ($;$) {
 
     my @failures= values %{ $fi->{Failures} };
 
+    # In psql 9.6 this WITH clause makes postgresql do the steps query
+    # first.  This is good because if this test never passed we can
+    # determine that really quickly using the new index, without
+    # having to scan the flights table.  (If the test passed we will
+    # probably not have to look at many flights to find one, so in
+    # that case this is not much worse.)
     my $anypassq= <<END;
-        SELECT * FROM flights JOIN steps USING (flight)
+        WITH s AS
+        (
+        SELECT * FROM steps
+         WHERE job=? and testid=? and status='pass'
+        )
+        SELECT * FROM flights JOIN s USING (flight)
             WHERE $branches_cond_q
-              AND job=? and testid=? and status='pass'
               AND $blessingscond
             LIMIT 1
 END