[OSSTEST PATCH 0/7] Apropos of XTF: Improve failed step selection

[OSSTEST PATCH 0/7] Apropos of XTF: Improve failed step selection

[Ian Jackson]

Running XTF in osstest is likely to produce failures where multiple
steps fail interestingly.  We would like to prefer to report, and
bisect, earlier steps.

This series does that.

Wei, NB, this has a conflict with the XTF pre series that you have
taken over.  The conflict is not trivial, but easy.  The conflicting
patch is:
  Executive: Previous duration estimator: use overall time, not sum of steps

You may want to consult me when you rebase, or I can move that patch
into this series (which is otherwise ready to push, I think, subject
to any review anyone wants to conduct).

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[OSSTEST PATCH 1/7] duration_estimator: Introduce $duration_duration_qtxt and @d_d_args

[Ian Jackson]

This is going to make it easier to, conditionally, make this query
more complicated.

No functional change.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 Osstest/Executive.pm | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index 7a35a19..a57ea55 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -1056,12 +1056,14 @@ END
 END
     # s J J J # fix perl-mode
 
-    my $duration_duration_q= $dbh_tests->prepare(<<END);
+    my $duration_duration_qtxt= <<END;
             SELECT sum(finished-started) AS duration FROM steps
 		          WHERE flight=? AND job=?
                             AND step != 'ts-hosts-allocate'
 END
 
+    my $duration_duration_q = $dbh_tests->prepare($duration_duration_qtxt);
+
     return sub {
         my ($job, $hostidname, $onhost) = @_;
 
@@ -1098,7 +1100,8 @@ END
 
         my $duration_max= 0;
         foreach my $ref (@$refs) {
-            $duration_duration_q->execute($ref->{flight}, $job);
+	    my @d_d_args = ($ref->{flight}, $job);
+            $duration_duration_q->execute(@d_d_args);
             my ($duration) = $duration_duration_q->fetchrow_array();
             $duration_duration_q->finish();
             if ($duration) {
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[OSSTEST PATCH 2/7] duration_estimator: Reformat inner SQL query with more newlines

[Ian Jackson]

This is going to make the next patch, which adds more complication,
easier to read.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 Osstest/Executive.pm | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index a57ea55..116012b 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -1057,9 +1057,11 @@ END
     # s J J J # fix perl-mode
 
     my $duration_duration_qtxt= <<END;
-            SELECT sum(finished-started) AS duration FROM steps
-		          WHERE flight=? AND job=?
-                            AND step != 'ts-hosts-allocate'
+            SELECT sum(finished-started)
+                AS duration
+              FROM steps
+             WHERE flight=? AND job=?
+               AND step != 'ts-hosts-allocate'
 END
 
     my $duration_duration_q = $dbh_tests->prepare($duration_duration_qtxt);
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[OSSTEST PATCH 3/7] duration_estimator: Reorganise inner SQL query to use WITH

[Ian Jackson]

We are going to, sometimes, want to ask more complicated questions
about the steps in the job of interest.

No functional change, except perhaps to query optimisation.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 Osstest/Executive.pm | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index 116012b..10fdc8d 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -1057,13 +1057,18 @@ END
     # s J J J # fix perl-mode
 
     my $duration_duration_qtxt= <<END;
-            SELECT sum(finished-started)
-                AS duration
+        WITH tsteps AS 
+        (
+            SELECT *
               FROM steps
              WHERE flight=? AND job=?
-               AND step != 'ts-hosts-allocate'
+        )
+            SELECT sum(finished-started)
+                AS duration
+              FROM tsteps
+             WHERE step != 'ts-hosts-allocate'
 END
-
+	
     my $duration_duration_q = $dbh_tests->prepare($duration_duration_qtxt);
 
     return sub {
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[OSSTEST PATCH 4/7] duration_estimator: Be able to estimate job duration up to a particular step

[Ian Jackson]

If this is passed, we are interested only in the duration up to and
including the specified test step.  (If the specified test step is not
present or didn't have a recorded finish, we look at the whole job.)

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 Osstest/Executive.pm | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index 10fdc8d..7e31b35 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -1012,16 +1012,21 @@ END
 
 #---------- duration estimator ----------
 
-sub duration_estimator ($$;$) {
-    my ($branch, $blessing, $debug) = @_;
+sub duration_estimator ($$;$$) {
+    my ($branch, $blessing, $debug, $will_uptoincl_testid) = @_;
     # returns a function which you call like this
-    #    $durest->($job, $hostidname, $onhost)
+    #    $durest->($job, $hostidname, $onhost [, $uptoincl_testid])
     # and returns one of
     #    ($seconds, $samehostlaststarttime, $samehostlaststatus)
     #    ($seconds, undef, undef)
     #    ()
     # $debug should be something like sub { print DEBUG "@_\n"; }.
     # Pass '' for $hostidname and $onhost for asking about on any host
+    #
+    # $uptincl_testid must be passed iff $will_uptoincl_testid, in
+    # which case the duration up to and including that step will be
+    # estimated (and only jobs which contained that step will be
+    # considered).
 
     my $recentflights_q= $dbh_tests->prepare(<<END);
             SELECT f.flight AS flight,
@@ -1068,11 +1073,20 @@ END
               FROM tsteps
              WHERE step != 'ts-hosts-allocate'
 END
+
+    if ($will_uptoincl_testid) {
+	$duration_duration_qtxt .= <<END;
+               AND finished <=
+                     (SELECT finished
+                        FROM tsteps
+                       WHERE tsteps.testid = ?)
+END
+    }
 	
     my $duration_duration_q = $dbh_tests->prepare($duration_duration_qtxt);
 
     return sub {
-        my ($job, $hostidname, $onhost) = @_;
+        my ($job, $hostidname, $onhost, $uptoincl_testid) = @_;
 
         my $dbg= $debug ? sub {
             $debug->("DUR $branch $blessing $job $hostidname $onhost @_");
@@ -1108,6 +1122,7 @@ END
         my $duration_max= 0;
         foreach my $ref (@$refs) {
 	    my @d_d_args = ($ref->{flight}, $job);
+	    push @d_d_args, $uptoincl_testid if $will_uptoincl_testid;
             $duration_duration_q->execute(@d_d_args);
             my ($duration) = $duration_duration_q->fetchrow_array();
             $duration_duration_q->finish();
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[OSSTEST PATCH 5/7] sg-report-flight: Fix misleading value in call to duration_estimator

[Ian Jackson]

The third argument is $debug.  If it's falseish, a default subref is
used instead.  Passing '' is confusing; pass undef instead.

No functional change.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 sg-report-flight | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sg-report-flight b/sg-report-flight
index de1fd14..47ecf07 100755
--- a/sg-report-flight
+++ b/sg-report-flight
@@ -763,7 +763,7 @@ END
 
     my $duration_estimator= duration_estimator($branch, $blessings[0]);
     foreach my $failv (@failures) {
-        my ($est) = $duration_estimator->($failv->{Job}{job},'','');
+        my ($est) = $duration_estimator->($failv->{Job}{job},'',undef);
         if (!defined $est) { $est = 1e5; }
         print DEBUG "DE $failv->{Job}{job} $est\n";
         $failv->{DurationEstimate}= $est;
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[OSSTEST PATCH 6/7] sg-report-flight: Fix a missing newline in a DEBUG message

[Ian Jackson]

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 sg-report-flight | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sg-report-flight b/sg-report-flight
index 47ecf07..d1acb60 100755
--- a/sg-report-flight
+++ b/sg-report-flight
@@ -569,7 +569,7 @@ sub print_pushgate_summary () {
     my @thistree = sort keys %{ $specver{'this'} };
     my @thattree = sort keys %{ $specver{'that'} };
     if (!(@thistree==1 && @thattree==1 && $thistree[0] eq $thattree[0])) {
-	print DEBUG "NO PUSHGATE SUMMARY (@thistree) != (@thattree)";
+	print DEBUG "NO PUSHGATE SUMMARY (@thistree) != (@thattree)\n";
 	return
     }
     my $tree = $thistree[0];
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[OSSTEST PATCH 7/7] sg-report-flight: Report earlier, earlier step failures

[Ian Jackson]

The order of results reported by sg-report-flight determines which
testid the bisector will try to work on first.  It also determines the
order in which failures are shown in the email reports.  We currently
sort them by the duration estimate (for each failure's containing job).

We should prefer earlier steps.  So change the first sort key to be
the duration estimate only for the steps leading up to the step of
interest for each failure.  (By passing the testid to the duration
estimator.)

Since the granularity is in seconds, this may still not distinguish
when there are fast steps.  So as a secondary sort criterion, use the
stepno.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

squash! sg-report-flight: Report earlier, earlier step failures
---
 sg-report-flight | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/sg-report-flight b/sg-report-flight
index d1acb60..ca9ba18 100755
--- a/sg-report-flight
+++ b/sg-report-flight
@@ -761,15 +761,29 @@ sub justifyfailures ($;$) {
 END
     $anypassq= db_prepare($anypassq);
 
-    my $duration_estimator= duration_estimator($branch, $blessings[0]);
+    my $duration_estimator=
+	duration_estimator($branch, $blessings[0], undef, 1);
     foreach my $failv (@failures) {
-        my ($est) = $duration_estimator->($failv->{Job}{job},'',undef);
+        my ($est) = $duration_estimator->($failv->{Job}{job},'',undef,
+					 $failv->{Step}{testid});
         if (!defined $est) { $est = 1e5; }
         print DEBUG "DE $failv->{Job}{job} $est\n";
         $failv->{DurationEstimate}= $est;
     }
 
-    @failures= sort { $a->{DurationEstimate} <=> $b->{DurationEstimate} }
+    @failures= sort {
+	$a->{DurationEstimate} <=> $b->{DurationEstimate}
+	or $a->{Step}{stepno} <=> $b->{Step}{stepno}
+	# stepno is sequential only within each job, so strictly
+	# speaking this is not really a valid comparison: we will
+	# usually be comparing failed steps in different jobs.  But
+	# this comparison is a backstup, secondary to the duration
+	# estimate; and it does mean that if the two failures are
+	# different steps of the same job, and they ran fast so that
+	# they finished in the same second, we pick the lower-numbered
+	# step, which is the earlier one (if they are sequential at
+	# all).
+    }
         @failures;
 
     my @heisenflights;
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [OSSTEST PATCH 0/7] Apropos of XTF: Improve failed step selection

[Wei Liu]

On Thu, Aug 11, 2016 at 05:17:56PM +0100, Ian Jackson wrote:
> Running XTF in osstest is likely to produce failures where multiple
> steps fail interestingly.  We would like to prefer to report, and
> bisect, earlier steps.
> 
> This series does that.
> 
> Wei, NB, this has a conflict with the XTF pre series that you have
> taken over.  The conflict is not trivial, but easy.  The conflicting
> patch is:
>   Executive: Previous duration estimator: use overall time, not sum of steps
> 
> You may want to consult me when you rebase, or I can move that patch
> into this series (which is otherwise ready to push, I think, subject
> to any review anyone wants to conduct).
> 

Thanks for the heads-up!

Wei.

> Thanks,
> Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [OSSTEST PATCH 0/7] Apropos of XTF: Improve failed step selection

[Wei Liu]

On Thu, Aug 11, 2016 at 05:17:56PM +0100, Ian Jackson wrote:
> Running XTF in osstest is likely to produce failures where multiple
> steps fail interestingly.  We would like to prefer to report, and
> bisect, earlier steps.
> 
> This series does that.
> 
> Wei, NB, this has a conflict with the XTF pre series that you have
> taken over.  The conflict is not trivial, but easy.  The conflicting
> patch is:
>   Executive: Previous duration estimator: use overall time, not sum of steps
> 

The rebased patch is as followed, please check.

---8<---
From bbdc727965b4d4280bac670823f013d078a54b93 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Fri, 8 Jul 2016 19:30:58 +0100
Subject: [OSSTEST PATCH] Executive: Previous duration estimator: use overall
 time, not sum of steps
Cc: ian.jackson@eu.citrix.com

Some jobs runs steps in parallel.  Do not add up all the insividual
step durations.  Instead, calculate the duration as the time between
first step start and last step finish.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
[ wei: rebase and fix conflict ]
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 Osstest/Executive.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index 7e31b35..2187a73 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -1068,7 +1068,7 @@ END
               FROM steps
              WHERE flight=? AND job=?
         )
-            SELECT sum(finished-started)
+            SELECT sum(max(finished)-min(started))
                 AS duration
               FROM tsteps
              WHERE step != 'ts-hosts-allocate'
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [OSSTEST PATCH 0/7] Apropos of XTF: Improve failed step selection

[Ian Jackson]

Wei Liu writes ("Re: [OSSTEST PATCH 0/7] Apropos of XTF: Improve failed step selection"):
> The rebased patch is as followed, please check.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [OSSTEST PATCH 0/7] Apropos of XTF: Improve failed step selection

[Andrew Cooper]

On 30/08/16 17:00, Wei Liu wrote:
> On Thu, Aug 11, 2016 at 05:17:56PM +0100, Ian Jackson wrote:
>> Running XTF in osstest is likely to produce failures where multiple
>> steps fail interestingly.  We would like to prefer to report, and
>> bisect, earlier steps.
>>
>> This series does that.
>>
>> Wei, NB, this has a conflict with the XTF pre series that you have
>> taken over.  The conflict is not trivial, but easy.  The conflicting
>> patch is:
>>   Executive: Previous duration estimator: use overall time, not sum of steps
>>
> The rebased patch is as followed, please check.
>
> ---8<---
> From bbdc727965b4d4280bac670823f013d078a54b93 Mon Sep 17 00:00:00 2001
> From: Ian Jackson <ian.jackson@eu.citrix.com>
> Date: Fri, 8 Jul 2016 19:30:58 +0100
> Subject: [OSSTEST PATCH] Executive: Previous duration estimator: use overall
>  time, not sum of steps
> Cc: ian.jackson@eu.citrix.com
>
> Some jobs runs steps in parallel.  Do not add up all the insividual

individual

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [OSSTEST PATCH 0/7] Apropos of XTF: Improve failed step selection

[Wei Liu]

On 
> On Thu, Aug 11, 2016 at 05:17:56PM +0100, Ian Jackson wrote:
> > Running XTF in osstest is likely to produce failures where multiple
> > steps fail interestingly.  We would like to prefer to report, and
> > bisect, earlier steps.
> > 
> > This series does that.
> > 
> > Wei, NB, this has a conflict with the XTF pre series that you have
> > taken over.  The conflict is not trivial, but easy.  The conflicting
> > patch is:
> >   Executive: Previous duration estimator: use overall time, not sum of steps
> > 
> 
> The rebased patch is as followed, please check.
> 
> ---8<---
> From bbdc727965b4d4280bac670823f013d078a54b93 Mon Sep 17 00:00:00 2001
> From: Ian Jackson <ian.jackson@eu.citrix.com>
> Date: Fri, 8 Jul 2016 19:30:58 +0100
> Subject: [OSSTEST PATCH] Executive: Previous duration estimator: use overall
>  time, not sum of steps
> Cc: ian.jackson@eu.citrix.com
> 
> Some jobs runs steps in parallel.  Do not add up all the insividual
> step durations.  Instead, calculate the duration as the time between
> first step start and last step finish.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> [ wei: rebase and fix conflict ]
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  Osstest/Executive.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
> index 7e31b35..2187a73 100644
> --- a/Osstest/Executive.pm
> +++ b/Osstest/Executive.pm
> @@ -1068,7 +1068,7 @@ END
>                FROM steps
>               WHERE flight=? AND job=?
>          )
> -            SELECT sum(finished-started)
> +            SELECT sum(max(finished)-min(started))
>                  AS duration
>                FROM tsteps
>               WHERE step != 'ts-hosts-allocate'
> -- 
> 2.1.4
> 


Hmm... there seems to be a problem with this patch.

http://osstest.xs.citrite.net/~osstest/testlogs/logs/67641/build-amd64-xtf/2.ts-hosts-allocate.log

2016-09-05 17:51:48 Z allocating hosts: host 
DBD::Pg::st execute failed: ERROR:  aggregate function calls cannot be nested
LINE 7:             SELECT sum(max(finished)-min(started))
                               ^ [for Statement "        WITH tsteps AS 
        (
            SELECT *
              FROM steps
             WHERE flight=? AND job=?
        )
            SELECT sum(max(finished)-min(started))
                AS duration
              FROM tsteps
             WHERE step != 'ts-hosts-allocate'
" with ParamValues: 1='67640', 2='build-amd64-xtf'] at Osstest/Executive.pm line 1127, <GEN4> line 10.
resourcecall DBD::Pg::st execute failed: ERROR:  aggregate function calls cannot be nested
LINE 7:             SELECT sum(max(finished)-min(started))
                               ^ [for Statement "        WITH tsteps AS 
        (
            SELECT *
              FROM steps
             WHERE flight=? AND job=?
        )
            SELECT sum(max(finished)-min(started))
                AS duration
              FROM tsteps
             WHERE step != 'ts-hosts-allocate'
" with ParamValues: 1='67640', 2='build-amd64-xtf'] at Osstest/Executive.pm line 1127, <GEN4> line 10.
Died at Osstest/Executive.pm line 903, <GEN4> line 10.
+ rc=255
+ date -u +%Y-%m-%d %H:%M:%S Z exit status 255
2016-09-05 17:51:49 Z exit status 255
+ exit 255

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [OSSTEST PATCH 0/7] Apropos of XTF: Improve failed step selection

[Ian Jackson]

Wei Liu writes ("Re: [OSSTEST PATCH 0/7] Apropos of XTF: Improve failed step selection"):
> > On Thu, Aug 11, 2016 at 05:17:56PM +0100, Ian Jackson wrote:
> > -            SELECT sum(finished-started)
> > +            SELECT sum(max(finished)-min(started))
...
> resourcecall DBD::Pg::st execute failed: ERROR:  aggregate function calls cannot be nested

I am a total idiot.  I didn't spot you had botched the merge.  The new
code should be

  max(finished)-min(started)

without the sum().

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [OSSTEST PATCH 0/7] Apropos of XTF: Improve failed step selection

[Wei Liu]

On Tue, Sep 06, 2016 at 10:45:08AM +0100, Ian Jackson wrote:
> Wei Liu writes ("Re: [OSSTEST PATCH 0/7] Apropos of XTF: Improve failed step selection"):
> > > On Thu, Aug 11, 2016 at 05:17:56PM +0100, Ian Jackson wrote:
> > > -            SELECT sum(finished-started)
> > > +            SELECT sum(max(finished)-min(started))
> ...
> > resourcecall DBD::Pg::st execute failed: ERROR:  aggregate function calls cannot be nested
> 
> I am a total idiot.  I didn't spot you had botched the merge.  The new
> code should be
> 
>   max(finished)-min(started)
> 
> without the sum().

OK. I will fix that. Thanks.

Wei.

> 
> Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel