From 57b2bb067419a07b207f55e82ef309533b9dbee5 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <eelco.dolstra@logicblox.com>
Date: Thu, 23 May 2013 10:45:49 -0400
Subject: [PATCH] Let Builds.timestamp refer to the time the build was added

Previously, for scheduled builds, "timestamp" contained the time the
build was added to the queue, while for finished builds, it was the
time the build finished.  Now it's always the former.
---
 src/lib/Hydra/Base/Controller/ListBuilds.pm |  6 +++---
 src/lib/Hydra/Controller/API.pm             |  4 ++--
 src/lib/Hydra/Controller/Admin.pm           |  3 ++-
 src/lib/Hydra/Controller/Build.pm           |  7 +++++--
 src/lib/Hydra/Controller/Job.pm             |  4 ++--
 src/lib/Hydra/Controller/Root.pm            |  2 +-
 src/lib/Hydra/Helper/AddBuilds.pm           |  1 -
 src/lib/Hydra/Helper/CatalystUtils.pm       |  2 +-
 src/lib/Hydra/Helper/Nix.pm                 |  4 ++--
 src/root/all.tt                             |  2 +-
 src/root/build.tt                           | 12 ++++++------
 src/root/common.tt                          |  4 ++--
 src/script/hydra-build                      |  2 --
 src/script/hydra-queue-runner               |  2 +-
 src/sql/hydra.sql                           |  6 +++---
 15 files changed, 31 insertions(+), 30 deletions(-)

diff --git a/src/lib/Hydra/Base/Controller/ListBuilds.pm b/src/lib/Hydra/Base/Controller/ListBuilds.pm
index 73fc9c6a7..fe2a9770c 100644
--- a/src/lib/Hydra/Base/Controller/ListBuilds.pm
+++ b/src/lib/Hydra/Base/Controller/ListBuilds.pm
@@ -66,7 +66,7 @@ sub all : Chained('get_builds') PathPart {
 
     $c->stash->{builds} = [ $c->stash->{allBuilds}->search(
         { finished => 1 },
-        { order_by => "timestamp DESC"
+        { order_by => "stoptime DESC"
         , columns => [@buildListColumns]
         , rows => $resultsPerPage
         , page => $page }) ];
@@ -97,7 +97,7 @@ sub latest : Chained('get_builds') PathPart('latest') {
     my ($self, $c, @rest) = @_;
 
     my ($latest) = $c->stash->{allBuilds}->search(
-        {finished => 1, buildstatus => 0}, {order_by => ["isCurrent DESC", "timestamp DESC"]});
+        {finished => 1, buildstatus => 0}, {order_by => ["id DESC"]});
 
     notFound($c, "There is no successful build to redirect to.") unless defined $latest;
 
@@ -112,7 +112,7 @@ sub latest_for : Chained('get_builds') PathPart('latest-for') {
     notFound($c, "You need to specify a platform type in the URL.") unless defined $system;
 
     my ($latest) = $c->stash->{allBuilds}->search(
-        {finished => 1, buildstatus => 0, system => $system}, {order_by => ["isCurrent DESC", "timestamp DESC"]});
+        {finished => 1, buildstatus => 0, system => $system}, {order_by => ["id DESC"]});
 
     notFound($c, "There is no successful build for platform `$system' to redirect to.") unless defined $latest;
 
diff --git a/src/lib/Hydra/Controller/API.pm b/src/lib/Hydra/Controller/API.pm
index 36168fd29..04a6e54b9 100644
--- a/src/lib/Hydra/Controller/API.pm
+++ b/src/lib/Hydra/Controller/API.pm
@@ -90,7 +90,7 @@ sub latestbuilds : Chained('api') PathPart('latestbuilds') Args(0) {
     $filter->{job} = $job if !$job eq "";
     $filter->{system} = $system if !$system eq "";
 
-    my @latest = $c->model('DB::Builds')->search($filter, {rows => $nr, order_by => ["timestamp DESC"] });
+    my @latest = $c->model('DB::Builds')->search($filter, {rows => $nr, order_by => ["id DESC"] });
 
     my @list;
     push @list, buildToHash($_) foreach @latest;
@@ -142,7 +142,7 @@ sub queue : Chained('api') PathPart('queue') Args(0) {
     my $nr = $c->request->params->{nr};
     error($c, "Parameter not defined!") if !defined $nr;
 
-    my @builds = $c->model('DB::Builds')->search({finished => 0}, {rows => $nr, order_by => ["busy DESC", "priority DESC", "timestamp"]});
+    my @builds = $c->model('DB::Builds')->search({finished => 0}, {rows => $nr, order_by => ["busy DESC", "priority DESC", "id"]});
 
     my @list;
     push @list, buildToHash($_) foreach @builds;
diff --git a/src/lib/Hydra/Controller/Admin.pm b/src/lib/Hydra/Controller/Admin.pm
index 780835c29..73d884e2b 100644
--- a/src/lib/Hydra/Controller/Admin.pm
+++ b/src/lib/Hydra/Controller/Admin.pm
@@ -34,7 +34,8 @@ sub machines : Chained('admin') PathPart('machines') Args(0) {
 
 sub clear_queue_non_current : Chained('admin') PathPart('clear-queue-non-current') Args(0) {
     my ($self, $c) = @_;
-    $c->model('DB::Builds')->search({finished => 0, iscurrent => 0, busy => 0})->update({ finished => 1, buildstatus => 4, timestamp => time});
+    my $time = time();
+    $c->model('DB::Builds')->search({finished => 0, iscurrent => 0, busy => 0})->update({ finished => 1, buildstatus => 4, starttime => $time, stoptime => $time });
     $c->res->redirect($c->request->referer // "/admin");
 }
 
diff --git a/src/lib/Hydra/Controller/Build.pm b/src/lib/Hydra/Controller/Build.pm
index 4a05edd6e..0c5fa0bf0 100644
--- a/src/lib/Hydra/Controller/Build.pm
+++ b/src/lib/Hydra/Controller/Build.pm
@@ -216,7 +216,7 @@ sub download : Chained('build') PathPart {
     notFound($c, "Path $path is a directory.") if -d $path;
 
     $c->serve_static_file($path);
-    $c->response->headers->last_modified($c->stash->{build}->timestamp);
+    $c->response->headers->last_modified($c->stash->{build}->stoptime);
 }
 
 
@@ -423,9 +423,12 @@ sub cancel : Chained('build') PathPart Args(0) {
         # builds as well, but we would have to send a signal or
         # something to the build process.
 
+        my $time = time();
         $build->update(
-            { finished => 1, busy => 0, timestamp => time
+            { finished => 1, busy => 0
             , iscachedbuild => 0, buildstatus => 4 # = cancelled
+            , starttime => $time
+            , stoptime => $time
             });
     });
 
diff --git a/src/lib/Hydra/Controller/Job.pm b/src/lib/Hydra/Controller/Job.pm
index db191c336..e062e14a3 100644
--- a/src/lib/Hydra/Controller/Job.pm
+++ b/src/lib/Hydra/Controller/Job.pm
@@ -25,13 +25,13 @@ sub overview : Chained('job') PathPart('') Args(0) {
 
     $c->stash->{lastBuilds} =
         [ $c->stash->{job}->builds->search({ finished => 1 },
-            { order_by => 'timestamp DESC', rows => 10, columns => [@buildListColumns] }) ];
+            { order_by => 'id DESC', rows => 10, columns => [@buildListColumns] }) ];
 
     $c->stash->{queuedBuilds} = [
         $c->stash->{job}->builds->search(
             { finished => 0 },
             { join => ['project']
-            , order_by => ["priority DESC", "timestamp"]
+            , order_by => ["priority DESC", "id"]
             , '+select' => ['project.enabled']
             , '+as' => ['enabled']
             }
diff --git a/src/lib/Hydra/Controller/Root.pm b/src/lib/Hydra/Controller/Root.pm
index 6d5a10053..f49d27230 100644
--- a/src/lib/Hydra/Controller/Root.pm
+++ b/src/lib/Hydra/Controller/Root.pm
@@ -43,7 +43,7 @@ sub queue :Local {
     my ($self, $c) = @_;
     $c->stash->{template} = 'queue.tt';
     $c->stash->{queue} = [$c->model('DB::Builds')->search(
-        {finished => 0}, { join => ['project'], order_by => ["priority DESC", "timestamp"], columns => [@buildListColumns], '+select' => ['project.enabled'], '+as' => ['enabled'] })];
+        {finished => 0}, { join => ['project'], order_by => ["priority DESC", "id"], columns => [@buildListColumns], '+select' => ['project.enabled'], '+as' => ['enabled'] })];
     $c->stash->{flashMsg} //= $c->flash->{buildMsg};
 }
 
diff --git a/src/lib/Hydra/Helper/AddBuilds.pm b/src/lib/Hydra/Helper/AddBuilds.pm
index 531e1c1f2..cc682665b 100644
--- a/src/lib/Hydra/Helper/AddBuilds.pm
+++ b/src/lib/Hydra/Helper/AddBuilds.pm
@@ -1039,7 +1039,6 @@ sub restartBuild {
 
         $build->update(
             { finished => 0
-            , timestamp => time
             , busy => 0
             , locker => ""
             , iscachedbuild => 0
diff --git a/src/lib/Hydra/Helper/CatalystUtils.pm b/src/lib/Hydra/Helper/CatalystUtils.pm
index 1cd863291..cb3840b0a 100644
--- a/src/lib/Hydra/Helper/CatalystUtils.pm
+++ b/src/lib/Hydra/Helper/CatalystUtils.pm
@@ -28,7 +28,7 @@ our @EXPORT = qw(
 
 
 # Columns from the Builds table needed to render build lists.
-Readonly our @buildListColumns => ('id', 'finished', 'timestamp', 'project', 'jobset', 'job', 'nixname', 'system', 'priority', 'busy', 'buildstatus', 'releasename');
+Readonly our @buildListColumns => ('id', 'finished', 'timestamp', 'stoptime', 'project', 'jobset', 'job', 'nixname', 'system', 'priority', 'busy', 'buildstatus', 'releasename');
 
 
 sub getBuild {
diff --git a/src/lib/Hydra/Helper/Nix.pm b/src/lib/Hydra/Helper/Nix.pm
index e320d7352..ababdfd8e 100644
--- a/src/lib/Hydra/Helper/Nix.pm
+++ b/src/lib/Hydra/Helper/Nix.pm
@@ -100,7 +100,7 @@ sub allPrimaryBuilds {
     my ($project, $primaryJob) = @_;
     my $allPrimaryBuilds = $project->builds->search(
         { jobset => $primaryJob->get_column('jobset'), job => $primaryJob->get_column('job'), finished => 1 },
-        { order_by => "timestamp DESC"
+        { order_by => "id DESC"
         , where => \ attrsToSQL($primaryJob->attrs, "me.id")
         });
     return $allPrimaryBuilds;
@@ -153,7 +153,7 @@ sub findLastJobForBuilds {
         , job => $job->get_column('job'), finished => 1
         },
         { rows => 1
-        , order_by => ["buildstatus", "timestamp"]
+        , order_by => ["buildstatus", "id"]
         , where => \ attrsToSQL($job->attrs, "build.id")
         })
         unless defined $thisBuild;
diff --git a/src/root/all.tt b/src/root/all.tt
index de92fc21c..0d065a1ed 100644
--- a/src/root/all.tt
+++ b/src/root/all.tt
@@ -4,7 +4,7 @@
      project ? " for project $project.name" : "") %]
 [% PROCESS common.tt %]
 
-<p>Showing builds [% (page - 1) * resultsPerPage + 1 %] - [% (page - 1) * resultsPerPage + builds.size %] out of [% total %] in order of descending timestamp.</p>
+<p>Showing builds [% (page - 1) * resultsPerPage + 1 %] - [% (page - 1) * resultsPerPage + builds.size %] out of [% total %] in order of descending finish time.</p>
 
 [% INCLUDE renderBuildList hideProjectName=project hideJobsetName=jobset hideJobName=job %]
 [% INCLUDE renderPager %]
diff --git a/src/root/build.tt b/src/root/build.tt
index 8fdaea8aa..9da054fc7 100644
--- a/src/root/build.tt
+++ b/src/root/build.tt
@@ -150,8 +150,8 @@
             <tr>
               <th>Duration:</th>
               <td>[% actualBuild = build.iscachedbuild ? cachedBuild : build;
-                     INCLUDE renderDuration duration = actualBuild.stoptime - actualBuild.starttime %]; finished at [% INCLUDE renderDateTime timestamp = actualBuild.stoptime
-                  %]</td>
+                     INCLUDE renderDuration duration = actualBuild.stoptime - actualBuild.starttime %];
+                     finished at [% INCLUDE renderDateTime timestamp = actualBuild.stoptime %]</td>
             </tr>
             [% END %]
             [% IF log_exists(build.drvpath) %]
@@ -297,17 +297,17 @@
         <td><tt>[% INCLUDE renderOutputs outputs=build.buildoutputs %]</tt></td>
       </tr>
       <tr>
-        <th>Time added:</th>
+        <th>Queued:</th>
         <td>[% INCLUDE renderDateTime timestamp = build.timestamp %]</td>
       </tr>
-      [% IF build.finished && build.buildstatus != 4 %]
+      [% IF build.finished && !build.iscachedbuild %]
         <tr>
           <th>Build started:</th>
-          <td>[% IF build.starttime %][% INCLUDE renderDateTime timestamp = build.starttime %][% ELSE %]<em>(cached build)</em>[% END %]</td>
+          <td>[% INCLUDE renderDateTime timestamp = build.starttime %]</td>
         </tr>
         <tr>
           <th>Build finished:</th>
-          <td>[% IF build.stoptime %][% INCLUDE renderDateTime timestamp = build.stoptime %][% ELSE %]<em>(cached build)</em>[% END %]</td>
+          <td>[% INCLUDE renderDateTime timestamp = build.stoptime %]</td>
         </tr>
       [% END %]
       [% IF !build.finished %]
diff --git a/src/root/common.tt b/src/root/common.tt
index f2d4b6019..0fa0c2339 100644
--- a/src/root/common.tt
+++ b/src/root/common.tt
@@ -80,7 +80,7 @@ BLOCK renderBuildListHeader %]
         [% END %]
         <th>Release Name</th>
         <th>System</th>
-        <th>Timestamp</th>
+        <th>[% IF showSchedulingInfo %]Queued at[% ELSE %]Finished at[% END %]</th>
         [% IF showStatusChange %]
           <th class="headerSortUp">Last status change</th>
         [% END %]
@@ -113,7 +113,7 @@ BLOCK renderBuildListBody;
       [% END %]
       <td>[% !showSchedulingInfo and build.get_column('releasename') ? build.get_column('releasename') : build.nixname %]</td>
       <td><tt>[% build.system %]</tt></td>
-      <td>[% date.format(build.timestamp, '%Y-%m-%d %H:%M:%S') %]</td>
+      <td>[% date.format(showSchedulingInfo ? build.timestamp : build.stoptime, '%Y-%m-%d %H:%M:%S') %]</td>
       [% IF showStatusChange %]
         <td>
           [% IF build.get_column('statusChangeTime') %]
diff --git a/src/script/hydra-build b/src/script/hydra-build
index 1086efa5d..9768f5f7c 100755
--- a/src/script/hydra-build
+++ b/src/script/hydra-build
@@ -54,7 +54,6 @@ sub failDependents {
             $d->update(
                 { finished => 1
                 , logfile => ''
-                , timestamp => time # !!! Why change the timestamp?
                 , iscachedbuild => 0
                 , buildstatus => $drvPath eq $d->drvpath ? 1 : 2
                 , starttime => $time
@@ -307,7 +306,6 @@ sub doBuild {
             , busy => 0
             , locker => ''
             , logfile => ''
-            , timestamp => time # !!! Why change the timestamp?
             , iscachedbuild => $isCachedBuild
             , buildstatus => $buildStatus
             , starttime => $startTime
diff --git a/src/script/hydra-queue-runner b/src/script/hydra-queue-runner
index e5621f5d5..f876bce28 100755
--- a/src/script/hydra-queue-runner
+++ b/src/script/hydra-queue-runner
@@ -100,7 +100,7 @@ sub checkBuilds {
             # Select the highest-priority builds to start.
             my @builds = $extraAllowed == 0 ? () : $db->resultset('Builds')->search(
                 { finished => 0, busy => 0, system => $system->system, enabled => 1 },
-                { join => ['project'], order_by => ["priority DESC", "timestamp"],
+                { join => ['project'], order_by => ["priority DESC", "id"],
                   rows => $extraAllowed });
 
             if (scalar(@builds) > 0) {
diff --git a/src/sql/hydra.sql b/src/sql/hydra.sql
index 4e2521304..ec6177dc2 100644
--- a/src/sql/hydra.sql
+++ b/src/sql/hydra.sql
@@ -122,7 +122,7 @@ create table Builds (
 
     finished      integer not null, -- 0 = scheduled, 1 = finished
 
-    timestamp     integer not null, -- time this build was scheduled / finished building
+    timestamp     integer not null, -- time this build was added
 
     -- Info about the inputs.
     project       text not null,
@@ -158,8 +158,8 @@ create table Builds (
 
     logfile       text, -- if busy, the path of the logfile
 
-    startTime     integer, -- if busy, time we started
-    stopTime      integer,
+    startTime     integer, -- if busy/finished, time we started
+    stopTime      integer, -- if finished, time we finished
 
     -- Information about finished builds.
     isCachedBuild integer, -- boolean