| Message ID | 1398677082-30965-1-git-send-email-ian.campbell@citrix.com |
|---|---|
| State | New |
| Headers | show |
Ian Campbell writes ("[OSSTEST] ms-planner: add a flight summary html report"):
> I often find myself intested in when a given flight (or the current flight for
> a branch) will complete, which with the current resource plan means searching
> and cycling through trying to figure out which box ends the latest.
>
> Add an explicit flight status summary, which produces a table for each flight
> listing the jobs, there start and end times and the host they run on. The
> output is something like a list of these, ordered by flight number (rendered
> with links(1), header is bold):
>
> Flight 26043 [linux-3.4 real] 4 jobs expected to run until 2014-Apr-27 Sun 23:54:19
> build-amd64-pvops 2014-Apr-27 Sun 20:06:09 2014-Apr-27 Sun 21:21:23 host leaf-beetle
> build-i386-pvops 2014-Apr-27 Sun 21:20:15 2014-Apr-27 Sun 21:31:27 host grain-weevil
> build-amd64 2014-Apr-27 Sun 21:21:23 2014-Apr-27 Sun 21:44:22 host leaf-beetle
> build-i386 2014-Apr-27 Sun 22:18:43 2014-Apr-27 Sun 23:54:19 host field-cricket
>
> I renamed the existing show-html option to show-resource-plan-html for clarity.
Hrm.
I think this is a nice thing to have, but:
> + while (my ($reso,$evts) = each %{ $plan->{Events} }) {
> + # [osstest real] job 26010.test-amd64-amd64-xl-win7-amd64
> + foreach my $evt ( @{$evts} ) {
> + next unless $evt->{Type} =~ m/^(Start|End)$/;
> + next unless $evt->{Info} =~ m/^\[(\S+) (\S+)\] job ([0-9]+)\
\.(\S+) (.*)/;
This is rather a layering violation. Info is for the consumption of
humans. Perhaps the plan should have an explicit "flight.job" field.
You could compute the "[linux-3.4 real]" part by looking at the common
prefix of all the Infos.
Also, can you wrap everything to 75 or so ?
> + $flights{$flight}->{Jobs}{$job} = {
> + Reso => $reso
> + } unless $flights{$flight}->{Jobs}{$job};
This isn't right. A job can have more than one resource. You should
either list each resource as its own line, accumulate a list of
resources.
> + if ( $evt->{Time} > $flights{$flight}->{End} ) {
> + $flights{$flight}->{Last} = $evt;
> + $flights{$flight}->{LastReso} = $reso;
You don't seem to use LastReso and I can't seem to see why you'd want
it.
> + flight_hdr("Flight $flight [$inf->{Branch} $inf->{Blessing}] ".
> + (keys %{$inf->{Jobs}})." jobs ".
> + "expected to run until ".strftime("%Y-%b-%d %a %H:%M:%S"\
, localtime $inf->{End}));
You repeat the strftime rune many times. You should probably use
show_abs_time. (Which produces UTC. I see that ms-planner has a pair
of localtimes in it, which should perhaps be changed.)
> + foreach my $job (sort { $inf->{Jobs}{$a}->{Start}->{Time} cmp $\
inf->{Jobs}{$b}->{Start}->{Time} } keys %{ $inf->{Jobs} }) {
You repeat the $inf->{Jobs}{$x}->{Start}->{Time} expression. Make it
an anonymous subref or something and this will become much clearer.
And I think you should probably sort the jobs by end time, not start
time.
> + my $sevt = $inf->{Jobs}{$job}->{Start};
> + my $eevt = $inf->{Jobs}{$job}->{End};
> + print("<tr>\n");
> + cell($job);
> + cell(strftime("%Y-%b-%d %a %H:%M:%S", localtime $inf->{Jobs\
}{$job}->{Start}->{Time}));
> + cell(strftime("%Y-%b-%d %a %H:%M:%S", localtime $inf->{Jobs\
}{$job}->{End}->{Time}));
How about
foreach my $se (qw(Start End)) {
?
> diff --git a/ms-queuedaemon b/ms-queuedaemon
> index 26d83e2..a5bebd3 100755
> --- a/ms-queuedaemon
> +++ b/ms-queuedaemon
> @@ -224,12 +224,18 @@ proc queuerun-perhaps-step {} {
> proc report-plan {} {
> global c
> if {[catch {
> - exec ./ms-planner show-html > "$c(WebspaceFile)/resource-plan.html"
> + exec ./ms-planner show-resource-plan-html > "$c(WebspaceFile)/r\
esource-plan.html"
> } emsg]} {
> - log "INTERNAL ERROR showing plan html: $emsg"
> + log "INTERNAL ERROR showing resource plan html: $emsg"
> } else {
> log "report-plan OK"
> }
> + if {[catch {
> + exec ./ms-planner show-flight-summary-html > "$c(WebspaceFile)/\
flight-summary.html"
> + } emsg]} {
> + log "INTERNAL ERROR showing flight summary html: $emsg"
> + } else {
> + log "report-flight-summary OK"
Repetition.
Thanks,
Ian.
On Wed, 2014-04-30 at 16:44 +0100, Ian Jackson wrote: > > + while (my ($reso,$evts) = each %{ $plan->{Events} }) { > > + # [osstest real] job 26010.test-amd64-amd64-xl-win7-amd64 > > + foreach my $evt ( @{$evts} ) { > > + next unless $evt->{Type} =~ m/^(Start|End)$/; > > + next unless $evt->{Info} =~ m/^\[(\S+) (\S+)\] job ([0-9]+)\ > \.(\S+) (.*)/; > > This is rather a layering violation. Info is for the consumption of > humans. Perhaps the plan should have an explicit "flight.job" field. I was a bit more scared of touching the actual planner daemon... But ok. > You could compute the "[linux-3.4 real]" part by looking at the common > prefix of all the Infos. > > Also, can you wrap everything to 75 or so ? Sure. > > + $flights{$flight}->{Jobs}{$job} = { > > + Reso => $reso > > + } unless $flights{$flight}->{Jobs}{$job}; > > This isn't right. A job can have more than one resource. You should > either list each resource as its own line, accumulate a list of > resources. Oh yes, I'd forgotten this. > > + if ( $evt->{Time} > $flights{$flight}->{End} ) { > > + $flights{$flight}->{Last} = $evt; > > + $flights{$flight}->{LastReso} = $reso; > > You don't seem to use LastReso and I can't seem to see why you'd want > it. I think I was just being a completest in case I decided I wanted it. > > > + flight_hdr("Flight $flight [$inf->{Branch} $inf->{Blessing}] ". > > + (keys %{$inf->{Jobs}})." jobs ". > > + "expected to run until ".strftime("%Y-%b-%d %a %H:%M:%S"\ > , localtime $inf->{End})); > > You repeat the strftime rune many times. You should probably use > show_abs_time. (Which produces UTC. I see that ms-planner has a pair > of localtimes in it, which should perhaps be changed.) Ack > > + foreach my $job (sort { $inf->{Jobs}{$a}->{Start}->{Time} cmp $\ > inf->{Jobs}{$b}->{Start}->{Time} } keys %{ $inf->{Jobs} }) { > > You repeat the $inf->{Jobs}{$x}->{Start}->{Time} expression. Make it > an anonymous subref or something and this will become much clearer. You mean like: my $ta = sub { return $inf->{Jobs}{$a}->.... } my $tb = sub { return $inf->{Jobs}{$b}->.... } foreach ... (sort { $ta() cmp $tb() } keys ... ) { Or my $to = sub($) { my $it=shift; return $info-{Jobs}{$it}... } and $to($a) ? > And I think you should probably sort the jobs by end time, not start > time. OK. I should note that I realised after I wrote this patch that as jobs complete others might appear, so you can't necessarily know when the flight will end, although once the build jobs are done you can generally get a pretty good idea. > > + my $sevt = $inf->{Jobs}{$job}->{Start}; > > + my $eevt = $inf->{Jobs}{$job}->{End}; > > + print("<tr>\n"); > > + cell($job); > > + cell(strftime("%Y-%b-%d %a %H:%M:%S", localtime $inf->{Jobs\ > }{$job}->{Start}->{Time})); > > + cell(strftime("%Y-%b-%d %a %H:%M:%S", localtime $inf->{Jobs\ > }{$job}->{End}->{Time})); > > How about > foreach my $se (qw(Start End)) { > ? OK. I never would have come up with that myself. > > diff --git a/ms-queuedaemon b/ms-queuedaemon > > index 26d83e2..a5bebd3 100755 > > --- a/ms-queuedaemon > > +++ b/ms-queuedaemon > > @@ -224,12 +224,18 @@ proc queuerun-perhaps-step {} { > > proc report-plan {} { > > global c > > if {[catch { > > - exec ./ms-planner show-html > "$c(WebspaceFile)/resource-plan.html" > > + exec ./ms-planner show-resource-plan-html > "$c(WebspaceFile)/r\ > esource-plan.html" > > } emsg]} { > > - log "INTERNAL ERROR showing plan html: $emsg" > > + log "INTERNAL ERROR showing resource plan html: $emsg" > > } else { > > log "report-plan OK" > > } > > + if {[catch { > > + exec ./ms-planner show-flight-summary-html > "$c(WebspaceFile)/\ > flight-summary.html" > > + } emsg]} { > > + log "INTERNAL ERROR showing flight summary html: $emsg" > > + } else { > > + log "report-flight-summary OK" > > Repetition. I suppose I can probably figure out enough Tcl to avoid it... I was being cowardly about making no-trivial mods ;-) Ian.
Ian Campbell writes ("Re: [OSSTEST] ms-planner: add a flight summary html report"):
> On Wed, 2014-04-30 at 16:44 +0100, Ian Jackson wrote:
> > This is rather a layering violation. Info is for the consumption of
> > humans. Perhaps the plan should have an explicit "flight.job" field.
>
> I was a bit more scared of touching the actual planner daemon... But ok.
I think you can safely add new fields to the events, although the
existing planner daemon probably won't propagate them.
> > > + if ( $evt->{Time} > $flights{$flight}->{End} ) {
> > > + $flights{$flight}->{Last} = $evt;
> > > + $flights{$flight}->{LastReso} = $reso;
> >
> > You don't seem to use LastReso and I can't seem to see why you'd want
> > it.
>
> I think I was just being a completest in case I decided I wanted it.
Right. I think it's probably going to be tedious to define exactly
what this ought to be so I would leave it out.
> > > + foreach my $job (sort { $inf->{Jobs}{$a}->{Start}->{Time} cmp $\
> > inf->{Jobs}{$b}->{Start}->{Time} } keys %{ $inf->{Jobs} }) {
> >
> > You repeat the $inf->{Jobs}{$x}->{Start}->{Time} expression. Make it
> > an anonymous subref or something and this will become much clearer.
>
> You mean like:
> my $ta = sub { return $inf->{Jobs}{$a}->.... }
> my $tb = sub { return $inf->{Jobs}{$b}->.... }
> foreach ... (sort { $ta() cmp $tb() } keys ... ) {
>
> Or my $to = sub($) { my $it=shift; return $info-{Jobs}{$it}... } and
> $to($a) ?
The latter. (Or a http://en.wikipedia.org/wiki/Schwartzian_transform
but that seems overkill.)
Hrm, it turns out that some time in the last decade it became possible
to write local lexical subroutines that aren't coderefs. So try
sub sortkey { ... }
just before the sort.
> I should note that I realised after I wrote this patch that as jobs
> complete others might appear, so you can't necessarily know when the
> flight will end, although once the build jobs are done you can generally
> get a pretty good idea.
Indeed. The reader will have to know about that.
> > How about
> > foreach my $se (qw(Start End)) {
> > ?
>
> OK. I never would have come up with that myself.
Learn Common Lisp :-).
> > > + if {[catch {
> > > + exec ./ms-planner show-flight-summary-html > "$c(WebspaceFile)/\
> > flight-summary.html"
> > > + } emsg]} {
> > > + log "INTERNAL ERROR showing flight summary html: $emsg"
> > > + } else {
> > > + log "report-flight-summary OK"
> >
> > Repetition.
>
> I suppose I can probably figure out enough Tcl to avoid it... I was
> being cowardly about making no-trivial mods ;-)
Heh. Tcl, like Perl, is a nice language to abstract in.
Thanks,
Ian.
On Wed, 2014-04-30 at 17:21 +0100, Ian Jackson wrote: > Ian Campbell writes ("Re: [OSSTEST] ms-planner: add a flight summary html report"): > > On Wed, 2014-04-30 at 16:44 +0100, Ian Jackson wrote: > > > This is rather a layering violation. Info is for the consumption of > > > humans. Perhaps the plan should have an explicit "flight.job" field. > > > > I was a bit more scared of touching the actual planner daemon... But ok. > > I think you can safely add new fields to the events, although the > existing planner daemon probably won't propagate them. "probably", fantastic ;-) > > > > + foreach my $job (sort { $inf->{Jobs}{$a}->{Start}->{Time} cmp $\ > > > inf->{Jobs}{$b}->{Start}->{Time} } keys %{ $inf->{Jobs} }) { > > > > > > You repeat the $inf->{Jobs}{$x}->{Start}->{Time} expression. Make it > > > an anonymous subref or something and this will become much clearer. > > > > You mean like: > > my $ta = sub { return $inf->{Jobs}{$a}->.... } > > my $tb = sub { return $inf->{Jobs}{$b}->.... } > > foreach ... (sort { $ta() cmp $tb() } keys ... ) { > > > > Or my $to = sub($) { my $it=shift; return $info-{Jobs}{$it}... } and > > $to($a) ? > > The latter. (Or a http://en.wikipedia.org/wiki/Schwartzian_transform > but that seems overkill.) Clever. But yes perhaps overkill... > Hrm, it turns out that some time in the last decade it became possible > to write local lexical subroutines that aren't coderefs. So try > sub sortkey { ... } > just before the sort. foreach my $foo(sub sortkey{} sort (keys %hash)) {} ? I don't think that can be what you meant. > > > How about > > > foreach my $se (qw(Start End)) { > > > ? > > > > OK. I never would have come up with that myself. > > Learn Common Lisp :-). Now I have two problems. ;-) Ian.
Ian Campbell writes ("Re: [OSSTEST] ms-planner: add a flight summary html report"):
> On Wed, 2014-04-30 at 17:21 +0100, Ian Jackson wrote:
> > Hrm, it turns out that some time in the last decade it became possible
> > to write local lexical subroutines that aren't coderefs. So try
> > sub sortkey { ... }
> > just before the sort.
These lexical subroutines don't work properly anyway. I knew there
was a reason I didn't do that. Forget about that. What I meant the
first time was something like
my $jsortkey = sub { $inf->{Jobs}{$_[0]}->{Start}->{Time}; };
foreach my $job (sort { $jsortkey->($a) cmp $jsortkey->(b) } keys ...
Ian.
diff --git a/ms-planner b/ms-planner index f045bbf..4d9787d 100755 --- a/ms-planner +++ b/ms-planner @@ -536,7 +536,80 @@ sub showsharetype ($) { return $_; } -sub cmd_show_html () { +sub cmd_show_flight_summary_html () { + get_current_plan(); + + my $earliest= $plan->{Start}; + + my %flights; + my $jobs = 0; + while (my ($reso,$evts) = each %{ $plan->{Events} }) { + # [osstest real] job 26010.test-amd64-amd64-xl-win7-amd64 + foreach my $evt ( @{$evts} ) { + next unless $evt->{Type} =~ m/^(Start|End)$/; + next unless $evt->{Info} =~ m/^\[(\S+) (\S+)\] job ([0-9]+)\.(\S+) (.*)/; + my ($branch,$blessing,$flight,$job,$rest) = ($1,$2,$3,$4,$5); + $flights{$flight} = { + Branch => $branch, + Blessing => $blessing, + End => $evt->{Time}, + Jobs => {}, + Last => $evt, + LastReso => $reso, + } unless $flights{$flight}; + $jobs++; + $flights{$flight}->{Jobs}{$job} = { + Reso => $reso + } unless $flights{$flight}->{Jobs}{$job}; + + $flights{$flight}->{Jobs}{$job}->{Start} = $evt if $evt->{Type} eq "Start"; + $flights{$flight}->{Jobs}{$job}->{End} = $evt if $evt->{Type} eq "End"; + + if ( $evt->{Time} > $flights{$flight}->{End} ) { + $flights{$flight}->{Last} = $evt; + $flights{$flight}->{LastReso} = $reso; + $flights{$flight}->{End} = $evt->{Time}; + } + } + } + + my @cols = ("Job", "Start", "End", "Host"); + + printf("<table>\n<tr>\n"); + printf(" <th align='left'>%s</th>\n", encode_entities($_)) foreach @cols; + printf("</tr>\n"); + + sub flight_hdr($) { + my $text = encode_entities(shift); + printf("<tr><td colspan=%d><b>$text</b></td></tr>", scalar @cols); + } + sub cell($) { + my $text = encode_entities(shift); + printf(" <td>$text</td>\n"); + } + foreach my $flight (sort keys %flights) { + my $inf = $flights{$flight}; + + flight_hdr("Flight $flight [$inf->{Branch} $inf->{Blessing}] ". + (keys %{$inf->{Jobs}})." jobs ". + "expected to run until ".strftime("%Y-%b-%d %a %H:%M:%S", localtime $inf->{End})); + + foreach my $job (sort { $inf->{Jobs}{$a}->{Start}->{Time} cmp $inf->{Jobs}{$b}->{Start}->{Time} } keys %{ $inf->{Jobs} }) { + my $sevt = $inf->{Jobs}{$job}->{Start}; + my $eevt = $inf->{Jobs}{$job}->{End}; + print("<tr>\n"); + cell($job); + cell(strftime("%Y-%b-%d %a %H:%M:%S", localtime $inf->{Jobs}{$job}->{Start}->{Time})); + cell(strftime("%Y-%b-%d %a %H:%M:%S", localtime $inf->{Jobs}{$job}->{End}->{Time})); + cell($inf->{Jobs}{$job}->{Reso}); + print("</tr>\n"); + } + print("<tr><td> </td></tr>\n"); + } + print("</table>\n"); +} + +sub cmd_show_resource_plan_html () { get_current_plan(); my $now= time; diff --git a/ms-queuedaemon b/ms-queuedaemon index 26d83e2..a5bebd3 100755 --- a/ms-queuedaemon +++ b/ms-queuedaemon @@ -224,12 +224,18 @@ proc queuerun-perhaps-step {} { proc report-plan {} { global c if {[catch { - exec ./ms-planner show-html > "$c(WebspaceFile)/resource-plan.html" + exec ./ms-planner show-resource-plan-html > "$c(WebspaceFile)/resource-plan.html" } emsg]} { - log "INTERNAL ERROR showing plan html: $emsg" + log "INTERNAL ERROR showing resource plan html: $emsg" } else { log "report-plan OK" } + if {[catch { + exec ./ms-planner show-flight-summary-html > "$c(WebspaceFile)/flight-summary.html" + } emsg]} { + log "INTERNAL ERROR showing flight summary html: $emsg" + } else { + log "report-flight-summary OK" } proc we-are-thinking {chan} {
I often find myself intested in when a given flight (or the current flight for a branch) will complete, which with the current resource plan means searching and cycling through trying to figure out which box ends the latest. Add an explicit flight status summary, which produces a table for each flight listing the jobs, there start and end times and the host they run on. The output is something like a list of these, ordered by flight number (rendered with links(1), header is bold): Flight 26043 [linux-3.4 real] 4 jobs expected to run until 2014-Apr-27 Sun 23:54:19 build-amd64-pvops 2014-Apr-27 Sun 20:06:09 2014-Apr-27 Sun 21:21:23 host leaf-beetle build-i386-pvops 2014-Apr-27 Sun 21:20:15 2014-Apr-27 Sun 21:31:27 host grain-weevil build-amd64 2014-Apr-27 Sun 21:21:23 2014-Apr-27 Sun 21:44:22 host leaf-beetle build-i386 2014-Apr-27 Sun 22:18:43 2014-Apr-27 Sun 23:54:19 host field-cricket I renamed the existing show-html option to show-resource-plan-html for clarity. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> --- It'd be good to sync these reports to e.g. xenbits so other people can see them. Pressumably this would be reasonably easy with a ssh keypair restricted in authorized_keys. --- ms-planner | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++- ms-queuedaemon | 10 ++++++-- 2 files changed, 82 insertions(+), 3 deletions(-)