[OSSTEST PATCH 0/7] host allocation: Performance improvements

[OSSTEST PATCH 0/7] host allocation: Performance improvements

[Ian Jackson]

Ian Jackson (7):
  ts-hosts-allocate-Executive: Fix broken call to $duration_estimator
  resource allocation: Provide OSSTEST_ALLOC_FAKE_PLAN test facility
  ts-hosts-allocate-Executive: Do a pre-check
  duration estimates: Memoise results
  host allocation: Memoise duration estimates
  host allocation: Memoise $equivstatus query results
  schema: Provide index on flights by start time

 Osstest/Executive.pm             | 40 +++++++++++++++++++++++++++++++++++++---
 schema/flights-started-index.sql |  7 +++++++
 ts-hosts-allocate-Executive      | 26 ++++++++++++++++++--------
 3 files changed, 62 insertions(+), 11 deletions(-)
 create mode 100644 schema/flights-started-index.sql

-- 
2.11.0

[OSSTEST PATCH 1/7] ts-hosts-allocate-Executive: Fix broken call to $duration_estimator

[Ian Jackson]

The debug subref is passed to the constructor (and indeed we do that).
The final argument to the actual estimator is $uptoincl_testid (but we
didn't say $will_uptoincl_testid, so it is ignored).

The code was wrong, but with no effect.  So no functional change.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 ts-hosts-allocate-Executive | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/ts-hosts-allocate-Executive b/ts-hosts-allocate-Executive
index 3bc38102..e5a6fbfe 100755
--- a/ts-hosts-allocate-Executive
+++ b/ts-hosts-allocate-Executive
@@ -516,10 +516,7 @@ sub find_recent_duration ($$) {
     ($candrow->{Duration},
      $candrow->{MostRecentStarted},
      $candrow->{MostRecentStatus}) =
-        $duration_estimator->($job, $hid->{Ident}, $candrow->{resname},
-                              sub {
-#                                  print DEBUG "$dbg DUR-EST @_\n";
-                              });
+        $duration_estimator->($job, $hid->{Ident}, $candrow->{resname});
 }
 
 
-- 
2.11.0

[OSSTEST PATCH 2/7] resource allocation: Provide OSSTEST_ALLOC_FAKE_PLAN test facility

[Ian Jackson]

Set this variable (to a data-plan.final.pl, say) and it becomes
possible to test host allocation programs without actually allocating
anything and without engaging with the queue system.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 Osstest/Executive.pm | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index 0808202b..d6b2736b 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -767,6 +767,32 @@ sub alloc_resources {
 	? sub { print $debugfh @_, "\n" or die $!; }
         : sub { };
 
+    my $fake_plan = $ENV{OSSTEST_ALLOC_FAKE_PLAN};
+    if (defined $fake_plan) {
+	my $fake_data = do {
+	    local $/ = undef;
+	    open FAKEPLAN, "<", $ENV{OSSTEST_ALLOC_FAKE_PLAN} or die $!;
+	    my $r = <FAKEPLAN> // die $!;
+	    close FAKEPLAN;
+	    $r;
+	};
+	if ($fake_plan =~ m{\.pl$}) {
+	    $fake_data = eval $fake_data;
+	} elsif ($fake_plan =~ m{\.json$}) {
+	    $fake_data = from_json($fake_data);
+	} else {
+	    die;
+	}
+	db_retry($flight,'running', $dbh_tests, [], sub {
+            logm("fake resourcecall..");
+	    my ($ok, $bookinglist) = $resourcecall->($fake_data, 1);
+            logm("fake resourcecall ok=$ok");
+	    $dbh_tests->rollback();
+	    exit $ok;
+        });
+	die "unexpectedly left db_retry";
+    }
+
     my $set_info= sub {
         return if grep { !defined } @_;
         my @s;
-- 
2.11.0

[OSSTEST PATCH 3/7] ts-hosts-allocate-Executive: Do a pre-check

[Ian Jackson]

Call attempt_allocation with an empty plan and $mayalloc=0.

In the usual case this will arrange to prime our memoisation caches
before we get involved with the queueing system.

It will also arrange for various errors to be reported sooner.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 ts-hosts-allocate-Executive | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/ts-hosts-allocate-Executive b/ts-hosts-allocate-Executive
index e5a6fbfe..4140b65c 100755
--- a/ts-hosts-allocate-Executive
+++ b/ts-hosts-allocate-Executive
@@ -692,6 +692,13 @@ sub alloc_hosts () {
         return;
     }
 
+    {
+	logm("pre-checking resources...");
+	local $Osstest::TestSupport::logm_prefix = $logm_prefix.' (precheck)';
+	my ($ok, $bookinglist) = attempt_allocation({}, 0);
+	die $ok if $ok>1 && $ok != $alloc_starved_r;
+    }
+
     my $waitstartadjust=
         $jobinfo->{recipe} =~ m/build/
         ? -10000
-- 
2.11.0

[OSSTEST PATCH 4/7] duration estimates: Memoise results

[Ian Jackson]

The caller may provide a memoisation hash.  If they don't we embed
one in the estimator.

The estimator contains a db statement handle so shouldn't be so
long-lived that this gives significantly wrong answers.

I am aiming this work at ts-hosts-allocate-Executive, but it is
possible that this might speed up sg-report-flight.

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

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index d6b2736b..50c84cc3 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -1164,8 +1164,8 @@ sub hostalloc_starvation_calculate_X ($$$) {
 
 #---------- duration estimator ----------
 
-sub duration_estimator ($$;$$) {
-    my ($branch, $blessing, $debug, $will_uptoincl_testid) = @_;
+sub duration_estimator ($$;$$$) {
+    my ($branch, $blessing, $debug, $will_uptoincl_testid, $our_memo) = @_;
     # returns a function which you call like this
     #    $durest->($job, $hostidname, $onhost [, $uptoincl_testid])
     # and returns one of
@@ -1269,9 +1269,15 @@ END
     my $recentflights_q= $prepare_combi->($recentflights_qtxt);
     my $duration_anyref_q= $prepare_combi->($duration_anyref_qtxt);
 
+    $our_memo //= { };
+
     return sub {
         my ($job, $hostidname, $onhost, $uptoincl_testid) = @_;
 
+	my $memokey = "$job $hostidname $onhost $uptoincl_testid";
+	my $memo = $our_memo->{$memokey};
+	return @$memo if $memo;
+
 	my @x_params;
 	push @x_params, $uptoincl_testid if $will_uptoincl_testid;
 
@@ -1319,7 +1325,9 @@ END
             }
         }
 
-        return ($duration_max, $refs->[0]{started}, $refs->[0]{status});
+	$memo = [$duration_max, $refs->[0]{started}, $refs->[0]{status}];
+	$our_memo->{$memokey} = $memo;
+        return @$memo;
     };
 }
 
-- 
2.11.0

[OSSTEST PATCH 5/7] host allocation: Memoise duration estimates

[Ian Jackson]

We look at our own branch to estimate durations.  If somehow we are
one of multiple concurrent flights on this branch with the appropriate
blessing, we don't mind not noticing the doing of our peer flights so
that if our estimates are a bit out of date.

So it is fine to use an estimate no older than our own runtime.

Right now we generate a new duration estimator during each queueing
round, because it contains a statement handle and we must disconnect
from the db while waiting.  So the internal memo table gets thrown
away each time and is useless.

To actually memoise, pass our own hash which lives as long as we do.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 Osstest/Executive.pm        | 2 +-
 ts-hosts-allocate-Executive | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index 50c84cc3..61a99bc3 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -1274,7 +1274,7 @@ END
     return sub {
         my ($job, $hostidname, $onhost, $uptoincl_testid) = @_;
 
-	my $memokey = "$job $hostidname $onhost $uptoincl_testid";
+	my $memokey = "$job $hostidname $onhost ".($uptoincl_testid//"");
 	my $memo = $our_memo->{$memokey};
 	return @$memo if $memo;
 
diff --git a/ts-hosts-allocate-Executive b/ts-hosts-allocate-Executive
index 4140b65c..39c66346 100755
--- a/ts-hosts-allocate-Executive
+++ b/ts-hosts-allocate-Executive
@@ -145,8 +145,10 @@ END
 		      AND hostflag LIKE 'equiv-%'
 END
 
+    our %duration_memo;
     $duration_estimator= duration_estimator($fi->{branch}, $blessing,
-                                            sub { print DEBUG "@_\n"; });
+                                            sub { print DEBUG "@_\n"; },
+					    0, \%duration_memo);
 
     $resprop_q= $dbh_tests->prepare(<<END);
             SELECT * FROM resource_properties
-- 
2.11.0

[OSSTEST PATCH 6/7] host allocation: Memoise $equivstatus query results

[Ian Jackson]

This provides a very significant speedup.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 ts-hosts-allocate-Executive | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/ts-hosts-allocate-Executive b/ts-hosts-allocate-Executive
index 39c66346..a47bc499 100755
--- a/ts-hosts-allocate-Executive
+++ b/ts-hosts-allocate-Executive
@@ -467,9 +467,13 @@ END
         find_recent_duration($dbg,$hid,$candrow);
 
 	if ($candrow->{restype} eq 'host') {
-	    $equivstatusq->execute($job,$blessing,$fi->{branch},
-				   $hid->{Ident},$candrow->{resname});
-	    my $esrow = $equivstatusq->fetchrow_hashref();
+	    our %equivstatus_memo;
+	    my @params = ($job,$blessing,$fi->{branch},
+			  $hid->{Ident},$candrow->{resname});
+	    my $esrow = $equivstatus_memo{"@params"} //= do {
+		$equivstatusq->execute(@params);
+		$equivstatusq->fetchrow_hashref() // { };
+	    };
 	    $candrow->{EquivMostRecentStatus} = $esrow->{status};
 	    print DEBUG "$dbg EQUIV-MOST-RECENT ";
 	    print DEBUG ("$esrow->{flight}.$esrow->{job}".
-- 
2.11.0

[OSSTEST PATCH 7/7] schema: Provide index on flights by start time

[Ian Jackson]

We often use flight number as a proxy for ordering, but this is not
always appropriate and not always done (and sometimes it's a bit of a
bodge).

Provide an index to find flights by start time.  This significantly
speeds up the host allocation $equivstatusq query, and the duration
estimator.

(I have tested this by creating a trial index in the production
database.  That index can be dropped again, preferably after this
commit makes it to production.)

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 schema/flights-started-index.sql | 7 +++++++
 1 file changed, 7 insertions(+)
 create mode 100644 schema/flights-started-index.sql

diff --git a/schema/flights-started-index.sql b/schema/flights-started-index.sql
new file mode 100644
index 00000000..c230d9d8
--- /dev/null
+++ b/schema/flights-started-index.sql
@@ -0,0 +1,7 @@
+-- ##OSSTEST## 011 Harmless
+--
+-- This index helps ts-hosts-allocate-Executive find recent instances
+-- of the same job.  It may be useful for other things too.
+
+CREATE INDEX flights_blessing_started_idx
+    ON flights (blessing, started);
-- 
2.11.0