Re: [Xen-devel] [OSSTEST PATCH 7/9] ts-hosts-allocate-Executive: Score for equivalent previous failures

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2014-11-11 at 19:41 +0000, Ian Jackson wrote:
> Look to see whether the last run on any hosts which are equivalent to
> the ones we're looking at, failed.  This means that when host X is
> failing and we are considering host Y which is equivalent to X, we
> give Y a selection bonus.
> 
> This means that osstest will be less obsessive about sticking to the
> very same failing host.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> ---
>  ts-hosts-allocate-Executive |   45 
> +++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/ts-hosts-allocate-Executive b/ts-hosts-allocate-Executive
> index 9562a0a..24f78d3 100755
> --- a/ts-hosts-allocate-Executive
> +++ b/ts-hosts-allocate-Executive
> @@ -284,6 +284,29 @@ END
>          $findhostsq->execute("blessed-$fi->{intended}");
>      }
>  
> +    my $equivstatusq= $dbh_tests->prepare(<<END);
> +            SELECT flight, job, val, status
> +           FROM flights f
> +           JOIN jobs j USING (flight)
> +           JOIN runvars r USING (flight,job)
> +             WHERE j.job=?
> +               AND f.blessing=?
> +            AND f.branch=?
> +               AND r.name=?
> +            AND r.val IN (
> +                SELECT hostname
> +                  FROM hostflags
> +                 WHERE hostflag IN (
> +                       SELECT hostflag
> +                         FROM hostflags
> +                        WHERE hostname=?
> +                          AND hostflag LIKE 'equiv-%'
> +                    )
> +                )
> +       ORDER BY f.started DESC
> +          LIMIT 1;
> +END
> +
>      my @candidates;
>      my $any=0;
>  
> @@ -342,6 +365,17 @@ END
>  
>          find_recent_duration($dbg,$hid,$candrow);
>  
> +     if ($candrow->{restype} eq 'host') {
> +         $equivstatusq->execute($job,$fi->{intended},$fi->{branch},
> +                                $hid->{Ident},$candrow->{resname});
> +         my $esrow = $equivstatusq->fetchrow_hashref();

For the first flight on a new branch (or perhaps a new blessing), this
will return an undef, because there is no previous flight to match,
won't it?

http://search.cpan.org/~timb/DBI-1.632/DBI.pm#fetchrow_hashref says if
you get an undef you should check $equivstatusq->err to see if that was
due to an error vs. empty result set. Not sure if you'll care given this
is all heuristics though.

> +         $candrow->{EquivMostRecentStatus} = $esrow->{status};

Meaning this will fail, or perhaps just produce a warning.

> +         print DEBUG "$dbg EQUIV-MOST-RECENT ";
> +         print DEBUG ("$esrow->{flight}.$esrow->{job}".
> +                      " $esrow->{val} $esrow->{status}") if $esrow;
> +         print DEBUG ".\n";

And so will these?

> +     }
> +
>          foreach my $kcomb (qw(Shared-Max-Wear Shared-Max-Tasks)) {
>              my $kdb= $kcomb;  $kdb =~ y/-A-Z/ a-z/;
>              my $khash= $kcomb;  $khash =~ y/-//d;
> @@ -362,6 +396,7 @@ END
>       print DEBUG "$dbg CANDIDATE.\n";
>      }
>      $findhostsq->finish();
> +    $equivstatusq->finish();
>  
>      if (!@candidates) {
>          if (defined $use) {
> @@ -455,6 +490,7 @@ sub hid_recurse ($$) {
>      my $variation_age= 0;
>      my $duration= undef;
>      my $previously_failed = 0;
> +    my $previously_failed_equiv = 0;
>      foreach my $hid (@hids) {
>       my $cand= $hid->{Selected};
>       my $recentstarted= $cand->{MostRecentStarted};
> @@ -465,6 +501,8 @@ sub hid_recurse ($$) {
>           defined($cand->{Duration}) && $cand->{Duration} >= $duration;
>          $previously_failed++ if
>           ($cand->{MostRecentStatus} // '') eq 'fail';
> +     $previously_failed_equiv++ if
> +         ($cand->{EquivMostRecentStatus} // '') eq 'fail';
>      }
>      my $duration_rightaway_adjust= 0;
>      
> @@ -505,12 +543,15 @@ sub hid_recurse ($$) {
>  
>      my $cost= $start_time
>       + $duration_for_cost
> -        - $previously_failed * 366*86400
> +        - ($previously_failed      ==@hids ? 366*86400 :
> +        $previously_failed_equiv==@hids ? 365*86400 :
> +        0)

You've dropped the behaviour of multiplying 366*86400 by
$previously_failed, was that intentional?

I think you've also gone to giving a bonus at all only if all @hids
previously failed, instead of just at least one of them.

>          + ($previously_failed ? + $variation_age * 10 : - $variation_age / 
> 30)
>       - $share_reuse * 10000;
>      
>      print DEBUG "$dbg FINAL start=$start_time va=$variation_age".
> -        " previously_failed=$previously_failed cost=$cost\n";
> +        " previously_failed=$previously_failed".
> +     " previously_failed_equiv=$previously_failed_equiv cost=$cost\n";
>  
>      if (!defined $best || $cost < $best->{Cost}) {
>          print DEBUG "$dbg FINAL BEST: ".



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