* [OSSTEST PATCH v2 00/41] Performance work
@ 2020-07-31 11:37 Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 01/41] Add cperl-indent-level to .dir-locals.el Ian Jackson
` (40 more replies)
0 siblings, 41 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:37 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, George Dunlap
This is a combination of two series and some new work:
* [OSSTEST PATCH 00/14] Flight report performance improvements
* [OSSTEST PATCH 00/11] Improve performance of sg-report-host-history
* New work to improve the performance of cs-bisection-step
* Fixes to usage of SQL LIKE
Thanks to George for reviews of some of the most critical patches to
the regression analyser in sg-report-flight. Where necessary I have
rebased those reviewed patches over the SQL LIKE fixes, and in those
cases I retained the reviewed-by. I hope that's OK.
Outstanding in my perf programme is sg-report-job-history.
Ian Jackson (41):
Add cperl-indent-level to .dir-locals.el
SQL: Use "LIKE" rather than "like", etc.
SQL: Fix incorrect LIKE pattern syntax (literals)
SQL: Fix incorrect LIKE pattern syntax (program variables)
sg-report-flight: Add a comment re same-flight search narrowing
sg-report-flight: Sort failures by job name as last resort
schema: Provide indices for sg-report-flight
sg-report-flight: Ask the db for flights of interest
sg-report-flight: Use WITH to use best index use for $flightsq
sg-report-flight: Use WITH clause to use index for $anypassq
sg-report-flight: Use the job row from the intitial query
Executive: Use index for report__find_test
duration_estimator: Ignore truncated jobs unless we know the step
duration_estimator: Introduce some _qtxt variables
duration_estimator: Explicitly provide null in general host q
duration_estimator: Return job column in first query
duration_estimator: Move $uptincl_testid to separate @x_params
duration_estimator: Move duration query loop into database
Executive: Drop redundant AND clause
schema: Add index for quick lookup by host
sg-report-host-history: Find flight limit by flight start date
sg-report-host-history: Drop per-job debug etc.
Executive: Export opendb_tests
sg-report-host-history: Add a debug print after sorting jobs
sg-report-host-history: Do the main query per host
sg-report-host-history: Rerganisation: Make mainquery per-host
sg-report-host-history: Rerganisation: Read old logs later
sg-report-host-history: Rerganisation: Change loops
sg-report-host-history: Drop a redundznt AND clause
sg-report-host-history: Fork
schema: Add index to help cs-bisection-step
adhoc-revtuple-generator: Fix an undef warning in a debug print
cs-bisection-step: Generalise qtxt_common_rev_ok
cs-bisection-step: Move an AND
cs-bisection-step: Break out qtxt_common_ok
cs-bisection-step: Use db_prepare a few times instead of ->do
cs-bisection-step: temporary table: Insert only rows we care about
SQL: Change LIKE E'...\\_...' to LIKE '...\_...'
cs-bisection-step: Add a debug print when we run dot(1)
cs-bisection-step: Lay out the revision tuple graph once
duration_estimator: Clarify recentflights query a bit
.dir-locals.el | 3 +-
Osstest.pm | 8 +-
Osstest/Executive.pm | 79 +++++++++------
Osstest/JobDB/Executive.pm | 2 +-
adhoc-revtuple-generator | 2 +-
cr-ensure-disk-space | 4 +-
cs-adjust-flight | 2 +-
cs-bisection-step | 51 +++++++---
mg-force-push | 2 +-
mg-report-host-usage-collect | 10 +-
ms-planner | 2 +-
schema/runvars-built-index.sql | 7 ++
schema/runvars-host-index.sql | 8 ++
schema/runvars-revision-index.sql | 7 ++
schema/steps-broken-index.sql | 7 ++
schema/steps-job-index.sql | 7 ++
sg-report-flight | 129 ++++++++++++++++++++----
sg-report-host-history | 161 +++++++++++++++++-------------
sg-report-job-history | 4 +-
ts-logs-capture | 2 +-
20 files changed, 344 insertions(+), 153 deletions(-)
create mode 100644 schema/runvars-built-index.sql
create mode 100644 schema/runvars-host-index.sql
create mode 100644 schema/runvars-revision-index.sql
create mode 100644 schema/steps-broken-index.sql
create mode 100644 schema/steps-job-index.sql
--
2.20.1
^ permalink raw reply [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 01/41] Add cperl-indent-level to .dir-locals.el
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
@ 2020-07-31 11:37 ` Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 02/41] SQL: Use "LIKE" rather than "like", etc Ian Jackson
` (39 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:37 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
My personal config on my laptop has this set to 2 and that makes
editing osstest, which uses 4, quite annoying.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
New in v2.
---
.dir-locals.el | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/.dir-locals.el b/.dir-locals.el
index d87916f7..ad4fa3dc 100644
--- a/.dir-locals.el
+++ b/.dir-locals.el
@@ -1 +1,2 @@
-((nil . ((indent-tabs-mode . t))))
+((nil . ((indent-tabs-mode . t)))
+ (cperl-mode . ((cperl-indent-level . 4))))
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 02/41] SQL: Use "LIKE" rather than "like", etc.
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 01/41] Add cperl-indent-level to .dir-locals.el Ian Jackson
@ 2020-07-31 11:37 ` Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 03/41] SQL: Fix incorrect LIKE pattern syntax (literals) Ian Jackson
` (38 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:37 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
This is more like the rest of the style. It will also make it easier
to find instances of the mistaken LIKE syntax.
I found these with "git grep" and manually edited them. I have
checked the before-and-after result of
find * -type f | xargs perl -i~ -pe 's/\bLIKE\b/like/g'
and it has only the few expected changes to ANDs and ORs.
No functional change.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
New in v2.
---
cr-ensure-disk-space | 4 ++--
cs-adjust-flight | 2 +-
mg-force-push | 2 +-
mg-report-host-usage-collect | 10 +++++-----
ms-planner | 2 +-
sg-report-flight | 2 +-
sg-report-host-history | 4 ++--
7 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/cr-ensure-disk-space b/cr-ensure-disk-space
index 3e0288f9..11d801b0 100755
--- a/cr-ensure-disk-space
+++ b/cr-ensure-disk-space
@@ -99,8 +99,8 @@ my $chkq= db_prepare("SELECT * FROM flights WHERE flight=?");
my $refq= db_prepare(<<END);
SELECT flight, val
FROM runvars
- WHERE name like '%job'
- AND val like '%.%'
+ WHERE name LIKE '%job'
+ AND val LIKE '%.%'
AND flight >= ?
END
diff --git a/cs-adjust-flight b/cs-adjust-flight
index 98d40891..d04a2fd7 100755
--- a/cs-adjust-flight
+++ b/cs-adjust-flight
@@ -526,7 +526,7 @@ sub change__repro_buildjobs {
}
}
my $testq = db_prepare(<<END);
-SELECT name, val FROM runvars WHERE flight=? AND job=? AND name like '%job';
+SELECT name, val FROM runvars WHERE flight=? AND job=? AND name LIKE '%job';
END
my $buildq_txt = <<END;
SELECT name FROM runvars WHERE flight=? AND job=? AND ('f'
diff --git a/mg-force-push b/mg-force-push
index 1066a300..001e0c47 100755
--- a/mg-force-push
+++ b/mg-force-push
@@ -54,7 +54,7 @@ END
FROM rv url
JOIN rv built
ON url.job = built.job
- AND url.name like 'tree_%'
+ AND url.name LIKE 'tree_%'
AND built.name = 'built_revision_' || substring(url.name, 6)
WHERE url.val = ?
END
diff --git a/mg-report-host-usage-collect b/mg-report-host-usage-collect
index 160d295f..3fab490a 100755
--- a/mg-report-host-usage-collect
+++ b/mg-report-host-usage-collect
@@ -154,10 +154,10 @@ END
SELECT finished prep_finished,
status prep_status
FROM steps prep
- WHERE flight=? and job=?
+ WHERE flight=? AND job=?
AND prep.finished IS NOT NULL
AND (prep.step='ts-host-build-prep'
- OR prep.step like 'ts-host-install%')
+ OR prep.step LIKE 'ts-host-install%')
ORDER BY stepno DESC
LIMIT 1
END
@@ -165,14 +165,14 @@ END
my $hostsq = db_prepare(<<END);
SELECT val, synth
FROM runvars
- WHERE flight=? and job=?
- AND (name like '%_host' or name='host')
+ WHERE flight=? AND job=?
+ AND (name LIKE '%_host' OR name='host')
END
my $finishq = db_prepare(<<END);
SELECT max(finished) AS finished
FROM steps
- WHERE flight=? and job=?
+ WHERE flight=? AND job=?
END
progress1 "minflight $minflight executing...";
diff --git a/ms-planner b/ms-planner
index c70b46b0..11423404 100755
--- a/ms-planner
+++ b/ms-planner
@@ -72,7 +72,7 @@ sub allocations ($$) {
ON owntaskid = taskid
WHERE NOT (tasks.type='magic' AND
tasks.refkey='allocatable')
- AND NOT (resources.restype like 'share-%'
+ AND NOT (resources.restype LIKE 'share-%'
AND NOT EXISTS (
SELECT 1 FROM resource_sharing sh
WHERE sh.restype = substring(resources.restype from 7)
diff --git a/sg-report-flight b/sg-report-flight
index 6c481f6f..0edb6e1a 100755
--- a/sg-report-flight
+++ b/sg-report-flight
@@ -513,7 +513,7 @@ END
my $revh= db_prepare(<<END);
SELECT * FROM runvars
WHERE flight=$flight AND job='$j->{job}'
- AND name like 'built_revision_%'
+ AND name LIKE 'built_revision_%'
ORDER BY name
END
# We report in jobtext revisions in non-main-revision jobs, too.
diff --git a/sg-report-host-history b/sg-report-host-history
index 54738e68..c22a1704 100755
--- a/sg-report-host-history
+++ b/sg-report-host-history
@@ -37,7 +37,7 @@ our @blessings;
open DEBUG, ">/dev/null";
-my $namecond= "(name = 'host' or name like '%_host')";
+my $namecond= "(name = 'host' OR name LIKE '%_host')";
csreadconfig();
while (@ARGV && $ARGV[0] =~ m/^-/) {
@@ -456,7 +456,7 @@ foreach my $host (@ARGV) {
SELECT DISTINCT val
FROM runvars
WHERE flight=?
- AND (name = 'host' or name like '%_host')
+ AND (name = 'host' OR name LIKE '%_host')
END
$hostsinflightq->execute($flight);
while (my $row = $hostsinflightq->fetchrow_hashref()) {
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 03/41] SQL: Fix incorrect LIKE pattern syntax (literals)
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 01/41] Add cperl-indent-level to .dir-locals.el Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 02/41] SQL: Use "LIKE" rather than "like", etc Ian Jackson
@ 2020-07-31 11:37 ` Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 04/41] SQL: Fix incorrect LIKE pattern syntax (program variables) Ian Jackson
` (37 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:37 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
LIKE takes a weird SQLish glob pattern, where % is like a glob *
and (relevantly, here) _ is like a glob ?.
Every _ in one of these LIKE patterns needs to be escaped with \.
Do that for all the literal LIKE patterns.
This fixes bugs. Generally, bugs where the wrong rows might be
returned (except that the data probably doesn't have any such rows).
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
New in v2.
---
mg-force-push | 2 +-
mg-report-host-usage-collect | 2 +-
sg-report-flight | 2 +-
sg-report-host-history | 6 +++---
ts-logs-capture | 2 +-
5 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/mg-force-push b/mg-force-push
index 001e0c47..3a701a11 100755
--- a/mg-force-push
+++ b/mg-force-push
@@ -54,7 +54,7 @@ END
FROM rv url
JOIN rv built
ON url.job = built.job
- AND url.name LIKE 'tree_%'
+ AND url.name LIKE 'tree\_%'
AND built.name = 'built_revision_' || substring(url.name, 6)
WHERE url.val = ?
END
diff --git a/mg-report-host-usage-collect b/mg-report-host-usage-collect
index 3fab490a..1944c8d7 100755
--- a/mg-report-host-usage-collect
+++ b/mg-report-host-usage-collect
@@ -166,7 +166,7 @@ END
SELECT val, synth
FROM runvars
WHERE flight=? AND job=?
- AND (name LIKE '%_host' OR name='host')
+ AND (name LIKE '%\_host' OR name='host')
END
my $finishq = db_prepare(<<END);
diff --git a/sg-report-flight b/sg-report-flight
index 0edb6e1a..831917a9 100755
--- a/sg-report-flight
+++ b/sg-report-flight
@@ -513,7 +513,7 @@ END
my $revh= db_prepare(<<END);
SELECT * FROM runvars
WHERE flight=$flight AND job='$j->{job}'
- AND name LIKE 'built_revision_%'
+ AND name LIKE 'built\_revision\_%'
ORDER BY name
END
# We report in jobtext revisions in non-main-revision jobs, too.
diff --git a/sg-report-host-history b/sg-report-host-history
index c22a1704..7505b18b 100755
--- a/sg-report-host-history
+++ b/sg-report-host-history
@@ -37,7 +37,7 @@ our @blessings;
open DEBUG, ">/dev/null";
-my $namecond= "(name = 'host' OR name LIKE '%_host')";
+my $namecond= "(name = 'host' OR name LIKE '%\_host')";
csreadconfig();
while (@ARGV && $ARGV[0] =~ m/^-/) {
@@ -256,7 +256,7 @@ END
FROM runvars
WHERE flight=? AND job=?
AND (
- name LIKE (? || '_power_%')
+ name LIKE (? || '\_power\_%')
)
END
@@ -456,7 +456,7 @@ foreach my $host (@ARGV) {
SELECT DISTINCT val
FROM runvars
WHERE flight=?
- AND (name = 'host' OR name LIKE '%_host')
+ AND (name = 'host' OR name LIKE '%\_host')
END
$hostsinflightq->execute($flight);
while (my $row = $hostsinflightq->fetchrow_hashref()) {
diff --git a/ts-logs-capture b/ts-logs-capture
index d75a2fda..62c281b8 100755
--- a/ts-logs-capture
+++ b/ts-logs-capture
@@ -44,7 +44,7 @@ our (@allguests, @guests);
sub find_guests () {
my $sth= $dbh_tests->prepare(<<END);
SELECT name FROM runvars WHERE flight=? AND job=?
- AND name LIKE '%_domname'
+ AND name LIKE '%\_domname'
ORDER BY name
END
$sth->execute($flight, $job);
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 04/41] SQL: Fix incorrect LIKE pattern syntax (program variables)
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (2 preceding siblings ...)
2020-07-31 11:37 ` [OSSTEST PATCH v2 03/41] SQL: Fix incorrect LIKE pattern syntax (literals) Ian Jackson
@ 2020-07-31 11:37 ` Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 05/41] sg-report-flight: Add a comment re same-flight search narrowing Ian Jackson
` (36 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:37 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
In two places the pattern for LIKE is constructed programmatically.
In this case, too, we need to escape % and _.
We pass the actual pattern (or pattern fragment) via ?, so we do not
need to worry about '.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
New in v2.
---
Osstest.pm | 8 +++++++-
Osstest/JobDB/Executive.pm | 2 +-
sg-report-host-history | 3 ++-
3 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/Osstest.pm b/Osstest.pm
index 63dddd95..b2b6b741 100644
--- a/Osstest.pm
+++ b/Osstest.pm
@@ -39,7 +39,7 @@ BEGIN {
main_revision_job_cond other_revision_job_suffix
$dbh_tests db_retry db_retry_retry db_retry_abort
db_readonly_report
- db_begin_work db_prepare
+ db_begin_work db_prepare db_quote_like_pattern
get_harness_rev blessing_must_not_modify_host
ensuredir get_filecontents_core_quiet system_checked
nonempty visible_undef show_abs_time
@@ -358,6 +358,12 @@ sub postfork () {
$mjobdb->jobdb_postfork();
}
+sub db_quote_like_pattern ($) {
+ local ($_) = @_;
+ s{[_%\\]}{\\$&}g;
+ $_;
+}
+
#---------- script entrypoints ----------
sub csreadconfig () {
diff --git a/Osstest/JobDB/Executive.pm b/Osstest/JobDB/Executive.pm
index be5588fc..39deb8a2 100644
--- a/Osstest/JobDB/Executive.pm
+++ b/Osstest/JobDB/Executive.pm
@@ -143,7 +143,7 @@ sub _check_testdbs ($) {
AND live
AND username LIKE (? || '@%')
END
- $sth->execute($c{Username});
+ $sth->execute(db_quote_like_pattern($c{Username}));
my $allok = 1;
while (my $row = $sth->fetchrow_hashref()) {
next if $row->{dbname} =~ m/^$re$/o;
diff --git a/sg-report-host-history b/sg-report-host-history
index 7505b18b..9730ae7a 100755
--- a/sg-report-host-history
+++ b/sg-report-host-history
@@ -380,7 +380,8 @@ END
$runvarq_hits++;
} else {
$runvarq_misses++;
- $jrunvarq->execute($jr->{flight}, $jr->{job}, $ident);
+ $jrunvarq->execute($jr->{flight}, $jr->{job},
+ db_quote_like_pattern($ident));
my %runvars;
while (my ($n, $v) = $jrunvarq->fetchrow_array()) {
$runvars{$n} = $v;
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 05/41] sg-report-flight: Add a comment re same-flight search narrowing
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (3 preceding siblings ...)
2020-07-31 11:37 ` [OSSTEST PATCH v2 04/41] SQL: Fix incorrect LIKE pattern syntax (program variables) Ian Jackson
@ 2020-07-31 11:37 ` Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 06/41] sg-report-flight: Sort failures by job name as last resort Ian Jackson
` (35 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:37 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
In afe851ca1771e5da6395b596afa69e509dbbc278
sg-report-flight: When justifying, disregard out-of-flight build jobs
we narrowed sg-report-flight's search algorith.
An extensive justification is in the commit message. I think much of
this information belongs in-tree, so c&p it (with slight edits) here.
No code change.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
sg-report-flight | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/sg-report-flight b/sg-report-flight
index 831917a9..fc439495 100755
--- a/sg-report-flight
+++ b/sg-report-flight
@@ -242,9 +242,27 @@ END
# jobs. We start with all jobs in $tflight, and for each job
# we also process any other jobs it refers to in *buildjob runvars.
#
+ # The real thing we want to check that the build jobs *in the
+ # same flight as the justifying job* used the right revisions.
+ # Build jobs from other flights were either (i) build jobs for
+ # components not being targed for testing by this branch, but
+ # which were necessary for the justifying job and for which we
+ # decided to reuse another build job (in which case we don't
+ # really care what versions they used, even if underlying it
+ # all there might be a different version of a tree we are
+ # actually interested in (ii) the kind of continuous update
+ # thing seen with freebsdbuildjob.
+ #
+ # (This is rather different to cs-bisection-step, which is
+ # less focused on changes in a particular set of trees.)
+ #
+ # So we limit the scope of our recursive descent into build
+ # jobs, to jobs in the same flight.
+ #
# We don't actually use a recursive algorithm because that
# would involve recursive use of the same sql query object;
# hence the @binfos_todo queue.
+
my @binfos_todo;
my $binfos_queue = sub {
my ($inflight,$q,$why) = @_;
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 06/41] sg-report-flight: Sort failures by job name as last resort
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (4 preceding siblings ...)
2020-07-31 11:37 ` [OSSTEST PATCH v2 05/41] sg-report-flight: Add a comment re same-flight search narrowing Ian Jackson
@ 2020-07-31 11:37 ` Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 07/41] schema: Provide indices for sg-report-flight Ian Jackson
` (34 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:37 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
This removes some nondeterminism from the output.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
sg-report-flight | 1 +
1 file changed, 1 insertion(+)
diff --git a/sg-report-flight b/sg-report-flight
index fc439495..7f2790ce 100755
--- a/sg-report-flight
+++ b/sg-report-flight
@@ -813,6 +813,7 @@ END
# they finished in the same second, we pick the lower-numbered
# step, which is the earlier one (if they are sequential at
# all).
+ or $a->{Job} cmp $b->{Job}
}
@failures;
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 07/41] schema: Provide indices for sg-report-flight
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (5 preceding siblings ...)
2020-07-31 11:37 ` [OSSTEST PATCH v2 06/41] sg-report-flight: Sort failures by job name as last resort Ian Jackson
@ 2020-07-31 11:37 ` Ian Jackson
2020-07-31 14:21 ` George Dunlap
2020-07-31 11:37 ` [OSSTEST PATCH v2 08/41] sg-report-flight: Ask the db for flights of interest Ian Jackson
` (33 subsequent siblings)
40 siblings, 1 reply; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:37 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, George Dunlap
These indexes allow very fast lookup of "relevant" flights eg when
trying to justify failures.
In my ad-hoc test case, these indices (along with the subsequent
changes to sg-report-flight and Executive.pm, reduce the runtime of
sg-report-flight from 2-3ks (unacceptably long!) to as little as
5-7s seconds - a speedup of about 500x.
(Getting the database snapshot may take a while first, but deploying
this code should help with that too by reducing long-running
transactions. Quoted perf timings are from snapshot acquisition.)
Without these new indexes there may be a performance change from the
query changes. I haven't benchmarked this so I am setting the schema
updates to be Preparatory/Needed (ie, "Schema first" as
schema/README.updates has it), to say that the index should be created
before the new code is deployed.
Testing: I have tested this series by creating experimental indices
"trial_..." in the actual production instance. (Transactional DDL was
very helpful with this.) I have verified with \d that schema update
instructions in this commit generate indexes which are equivalent to
the trial indices.
Deployment: AFter these schema updates are applied, the trial indices
are redundant duplicates and should be deleted.
CC: George Dunlap <George.Dunlap@citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
v2: Use proper \ escaping for underscores in LIKE
---
schema/runvars-built-index.sql | 7 +++++++
schema/runvars-revision-index.sql | 7 +++++++
schema/steps-job-index.sql | 7 +++++++
3 files changed, 21 insertions(+)
create mode 100644 schema/runvars-built-index.sql
create mode 100644 schema/runvars-revision-index.sql
create mode 100644 schema/steps-job-index.sql
diff --git a/schema/runvars-built-index.sql b/schema/runvars-built-index.sql
new file mode 100644
index 00000000..7108e0af
--- /dev/null
+++ b/schema/runvars-built-index.sql
@@ -0,0 +1,7 @@
+-- ##OSSTEST## 007 Preparatory
+--
+-- This index helps sg-report-flight find relevant flights.
+
+CREATE INDEX runvars_built_revision_idx
+ ON runvars (val)
+ WHERE name LIKE 'built\_revision\_%';
diff --git a/schema/runvars-revision-index.sql b/schema/runvars-revision-index.sql
new file mode 100644
index 00000000..8871b528
--- /dev/null
+++ b/schema/runvars-revision-index.sql
@@ -0,0 +1,7 @@
+-- ##OSSTEST## 008 Preparatory
+--
+-- This index helps Executive::report__find_test find relevant flights.
+
+CREATE INDEX runvars_revision_idx
+ ON runvars (val)
+ WHERE name LIKE 'revision\_%';
diff --git a/schema/steps-job-index.sql b/schema/steps-job-index.sql
new file mode 100644
index 00000000..07dc5a30
--- /dev/null
+++ b/schema/steps-job-index.sql
@@ -0,0 +1,7 @@
+-- ##OSSTEST## 006 Preparatory
+--
+-- This index helps sg-report-flight find if a test ever passed.
+
+CREATE INDEX steps_job_testid_status_idx
+ ON steps (job, testid, status);
+
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 08/41] sg-report-flight: Ask the db for flights of interest
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (6 preceding siblings ...)
2020-07-31 11:37 ` [OSSTEST PATCH v2 07/41] schema: Provide indices for sg-report-flight Ian Jackson
@ 2020-07-31 11:37 ` Ian Jackson
2020-07-31 14:17 ` George Dunlap
2020-07-31 11:37 ` [OSSTEST PATCH v2 09/41] sg-report-flight: Use WITH to use best index use for $flightsq Ian Jackson
` (32 subsequent siblings)
40 siblings, 1 reply; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:37 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, George Dunlap
Specifically, we narrow the initial query to flights which have at
least some job with the built_revision_foo we are looking for.
This condition is strictly broader than that implemented inside the
flight search loop, so there is no functional change.
Perf: runtime of my test case now ~300s-500s.
Example query before (from the Perl DBI trace):
SELECT * FROM (
SELECT flight, blessing FROM flights
WHERE (branch='xen-unstable')
AND EXISTS (SELECT 1
FROM jobs
WHERE jobs.flight = flights.flight
AND jobs.job = ?)
AND ( (TRUE AND flight <= 151903) AND (blessing='real') )
ORDER BY flight DESC
LIMIT 1000
) AS sub
ORDER BY blessing ASC, flight DESC
With these bind variables:
"test-armhf-armhf-libvirt"
After:
SELECT * FROM (
SELECT DISTINCT flight, blessing
FROM flights
JOIN runvars r1 USING (flight)
WHERE (branch='xen-unstable')
AND ( (TRUE AND flight <= 151903) AND (blessing='real') )
AND EXISTS (SELECT 1
FROM jobs
WHERE jobs.flight = flights.flight
AND jobs.job = ?)
AND r1.name LIKE 'built\_revision\_%'
AND r1.name = ?
AND r1.val= ?
ORDER BY flight DESC
LIMIT 1000
) AS sub
ORDER BY blessing ASC, flight DESC
With these bind variables:
"test-armhf-armhf-libvirt"
'built_revision_xen'
'165f3afbfc3db70fcfdccad07085cde0a03c858b'
Diff to the query:
SELECT * FROM (
- SELECT flight, blessing FROM flights
+ SELECT DISTINCT flight, blessing
+ FROM flights
+ JOIN runvars r1 USING (flight)
+
WHERE (branch='xen-unstable')
+ AND ( (TRUE AND flight <= 151903) AND (blessing='real') )
AND EXISTS (SELECT 1
FROM jobs
WHERE jobs.flight = flights.flight
AND jobs.job = ?)
- AND ( (TRUE AND flight <= 151903) AND (blessing='real') )
+ AND r1.name LIKE 'built\_revision\_%'
+ AND r1.name = ?
+ AND r1.val= ?
+
ORDER BY flight DESC
LIMIT 1000
) AS sub
CC: George Dunlap <George.Dunlap@citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
v2: Use proper \ escaping for underscores in LIKE
---
schema/runvars-built-index.sql | 2 +-
sg-report-flight | 64 ++++++++++++++++++++++++++++++++--
2 files changed, 62 insertions(+), 4 deletions(-)
diff --git a/schema/runvars-built-index.sql b/schema/runvars-built-index.sql
index 7108e0af..128e69e9 100644
--- a/schema/runvars-built-index.sql
+++ b/schema/runvars-built-index.sql
@@ -1,4 +1,4 @@
--- ##OSSTEST## 007 Preparatory
+-- ##OSSTEST## 007 Needed
--
-- This index helps sg-report-flight find relevant flights.
diff --git a/sg-report-flight b/sg-report-flight
index 7f2790ce..10127582 100755
--- a/sg-report-flight
+++ b/sg-report-flight
@@ -185,19 +185,77 @@ END
if (defined $job) {
push @flightsq_params, $job;
$flightsq_jobcond = <<END;
- EXISTS (SELECT 1
+ AND EXISTS (SELECT 1
FROM jobs
WHERE jobs.flight = flights.flight
AND jobs.job = ?)
END
}
+ # We build a slightly complicated query to find possibly-relevant
+ # flights. A "possibly-relevant" flight is one which the main
+ # flight categorisation algorithm below (the loop over $tflight)
+ # *might* decide is of interest.
+ #
+ # That algorithm produces a table of which revision(s) of what
+ # %specver trees the build jobs for the relevant test job used.
+ # And then it insists (amongst other things) that for each such
+ # tree the revision in question appears.
+ #
+ # It only looks at build jobs within the flight. So any flight
+ # that the main algorithm finds interesting will have *some* job
+ # (in the same flight) mentioning that revision in a built
+ # revision runvar. So we can search the runvars table by its
+ # index on the revision.
+ #
+ # So we look for flights that have an appropriate entry in runvars
+ # for each %specver tree. We can do this by joining the runvar
+ # table once for each tree.
+ #
+ # The "osstest" tree is handled specially. as ever. (We use
+ # "r$ri" there too for orthogonality of the code, not because
+ # there could be multiple specifiations for the osstest revision.)
+ #
+ # This complex query is an optimisation: for correctness, we must
+ # still execute the full job-specific recursive examination, for
+ # each possibly-relevant flight - that's the $tflight loop body.
+
+ my $runvars_joins = '';
+ my $runvars_conds = '';
+ my $ri=0;
+ foreach my $tree (sort keys %{ $specver{$thisthat} }) {
+ $ri++;
+ if ($tree ne 'osstest') {
+ $runvars_joins .= <<END;
+ JOIN runvars r$ri USING (flight)
+END
+ $runvars_conds .= <<END;
+ AND r$ri.name LIKE 'built\_revision\_%'
+ AND r$ri.name = ?
+ AND r$ri.val= ?
+END
+ push @flightsq_params, "built_revision_$tree",
+ $specver{$thisthat}{$tree};
+ } else {
+ $runvars_joins .= <<END;
+ JOIN flights_harness_touched r$ri USING (flight)
+END
+ $runvars_conds .= <<END;
+ AND r$ri.harness= ?
+END
+ push @flightsq_params, $specver{$thisthat}{$tree};
+ }
+ }
+
my $flightsq= <<END;
SELECT * FROM (
- SELECT flight, blessing FROM flights
+ SELECT DISTINCT flight, blessing
+ FROM flights
+$runvars_joins
WHERE $branches_cond_q
- AND $flightsq_jobcond
AND $blessingscond
+$flightsq_jobcond
+$runvars_conds
ORDER BY flight DESC
LIMIT 1000
) AS sub
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 09/41] sg-report-flight: Use WITH to use best index use for $flightsq
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (7 preceding siblings ...)
2020-07-31 11:37 ` [OSSTEST PATCH v2 08/41] sg-report-flight: Ask the db for flights of interest Ian Jackson
@ 2020-07-31 11:37 ` Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 10/41] sg-report-flight: Use WITH clause to use index for $anypassq Ian Jackson
` (31 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:37 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, George Dunlap
While we're here, convert this EXISTS subquery to a JOIN.
Perf: runtime of my test case now ~200-300s.
Example query before (from the Perl DBI trace):
SELECT * FROM (
SELECT DISTINCT flight, blessing
FROM flights
JOIN runvars r1 USING (flight)
WHERE (branch='xen-unstable')
AND ( (TRUE AND flight <= 151903) AND (blessing='real') )
AND EXISTS (SELECT 1
FROM jobs
WHERE jobs.flight = flights.flight
AND jobs.job = ?)
AND r1.name LIKE 'built_revision_%'
AND r1.name = ?
AND r1.val= ?
ORDER BY flight DESC
LIMIT 1000
) AS sub
ORDER BY blessing ASC, flight DESC
With bind variables:
"test-armhf-armhf-libvirt"
'built_revision_xen'
'165f3afbfc3db70fcfdccad07085cde0a03c858b'
After:
WITH sub AS (
SELECT DISTINCT flight, blessing
FROM flights
JOIN runvars r1 USING (flight)
WHERE (branch='xen-unstable')
AND ( (TRUE AND flight <= 151903) AND (blessing='real') )
AND r1.name LIKE 'built_revision_%'
AND r1.name = ?
AND r1.val= ?
ORDER BY flight DESC
LIMIT 1000
)
SELECT *
FROM sub
JOIN jobs USING (flight)
WHERE (1=1)
AND jobs.job = ?
ORDER BY blessing ASC, flight DESC
With bind variables:
'built_revision_xen'
'165f3afbfc3db70fcfdccad07085cde0a03c858b'
"test-armhf-armhf-libvirt"
Diff to the query:
- SELECT * FROM (
+ WITH sub AS (
SELECT DISTINCT flight, blessing
FROM flights
JOIN runvars r1 USING (flight)
WHERE (branch='xen-unstable')
AND ( (TRUE AND flight <= 151903) AND (blessing='real') )
- AND EXISTS (SELECT 1
- FROM jobs
- WHERE jobs.flight = flights.flight
- AND jobs.job = ?)
-
AND r1.name LIKE 'built_revision_%'
AND r1.name = ?
AND r1.val= ?
ORDER BY flight DESC
LIMIT 1000
- ) AS sub
+ )
+ SELECT *
+ FROM sub
+ JOIN jobs USING (flight)
+
+ WHERE (1=1)
+ AND jobs.job = ?
+
ORDER BY blessing ASC, flight DESC
Reviewed-by: George Dunlap <George.Dunlap@citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
sg-report-flight | 39 ++++++++++++++++++++++++---------------
1 file changed, 24 insertions(+), 15 deletions(-)
diff --git a/sg-report-flight b/sg-report-flight
index 10127582..d06be292 100755
--- a/sg-report-flight
+++ b/sg-report-flight
@@ -180,18 +180,6 @@ END
return undef;
}
- my @flightsq_params;
- my $flightsq_jobcond='(1=1)';
- if (defined $job) {
- push @flightsq_params, $job;
- $flightsq_jobcond = <<END;
- AND EXISTS (SELECT 1
- FROM jobs
- WHERE jobs.flight = flights.flight
- AND jobs.job = ?)
-END
- }
-
# We build a slightly complicated query to find possibly-relevant
# flights. A "possibly-relevant" flight is one which the main
# flight categorisation algorithm below (the loop over $tflight)
@@ -220,6 +208,7 @@ END
# still execute the full job-specific recursive examination, for
# each possibly-relevant flight - that's the $tflight loop body.
+ my @flightsq_params;
my $runvars_joins = '';
my $runvars_conds = '';
my $ri=0;
@@ -247,18 +236,38 @@ END
}
}
+ my $flightsq_jobs_join = '';
+ my $flightsq_jobcond = '';
+ if (defined $job) {
+ push @flightsq_params, $job;
+ $flightsq_jobs_join = <<END;
+ JOIN jobs USING (flight)
+END
+ $flightsq_jobcond = <<END;
+ AND jobs.job = ?
+END
+ }
+
+ # In psql 9.6 this WITH clause makes postgresql do the flights
+ # query first. This is good because our built revision index finds
+ # relevant flights very quickly. Without this, postgresql seems
+ # to like to scan the jobs table.
my $flightsq= <<END;
- SELECT * FROM (
+ WITH sub AS (
SELECT DISTINCT flight, blessing
FROM flights
$runvars_joins
WHERE $branches_cond_q
AND $blessingscond
-$flightsq_jobcond
$runvars_conds
ORDER BY flight DESC
LIMIT 1000
- ) AS sub
+ )
+ SELECT *
+ FROM sub
+$flightsq_jobs_join
+ WHERE (1=1)
+$flightsq_jobcond
ORDER BY blessing ASC, flight DESC
END
$flightsq= db_prepare($flightsq);
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 10/41] sg-report-flight: Use WITH clause to use index for $anypassq
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (8 preceding siblings ...)
2020-07-31 11:37 ` [OSSTEST PATCH v2 09/41] sg-report-flight: Use WITH to use best index use for $flightsq Ian Jackson
@ 2020-07-31 11:37 ` Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 11/41] sg-report-flight: Use the job row from the intitial query Ian Jackson
` (30 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:37 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, George Dunlap
Perf: runtime of my test case now ~11s
Example query before (from the Perl DBI trace):
SELECT * FROM flights JOIN steps USING (flight)
WHERE (branch='xen-unstable')
AND job=? and testid=? and status='pass'
AND ( (TRUE AND flight <= 151903) AND (blessing='real') )
LIMIT 1
After:
WITH s AS
(
SELECT * FROM steps
WHERE job=? and testid=? and status='pass'
)
SELECT * FROM flights JOIN s USING (flight)
WHERE (branch='xen-unstable')
AND ( (TRUE AND flight <= 151903) AND (blessing='real') )
LIMIT 1
In both cases with bind vars:
"test-amd64-i386-xl-pvshim"
"guest-start"
Diff to the query:
- SELECT * FROM flights JOIN steps USING (flight)
+ WITH s AS
+ (
+ SELECT * FROM steps
+ WHERE job=? and testid=? and status='pass'
+ )
+ SELECT * FROM flights JOIN s USING (flight)
WHERE (branch='xen-unstable')
- AND job=? and testid=? and status='pass'
AND ( (TRUE AND flight <= 151903) AND (blessing='real') )
LIMIT 1
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
---
schema/steps-job-index.sql | 2 +-
sg-report-flight | 14 ++++++++++++--
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/schema/steps-job-index.sql b/schema/steps-job-index.sql
index 07dc5a30..2c33af72 100644
--- a/schema/steps-job-index.sql
+++ b/schema/steps-job-index.sql
@@ -1,4 +1,4 @@
--- ##OSSTEST## 006 Preparatory
+-- ##OSSTEST## 006 Needed
--
-- This index helps sg-report-flight find if a test ever passed.
diff --git a/sg-report-flight b/sg-report-flight
index d06be292..d218b24e 100755
--- a/sg-report-flight
+++ b/sg-report-flight
@@ -849,10 +849,20 @@ sub justifyfailures ($;$) {
my @failures= values %{ $fi->{Failures} };
+ # In psql 9.6 this WITH clause makes postgresql do the steps query
+ # first. This is good because if this test never passed we can
+ # determine that really quickly using the new index, without
+ # having to scan the flights table. (If the test passed we will
+ # probably not have to look at many flights to find one, so in
+ # that case this is not much worse.)
my $anypassq= <<END;
- SELECT * FROM flights JOIN steps USING (flight)
+ WITH s AS
+ (
+ SELECT * FROM steps
+ WHERE job=? and testid=? and status='pass'
+ )
+ SELECT * FROM flights JOIN s USING (flight)
WHERE $branches_cond_q
- AND job=? and testid=? and status='pass'
AND $blessingscond
LIMIT 1
END
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 11/41] sg-report-flight: Use the job row from the intitial query
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (9 preceding siblings ...)
2020-07-31 11:37 ` [OSSTEST PATCH v2 10/41] sg-report-flight: Use WITH clause to use index for $anypassq Ian Jackson
@ 2020-07-31 11:37 ` Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 12/41] Executive: Use index for report__find_test Ian Jackson
` (29 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:37 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
$jcheckq is redundant: we looked this up right at the start.
This is not expected to speed things up very much, but it makes things
somewhat cleaner and clearer.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
sg-report-flight | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/sg-report-flight b/sg-report-flight
index d218b24e..cb6b8174 100755
--- a/sg-report-flight
+++ b/sg-report-flight
@@ -160,10 +160,6 @@ sub findaflight ($$$$$) {
return undef;
}
- my $jcheckq= db_prepare(<<END);
- SELECT status FROM jobs WHERE flight=? AND job=?
-END
-
my $checkq= db_prepare(<<END);
SELECT status FROM steps WHERE flight=? AND job=? AND testid=?
AND status!='skip'
@@ -263,7 +259,7 @@ $runvars_conds
ORDER BY flight DESC
LIMIT 1000
)
- SELECT *
+ SELECT flight, jobs.status
FROM sub
$flightsq_jobs_join
WHERE (1=1)
@@ -304,7 +300,7 @@ END
WHERE flight=?
END
- while (my ($tflight) = $flightsq->fetchrow_array) {
+ while (my ($tflight, $tjstatus) = $flightsq->fetchrow_array) {
# Recurse from the starting flight looking for relevant build
# jobs. We start with all jobs in $tflight, and for each job
# we also process any other jobs it refers to in *buildjob runvars.
@@ -407,8 +403,7 @@ END
$checkq->execute($tflight, $job, $testid);
($chkst) = $checkq->fetchrow_array();
if (!defined $chkst) {
- $jcheckq->execute($tflight, $job);
- my ($jchkst) = $jcheckq->fetchrow_array();
+ my $jchkst = $tflight->{status};
$chkst = $jchkst if $jchkst eq 'starved';
}
}
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 12/41] Executive: Use index for report__find_test
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (10 preceding siblings ...)
2020-07-31 11:37 ` [OSSTEST PATCH v2 11/41] sg-report-flight: Use the job row from the intitial query Ian Jackson
@ 2020-07-31 11:37 ` Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 13/41] duration_estimator: Ignore truncated jobs unless we know the step Ian Jackson
` (28 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:37 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, George Dunlap
After we refactor this query then we can enable the index use.
(Both of these things together in this commit because I haven't perf
tested the version with just the refactoring.)
(We have provided an index that can answer this question really
quickly if a version is specified. But the query planner couldn't see
that because it works without seeing the bind variables, so doesn't
know that the value of name is going to be suitable for this index.)
* Convert the two EXISTS subqueries into JOIN/AND with a DISTINCT
clause naming the fields on flights, so as to replicate the previous
result rows. Then do $selection field last. The subquery is a
convenient way to let this do the previous thing for all the values
of $selection (including, notably, *).
* Add the additional AND clause for r.name, which has no logical
effect given the actual values of name, enabling the query planner
to use this index.
Perf: In my test case the sg-report-flight runtime is now ~8s. I am
reasonably confident that this will not make other use cases of this
code worse.
Perf: runtime of my test case now ~11s
Example query before (from the Perl DBI trace):
SELECT *
FROM flights f
WHERE
EXISTS (
SELECT 1
FROM runvars r
WHERE name=?
AND val=?
AND r.flight=f.flight
AND ( (CASE
WHEN (r.job) LIKE 'build-%-prev' THEN 'xprev'
WHEN ((r.job) LIKE 'build-%-freebsd'
AND 'x' = 'freebsdbuildjob') THEN 'DISCARD'
ELSE ''
END)
= '')
)
AND ( (TRUE AND flight <= 151903) AND (blessing='real') )
AND (branch=?)
ORDER BY flight DESC
LIMIT 1
After:
SELECT *
FROM ( SELECT DISTINCT
flight, started, blessing, branch, intended
FROM flights f
JOIN runvars r USING (flight)
WHERE name=?
AND name LIKE 'revision\_%'
AND val=?
AND r.flight=f.flight
AND ( (CASE
WHEN (r.job) LIKE 'build-%-prev' THEN 'xprev'
WHEN ((r.job) LIKE 'build-%-freebsd'
AND 'x' = 'freebsdbuildjob') THEN 'DISCARD'
ELSE ''
END)
= '')
AND ( (TRUE AND flight <= 151903) AND (blessing='real') )
AND (branch=?)
) AS sub WHERE TRUE
ORDER BY flight DESC
LIMIT 1
In both cases with bind vars:
'revision_xen'
'165f3afbfc3db70fcfdccad07085cde0a03c858b'
"xen-unstable"
Diff to the example query:
@@ -1,10 +1,10 @@
SELECT *
+ FROM ( SELECT DISTINCT
+ flight, started, blessing, branch, intended
FROM flights f
- WHERE
- EXISTS (
- SELECT 1
- FROM runvars r
+ JOIN runvars r USING (flight)
WHERE name=?
+ AND name LIKE 'revision\_%'
AND val=?
AND r.flight=f.flight
AND ( (CASE
@@ -14,8 +14,8 @@
ELSE ''
END)
= '')
- )
AND ( (TRUE AND flight <= 151903) AND (blessing='real') )
AND (branch=?)
+) AS sub WHERE TRUE
ORDER BY flight DESC
LIMIT 1
Reviewed-by: George Dunlap <George.Dunlap@citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
v2: Use proper \ escaping for underscores in LIKE
---
Osstest/Executive.pm | 20 ++++++++------------
schema/runvars-revision-index.sql | 2 +-
2 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index c3dc1261..9208d8af 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -415,37 +415,32 @@ sub report__find_test ($$$$$$$) {
my $querytext = <<END;
SELECT $selection
- FROM flights f
- WHERE
+ FROM ( SELECT DISTINCT
+ flight, started, blessing, branch, intended
+ FROM flights f
END
if (defined $revision) {
if ($tree eq 'osstest') {
$querytext .= <<END;
- EXISTS (
- SELECT 1
- FROM flights_harness_touched t
+ JOIN flights_harness_touched t USING (flight)
WHERE t.harness=?
- AND t.flight=f.flight
- )
END
push @params, $revision;
} else {
$querytext .= <<END;
- EXISTS (
- SELECT 1
- FROM runvars r
+ JOIN runvars r USING (flight)
WHERE name=?
+ AND name LIKE 'revision\_%'
AND val=?
AND r.flight=f.flight
AND ${\ main_revision_job_cond('r.job') }
- )
END
push @params, "revision_$tree", $revision;
}
} else {
$querytext .= <<END;
- TRUE
+ WHERE TRUE
END
}
@@ -460,6 +455,7 @@ END
END
push @params, @$branches;
+ $querytext .= ") AS sub WHERE TRUE\n";
$querytext .= $extracond;
$querytext .= $sortlimit;
diff --git a/schema/runvars-revision-index.sql b/schema/runvars-revision-index.sql
index 8871b528..25306354 100644
--- a/schema/runvars-revision-index.sql
+++ b/schema/runvars-revision-index.sql
@@ -1,4 +1,4 @@
--- ##OSSTEST## 008 Preparatory
+-- ##OSSTEST## 008 Needed
--
-- This index helps Executive::report__find_test find relevant flights.
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 13/41] duration_estimator: Ignore truncated jobs unless we know the step
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (11 preceding siblings ...)
2020-07-31 11:37 ` [OSSTEST PATCH v2 12/41] Executive: Use index for report__find_test Ian Jackson
@ 2020-07-31 11:37 ` Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 14/41] duration_estimator: Introduce some _qtxt variables Ian Jackson
` (27 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:37 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
If we are looking for a particular step then we will ignore jobs
without that step, so any job which was truncated before it will be
ignored.
Otherwise we are looking for the whole job duration and a truncated
job is not a good representative.
This is a bugfix (to duration estimation), not a performance
improvement like the preceding and subsequent changes.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Osstest/Executive.pm | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index 9208d8af..f528edd0 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -1142,6 +1142,10 @@ sub duration_estimator ($$;$$) {
# estimated (and only jobs which contained that step will be
# considered).
+ my $or_status_truncated = '';
+ if ($will_uptoincl_testid) {
+ $or_status_truncated = "OR j.status='truncated'!";
+ }
my $recentflights_q= $dbh_tests->prepare(<<END);
SELECT f.flight AS flight,
f.started AS started,
@@ -1156,8 +1160,8 @@ sub duration_estimator ($$;$$) {
AND f.branch=?
AND j.job=?
AND r.val=?
- AND (j.status='pass' OR j.status='fail' OR
- j.status='truncated')
+ AND (j.status='pass' OR j.status='fail'
+ $or_status_truncated)
AND f.started IS NOT NULL
AND f.started >= ?
ORDER BY f.started DESC
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 14/41] duration_estimator: Introduce some _qtxt variables
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (12 preceding siblings ...)
2020-07-31 11:37 ` [OSSTEST PATCH v2 13/41] duration_estimator: Ignore truncated jobs unless we know the step Ian Jackson
@ 2020-07-31 11:37 ` Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 15/41] duration_estimator: Explicitly provide null in general host q Ian Jackson
` (26 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:37 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
No functional change.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Osstest/Executive.pm | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index f528edd0..4cb22cc9 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -1146,7 +1146,7 @@ sub duration_estimator ($$;$$) {
if ($will_uptoincl_testid) {
$or_status_truncated = "OR j.status='truncated'!";
}
- my $recentflights_q= $dbh_tests->prepare(<<END);
+ my $recentflights_qtxt= <<END;
SELECT f.flight AS flight,
f.started AS started,
j.status AS status
@@ -1167,7 +1167,7 @@ sub duration_estimator ($$;$$) {
ORDER BY f.started DESC
END
- my $duration_anyref_q= $dbh_tests->prepare(<<END);
+ my $duration_anyref_qtxt= <<END;
SELECT f.flight AS flight,
max(s.finished) AS max_finished
FROM steps s JOIN flights f
@@ -1212,6 +1212,8 @@ END_UPTOINCL
AS duration
END_ALWAYS
+ my $recentflights_q= $dbh_tests->prepare($recentflights_qtxt);
+ my $duration_anyref_q= $dbh_tests->prepare($duration_anyref_qtxt);
my $duration_duration_q = $dbh_tests->prepare($duration_duration_qtxt);
return sub {
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 15/41] duration_estimator: Explicitly provide null in general host q
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (13 preceding siblings ...)
2020-07-31 11:37 ` [OSSTEST PATCH v2 14/41] duration_estimator: Introduce some _qtxt variables Ian Jackson
@ 2020-07-31 11:37 ` Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 16/41] duration_estimator: Return job column in first query Ian Jackson
` (25 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:37 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
Our spec. says we return nulls for started and status if we don't find
a job matching the host spec.
The way this works right now is that we look up the nonexistent
entries in $refs->[0]. This is not really brilliant and is going to
be troublesome as we continue to refactor.
Provide these values explicitly. No functional change.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Osstest/Executive.pm | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index 4cb22cc9..d45d6557 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -1169,6 +1169,8 @@ END
my $duration_anyref_qtxt= <<END;
SELECT f.flight AS flight,
+ NULL as started,
+ NULL as status,
max(s.finished) AS max_finished
FROM steps s JOIN flights f
ON s.flight=f.flight
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 16/41] duration_estimator: Return job column in first query
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (14 preceding siblings ...)
2020-07-31 11:37 ` [OSSTEST PATCH v2 15/41] duration_estimator: Explicitly provide null in general host q Ian Jackson
@ 2020-07-31 11:37 ` Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 17/41] duration_estimator: Move $uptincl_testid to separate @x_params Ian Jackson
` (24 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:37 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, George Dunlap
Right now this is pointless since the Perl code doesn't need it. But
this row is going to be part of a WITH clause soon.
No functional change.
Diffs to two example queries (from the Perl DBI trace):
SELECT f.flight AS flight,
+ j.job AS job,
f.started AS started,
j.status AS status
FROM flights f
JOIN jobs j USING (flight)
JOIN runvars r
ON f.flight=r.flight
AND r.name=?
WHERE j.job=r.job
AND f.blessing=?
AND f.branch=?
AND j.job=?
AND r.val=?
AND (j.status='pass' OR j.status='fail'
OR j.status='truncated'!)
AND f.started IS NOT NULL
AND f.started >= ?
ORDER BY f.started DESC
SELECT f.flight AS flight,
+ s.job AS job,
NULL as started,
NULL as status,
max(s.finished) AS max_finished
FROM steps s JOIN flights f
ON s.flight=f.flight
WHERE s.job=? AND f.blessing=? AND f.branch=?
AND s.finished IS NOT NULL
AND f.started IS NOT NULL
AND f.started >= ?
- GROUP BY f.flight
+ GROUP BY f.flight, s.job
ORDER BY max_finished DESC
CC: George Dunlap <George.Dunlap@citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Osstest/Executive.pm | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index d45d6557..359120c0 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -1148,6 +1148,7 @@ sub duration_estimator ($$;$$) {
}
my $recentflights_qtxt= <<END;
SELECT f.flight AS flight,
+ j.job AS job,
f.started AS started,
j.status AS status
FROM flights f
@@ -1169,6 +1170,7 @@ END
my $duration_anyref_qtxt= <<END;
SELECT f.flight AS flight,
+ s.job AS job,
NULL as started,
NULL as status,
max(s.finished) AS max_finished
@@ -1178,7 +1180,7 @@ END
AND s.finished IS NOT NULL
AND f.started IS NOT NULL
AND f.started >= ?
- GROUP BY f.flight
+ GROUP BY f.flight, s.job
ORDER BY max_finished DESC
END
# s J J J # fix perl-mode
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 17/41] duration_estimator: Move $uptincl_testid to separate @x_params
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (15 preceding siblings ...)
2020-07-31 11:37 ` [OSSTEST PATCH v2 16/41] duration_estimator: Return job column in first query Ian Jackson
@ 2020-07-31 11:37 ` Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 18/41] duration_estimator: Move duration query loop into database Ian Jackson
` (23 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:37 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
This is going to be useful soon.
No functional change.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Osstest/Executive.pm | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index 359120c0..fb975dac 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -1223,6 +1223,9 @@ END_ALWAYS
return sub {
my ($job, $hostidname, $onhost, $uptoincl_testid) = @_;
+ my @x_params;
+ push @x_params, $uptoincl_testid if $will_uptoincl_testid;
+
my $dbg= $debug ? sub {
$debug->("DUR $branch $blessing $job $hostidname $onhost @_");
} : sub { };
@@ -1257,7 +1260,7 @@ END_ALWAYS
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;
+ push @d_d_args, @x_params;
$duration_duration_q->execute(@d_d_args);
my ($duration) = $duration_duration_q->fetchrow_array();
$duration_duration_q->finish();
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 18/41] duration_estimator: Move duration query loop into database
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (16 preceding siblings ...)
2020-07-31 11:37 ` [OSSTEST PATCH v2 17/41] duration_estimator: Move $uptincl_testid to separate @x_params Ian Jackson
@ 2020-07-31 11:37 ` Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 19/41] Executive: Drop redundant AND clause Ian Jackson
` (22 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:37 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, George Dunlap
Stuff the two queries together: we use the firsty query as a WITH
clause. This is significantly faster, perhaps because the query
optimiser does a better job but probably just because it saves on
round trips.
No functional change.
Perf: subjectively this seemed to help when the cache was cold. Now I
have a warm cache and it doesn't seem to make much difference.
Perf: runtime of my test case now ~5-7s.
Example queries before (from the debugging output):
Query A part I:
SELECT f.flight AS flight,
j.job AS job,
f.started AS started,
j.status AS status
FROM flights f
JOIN jobs j USING (flight)
JOIN runvars r
ON f.flight=r.flight
AND r.name=?
WHERE j.job=r.job
AND f.blessing=?
AND f.branch=?
AND j.job=?
AND r.val=?
AND (j.status='pass' OR j.status='fail'
OR j.status='truncated'!)
AND f.started IS NOT NULL
AND f.started >= ?
ORDER BY f.started DESC
With bind variables:
"test-amd64-i386-xl-pvshim"
"guest-start"
Query B part I:
SELECT f.flight AS flight,
s.job AS job,
NULL as started,
NULL as status,
max(s.finished) AS max_finished
FROM steps s JOIN flights f
ON s.flight=f.flight
WHERE s.job=? AND f.blessing=? AND f.branch=?
AND s.finished IS NOT NULL
AND f.started IS NOT NULL
AND f.started >= ?
GROUP BY f.flight, s.job
ORDER BY max_finished DESC
With bind variables:
"test-armhf-armhf-libvirt"
'real'
"xen-unstable"
1594144469
Query common part II:
WITH tsteps AS
(
SELECT *
FROM steps
WHERE flight=? AND job=?
)
, tsteps2 AS
(
SELECT *
FROM tsteps
WHERE finished <=
(SELECT finished
FROM tsteps
WHERE tsteps.testid = ?)
)
SELECT (
SELECT max(finished)-min(started)
FROM tsteps2
) - (
SELECT sum(finished-started)
FROM tsteps2
WHERE step = 'ts-hosts-allocate'
)
AS duration
With bind variables from previous query, eg:
152045
"test-armhf-armhf-libvirt"
"guest-start.2"
After:
Query A (combined):
WITH f AS (
SELECT f.flight AS flight,
j.job AS job,
f.started AS started,
j.status AS status
FROM flights f
JOIN jobs j USING (flight)
JOIN runvars r
ON f.flight=r.flight
AND r.name=?
WHERE j.job=r.job
AND f.blessing=?
AND f.branch=?
AND j.job=?
AND r.val=?
AND (j.status='pass' OR j.status='fail'
OR j.status='truncated'!)
AND f.started IS NOT NULL
AND f.started >= ?
ORDER BY f.started DESC
)
SELECT flight, max_finished, job, started, status,
(
WITH tsteps AS
(
SELECT *
FROM steps
WHERE flight=f.flight AND job=f.job
)
, tsteps2 AS
(
SELECT *
FROM tsteps
WHERE finished <=
(SELECT finished
FROM tsteps
WHERE tsteps.testid = ?)
)
SELECT (
SELECT max(finished)-min(started)
FROM tsteps2
) - (
SELECT sum(finished-started)
FROM tsteps2
WHERE step = 'ts-hosts-allocate'
)
AS duration
) FROM f
Query B (combined):
WITH f AS (
SELECT f.flight AS flight,
s.job AS job,
NULL as started,
NULL as status,
max(s.finished) AS max_finished
FROM steps s JOIN flights f
ON s.flight=f.flight
WHERE s.job=? AND f.blessing=? AND f.branch=?
AND s.finished IS NOT NULL
AND f.started IS NOT NULL
AND f.started >= ?
GROUP BY f.flight, s.job
ORDER BY max_finished DESC
)
SELECT flight, max_finished, job, started, status,
(
WITH tsteps AS
(
SELECT *
FROM steps
WHERE flight=f.flight AND job=f.job
)
, tsteps2 AS
(
SELECT *
FROM tsteps
WHERE finished <=
(SELECT finished
FROM tsteps
WHERE tsteps.testid = ?)
)
SELECT (
SELECT max(finished)-min(started)
FROM tsteps2
) - (
SELECT sum(finished-started)
FROM tsteps2
WHERE step = 'ts-hosts-allocate'
)
AS duration
) FROM f
Diff for query A:
@@ -1,3 +1,4 @@
+ WITH f AS (
SELECT f.flight AS flight,
j.job AS job,
f.started AS started,
@@ -18,11 +19,14 @@
AND f.started >= ?
ORDER BY f.started DESC
+ )
+ SELECT flight, max_finished, job, started, status,
+ (
WITH tsteps AS
(
SELECT *
FROM steps
- WHERE flight=? AND job=?
+ WHERE flight=f.flight AND job=f.job
)
, tsteps2 AS
(
@@ -42,3 +46,5 @@
WHERE step = 'ts-hosts-allocate'
)
AS duration
+
+ ) FROM f
Diff for query B:
@@ -1,3 +1,4 @@
+ WITH f AS (
SELECT f.flight AS flight,
s.job AS job,
NULL as started,
@@ -12,11 +13,14 @@
GROUP BY f.flight, s.job
ORDER BY max_finished DESC
+ )
+ SELECT flight, max_finished, job, started, status,
+ (
WITH tsteps AS
(
SELECT *
FROM steps
- WHERE flight=? AND job=?
+ WHERE flight=f.flight AND job=f.job
)
, tsteps2 AS
(
@@ -36,3 +40,5 @@
WHERE step = 'ts-hosts-allocate'
)
AS duration
+
+ ) FROM f
Reviewed-by: George Dunlap <George.Dunlap@citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Osstest/Executive.pm | 31 ++++++++++++++++++++-----------
1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index fb975dac..684cafc3 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -1192,7 +1192,7 @@ END
(
SELECT *
FROM steps
- WHERE flight=? AND job=?
+ WHERE flight=f.flight AND job=f.job
)
END_ALWAYS
, tsteps2 AS
@@ -1216,9 +1216,20 @@ END_UPTOINCL
AS duration
END_ALWAYS
- my $recentflights_q= $dbh_tests->prepare($recentflights_qtxt);
- my $duration_anyref_q= $dbh_tests->prepare($duration_anyref_qtxt);
- my $duration_duration_q = $dbh_tests->prepare($duration_duration_qtxt);
+ my $prepare_combi = sub {
+ db_prepare(<<END);
+ WITH f AS (
+$_[0]
+ )
+ SELECT flight, max_finished, job, started, status,
+ (
+$duration_duration_qtxt
+ ) FROM f
+END
+ };
+
+ my $recentflights_q= $prepare_combi->($recentflights_qtxt);
+ my $duration_anyref_q= $prepare_combi->($duration_anyref_qtxt);
return sub {
my ($job, $hostidname, $onhost, $uptoincl_testid) = @_;
@@ -1239,14 +1250,16 @@ END_ALWAYS
$branch,
$job,
$onhost,
- $limit);
+ $limit,
+ @x_params);
$refs= $recentflights_q->fetchall_arrayref({});
$recentflights_q->finish();
$dbg->("SAME-HOST GOT ".scalar(@$refs));
}
if (!@$refs) {
- $duration_anyref_q->execute($job, $blessing, $branch, $limit);
+ $duration_anyref_q->execute($job, $blessing, $branch, $limit,
+ @x_params);
$refs= $duration_anyref_q->fetchall_arrayref({});
$duration_anyref_q->finish();
$dbg->("ANY-HOST GOT ".scalar(@$refs));
@@ -1259,11 +1272,7 @@ END_ALWAYS
my $duration_max= 0;
foreach my $ref (@$refs) {
- my @d_d_args = ($ref->{flight}, $job);
- push @d_d_args, @x_params;
- $duration_duration_q->execute(@d_d_args);
- my ($duration) = $duration_duration_q->fetchrow_array();
- $duration_duration_q->finish();
+ my ($duration) = $ref->{duration};
if ($duration) {
$dbg->("REF $ref->{flight} DURATION $duration ".
($ref->{status} // ''));
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 19/41] Executive: Drop redundant AND clause
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (17 preceding siblings ...)
2020-07-31 11:37 ` [OSSTEST PATCH v2 18/41] duration_estimator: Move duration query loop into database Ian Jackson
@ 2020-07-31 11:37 ` Ian Jackson
2020-07-31 14:17 ` George Dunlap
2020-07-31 11:37 ` [OSSTEST PATCH v2 20/41] schema: Add index for quick lookup by host Ian Jackson
` (21 subsequent siblings)
40 siblings, 1 reply; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:37 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, George Dunlap
In "Executive: Use index for report__find_test" we changed an EXISTS
subquery into a JOIN.
Now, the condition r.flight=f.flight is redundant because this is the
join column (from USING).
No functional change.
CC: George Dunlap <George.Dunlap@citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Osstest/Executive.pm | 1 -
1 file changed, 1 deletion(-)
diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index 684cafc3..2f81e89d 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -433,7 +433,6 @@ END
WHERE name=?
AND name LIKE 'revision\_%'
AND val=?
- AND r.flight=f.flight
AND ${\ main_revision_job_cond('r.job') }
END
push @params, "revision_$tree", $revision;
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 20/41] schema: Add index for quick lookup by host
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (18 preceding siblings ...)
2020-07-31 11:37 ` [OSSTEST PATCH v2 19/41] Executive: Drop redundant AND clause Ian Jackson
@ 2020-07-31 11:37 ` Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 21/41] sg-report-host-history: Find flight limit by flight start date Ian Jackson
` (20 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:37 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
v2: Use proper \ escaping for underscores in LIKE
---
schema/runvars-host-index.sql | 8 ++++++++
1 file changed, 8 insertions(+)
create mode 100644 schema/runvars-host-index.sql
diff --git a/schema/runvars-host-index.sql b/schema/runvars-host-index.sql
new file mode 100644
index 00000000..222a0a30
--- /dev/null
+++ b/schema/runvars-host-index.sql
@@ -0,0 +1,8 @@
+-- ##OSSTEST## 009 Preparatory
+--
+-- This index helps sg-report-host-history find relevant flights.
+
+CREATE INDEX runvars_host_idx
+ ON runvars (val, flight)
+ WHERE name ='host'
+ OR name LIKE '%\_host';
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 21/41] sg-report-host-history: Find flight limit by flight start date
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (19 preceding siblings ...)
2020-07-31 11:37 ` [OSSTEST PATCH v2 20/41] schema: Add index for quick lookup by host Ian Jackson
@ 2020-07-31 11:38 ` Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 22/41] sg-report-host-history: Drop per-job debug etc Ian Jackson
` (19 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:38 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
By default we look for anything in (roughly) the last year.
This query is in fact quite fast because the flights table is small.
There is still the per-host limit of $limit (2000) recent runs.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
sg-report-host-history | 56 ++++++++++++++++++++----------------------
1 file changed, 27 insertions(+), 29 deletions(-)
diff --git a/sg-report-host-history b/sg-report-host-history
index 9730ae7a..a159df3e 100755
--- a/sg-report-host-history
+++ b/sg-report-host-history
@@ -29,6 +29,7 @@ use POSIX;
use Osstest::Executive qw(:DEFAULT :colours);
our $limit= 2000;
+our $timelimit= 86400 * (366 + 14);
our $flightlimit;
our $htmlout = ".";
our $read_existing=1;
@@ -45,6 +46,8 @@ while (@ARGV && $ARGV[0] =~ m/^-/) {
last if m/^--?$/;
if (m/^--(limit)\=([1-9]\d*)$/) {
$$1= $2;
+ } elsif (m/^--time-limit\=([1-9]\d*)$/) {
+ $timelimit= $1;
} elsif (m/^--flight-limit\=([1-9]\d*)$/) {
$flightlimit= $1;
} elsif (restrictflight_arg($_)) {
@@ -108,38 +111,33 @@ sub read_existing_logs ($) {
}
sub computeflightsrange () {
- if (!$flightlimit) {
- my $flagscond =
- '('.join(' OR ', map { "f.hostflag = 'blessed-$_'" } @blessings).')';
- my $nhostsq = db_prepare(<<END);
- SELECT count(*)
- FROM resources r
- WHERE restype='host'
- AND EXISTS (SELECT 1
- FROM hostflags f
- WHERE f.hostname=r.resname
- AND $flagscond)
+ if ($flightlimit) {
+ my $minflightsq = db_prepare(<<END);
+ SELECT flight
+ FROM (
+ SELECT flight
+ FROM flights
+ WHERE $restrictflight_cond
+ ORDER BY flight DESC
+ LIMIT $flightlimit
+ ) f
+ ORDER BY flight ASC
+ LIMIT 1
END
- $nhostsq->execute();
- my ($nhosts) = $nhostsq->fetchrow_array();
- print DEBUG "COUNTED $nhosts hosts\n";
- $flightlimit = $nhosts * $limit * 2;
- }
-
- my $minflightsq = db_prepare(<<END);
- SELECT flight
- FROM (
+ $minflightsq->execute();
+ ($minflight,) = $minflightsq->fetchrow_array();
+ } else {
+ my $minflightsq = db_prepare(<<END);
SELECT flight
- FROM flights
- WHERE $restrictflight_cond
- ORDER BY flight DESC
- LIMIT $flightlimit
- ) f
- ORDER BY flight ASC
- LIMIT 1
+ FROM flights
+ WHERE started >= ?
+ ORDER BY flight ASC
+ LIMIT 1
END
- $minflightsq->execute();
- ($minflight,) = $minflightsq->fetchrow_array();
+ my $now = time // die $!;
+ $minflightsq->execute($now - $timelimit);
+ ($minflight,) = $minflightsq->fetchrow_array();
+ }
$minflight //= 0;
$flightcond = "(flight > $minflight)";
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 22/41] sg-report-host-history: Drop per-job debug etc.
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (20 preceding siblings ...)
2020-07-31 11:38 ` [OSSTEST PATCH v2 21/41] sg-report-host-history: Find flight limit by flight start date Ian Jackson
@ 2020-07-31 11:38 ` Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 23/41] Executive: Export opendb_tests Ian Jackson
` (18 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:38 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
This printing has a significant effect on the performance of this
program, at least after we optimise various other things.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
sg-report-host-history | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/sg-report-host-history b/sg-report-host-history
index a159df3e..a34458e0 100755
--- a/sg-report-host-history
+++ b/sg-report-host-history
@@ -102,9 +102,9 @@ sub read_existing_logs ($) {
my $k = $1;
s{\%([0-9a-f]{2})}{ chr hex $1 }ge;
$ch->{$k} = $_;
- print DEBUG "GOTCACHE $hostname $k\n";
+# print DEBUG "GOTCACHE $hostname $k\n";
}
- print DEBUG "GOTCACHE $hostname \@ $jr->{flight} $jr->{job} $jr->{status},$jr->{name}\n";
+# print DEBUG "GOTCACHE $hostname \@ $jr->{flight} $jr->{job} $jr->{status},$jr->{name}\n";
$tcache->{$jr->{flight},$jr->{job},$jr->{status},$jr->{name}} = $jr;
}
close H;
@@ -272,7 +272,7 @@ END
my @rows;
my $cachehits = 0;
foreach my $jr (@$inrows) {
- print DEBUG "JOB $jr->{flight}.$jr->{job} ";
+ #print DEBUG "JOB $jr->{flight}.$jr->{job} ";
my $cacherow =
$tcache->{$jr->{flight},$jr->{job},$jr->{status},$jr->{name}};
@@ -283,11 +283,11 @@ END
my $endedrow = jobquery($endedq, $jr, 'e');
if (!$endedrow) {
- print DEBUG "no-finished\n";
+ #print DEBUG "no-finished\n";
next;
}
- print DEBUG join " ", map { $endedrow->{$_} } sort keys %$endedrow;
- print DEBUG ".\n";
+ #print DEBUG join " ", map { $endedrow->{$_} } sort keys %$endedrow;
+ #print DEBUG ".\n";
push @rows, { %$jr, %$endedrow };
}
@@ -329,7 +329,7 @@ END
next;
}
- print DEBUG "JR $jr->{flight}.$jr->{job}\n";
+ #print DEBUG "JR $jr->{flight}.$jr->{job}\n";
my $ir = jobquery($infoq, $jr, 'i');
my $ar = jobquery($allocdq, $jr, 'a');
my $ident = $jr->{name};
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 23/41] Executive: Export opendb_tests
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (21 preceding siblings ...)
2020-07-31 11:38 ` [OSSTEST PATCH v2 22/41] sg-report-host-history: Drop per-job debug etc Ian Jackson
@ 2020-07-31 11:38 ` Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 24/41] sg-report-host-history: Add a debug print after sorting jobs Ian Jackson
` (17 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:38 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
sg-report-host-history is going to want this in a moment
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Osstest/Executive.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index 2f81e89d..8e4c5b9a 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -49,7 +49,7 @@ BEGIN {
task_spec_desc findtask findtask_spec @all_lock_tables
restrictflight_arg restrictflight_cond
report_run_getinfo report_altcolour
- report_altchangecolour
+ report_altchangecolour opendb_tests
report_blessingscond report_find_push_age_info
tcpconnect_queuedaemon plan_search
manual_allocation_base_jobinfo
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 24/41] sg-report-host-history: Add a debug print after sorting jobs
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (22 preceding siblings ...)
2020-07-31 11:38 ` [OSSTEST PATCH v2 23/41] Executive: Export opendb_tests Ian Jackson
@ 2020-07-31 11:38 ` Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 25/41] sg-report-host-history: Do the main query per host Ian Jackson
` (16 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:38 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
This helps rule this sort out as a source of slowness.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
sg-report-host-history | 2 ++
1 file changed, 2 insertions(+)
diff --git a/sg-report-host-history b/sg-report-host-history
index a34458e0..1c2d19ae 100755
--- a/sg-report-host-history
+++ b/sg-report-host-history
@@ -318,6 +318,8 @@ END
@rows = sort { $b->{finished} <=> $a->{finished} } @rows;
+ print DEBUG "SORTED\n";
+
my $alternate = 0;
my $wrote = 0;
my $runvarq_hits = 0;
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 25/41] sg-report-host-history: Do the main query per host
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (23 preceding siblings ...)
2020-07-31 11:38 ` [OSSTEST PATCH v2 24/41] sg-report-host-history: Add a debug print after sorting jobs Ian Jackson
@ 2020-07-31 11:38 ` Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 26/41] sg-report-host-history: Rerganisation: Make mainquery per-host Ian Jackson
` (15 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:38 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
In f6001d628c3b3fd42b10cd15351981a04bc02572 we combined these
queries into one:
sg-report-host-history: Aggregate runvars query for all hosts
Now that we have an index, there is a faster way for the db to do this
query: via that index. But it doesn't like to do that if be aggregate
the queries. Experimentally, doing this query separately once per
host is significantly faster.
Also, later, it will allow us to parallelise this work.
So, we undo that. (Not by reverting, though.)
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
v2: Use proper \ escaping for underscores in LIKE
---
schema/runvars-host-index.sql | 2 +-
sg-report-host-history | 27 +++++++++------------------
2 files changed, 10 insertions(+), 19 deletions(-)
diff --git a/schema/runvars-host-index.sql b/schema/runvars-host-index.sql
index 222a0a30..6a3ef377 100644
--- a/schema/runvars-host-index.sql
+++ b/schema/runvars-host-index.sql
@@ -1,4 +1,4 @@
--- ##OSSTEST## 009 Preparatory
+-- ##OSSTEST## 009 Needed
--
-- This index helps sg-report-host-history find relevant flights.
diff --git a/sg-report-host-history b/sg-report-host-history
index 1c2d19ae..15866ab6 100755
--- a/sg-report-host-history
+++ b/sg-report-host-history
@@ -165,34 +165,25 @@ sub jobquery ($$$) {
our %hosts;
sub mainquery () {
- our $valcond = join " OR ", map { "val = ?" } keys %hosts;
- our @params = keys %hosts;
-
our $runvarq //= db_prepare(<<END);
- SELECT flight, job, name, val, status
+ SELECT flight, job, name, status
FROM runvars
JOIN jobs USING (flight, job)
- WHERE $namecond
- AND ($valcond)
+ WHERE (name = 'host' OR name LIKE '%\_host')
+ AND val = ?
AND $flightcond
AND $restrictflight_cond
AND flight > ?
ORDER BY flight DESC
- LIMIT ($limit * 3 + 100) * ?
+ LIMIT $limit * 2
END
+ foreach my $host (sort keys %hosts) {
+ print DEBUG "MAINQUERY $host...\n";
+ $runvarq->execute($host, $minflight);
- push @params, $minflight;
- push @params, scalar keys %hosts;
-
- print DEBUG "MAINQUERY...\n";
- $runvarq->execute(@params);
-
- print DEBUG "FIRST PASS\n";
- while (my $jr= $runvarq->fetchrow_hashref()) {
- print DEBUG " $jr->{flight}.$jr->{job} ";
- push @{ $hosts{$jr->{val}} }, $jr;
+ $hosts{$host} = $runvarq->fetchall_arrayref({});
+ print DEBUG "MAINQUERY $host got ".(scalar @{ $hosts{$host} })."\n";
}
- print DEBUG "\n";
}
sub reporthost ($) {
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 26/41] sg-report-host-history: Rerganisation: Make mainquery per-host
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (24 preceding siblings ...)
2020-07-31 11:38 ` [OSSTEST PATCH v2 25/41] sg-report-host-history: Do the main query per host Ian Jackson
@ 2020-07-31 11:38 ` Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 27/41] sg-report-host-history: Rerganisation: Read old logs later Ian Jackson
` (14 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:38 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
This moves the loop over hosts into the main program. We are working
our way to a new code structure.
No functional change.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
sg-report-host-history | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/sg-report-host-history b/sg-report-host-history
index 15866ab6..34216aa2 100755
--- a/sg-report-host-history
+++ b/sg-report-host-history
@@ -164,7 +164,9 @@ sub jobquery ($$$) {
our %hosts;
-sub mainquery () {
+sub mainquery ($) {
+ my ($host) = @_;
+
our $runvarq //= db_prepare(<<END);
SELECT flight, job, name, status
FROM runvars
@@ -177,13 +179,12 @@ sub mainquery () {
ORDER BY flight DESC
LIMIT $limit * 2
END
- foreach my $host (sort keys %hosts) {
- print DEBUG "MAINQUERY $host...\n";
- $runvarq->execute($host, $minflight);
- $hosts{$host} = $runvarq->fetchall_arrayref({});
- print DEBUG "MAINQUERY $host got ".(scalar @{ $hosts{$host} })."\n";
- }
+ print DEBUG "MAINQUERY $host...\n";
+ $runvarq->execute($host, $minflight);
+
+ $hosts{$host} = $runvarq->fetchall_arrayref({});
+ print DEBUG "MAINQUERY $host got ".(scalar @{ $hosts{$host} })."\n";
}
sub reporthost ($) {
@@ -474,7 +475,9 @@ db_retry($dbh_tests, [], sub {
});
db_retry($dbh_tests, [], sub {
- mainquery();
+ foreach my $host (sort keys %hosts) {
+ mainquery($host);
+ }
});
foreach my $host (sort keys %hosts) {
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 27/41] sg-report-host-history: Rerganisation: Read old logs later
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (25 preceding siblings ...)
2020-07-31 11:38 ` [OSSTEST PATCH v2 26/41] sg-report-host-history: Rerganisation: Make mainquery per-host Ian Jackson
@ 2020-07-31 11:38 ` Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 28/41] sg-report-host-history: Rerganisation: Change loops Ian Jackson
` (13 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:38 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
Perhaps at one point something read from these logs influenced the db
query for thye flights range, but that is no longer the case and it
doesn't seem likely to need to come back.
We want to move the per-host stuff together.
No functional change.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
sg-report-host-history | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/sg-report-host-history b/sg-report-host-history
index 34216aa2..3f4670e5 100755
--- a/sg-report-host-history
+++ b/sg-report-host-history
@@ -466,14 +466,14 @@ END
exit 0 unless %hosts;
-foreach (keys %hosts) {
- read_existing_logs($_);
-}
-
db_retry($dbh_tests, [], sub {
computeflightsrange();
});
+foreach (keys %hosts) {
+ read_existing_logs($_);
+}
+
db_retry($dbh_tests, [], sub {
foreach my $host (sort keys %hosts) {
mainquery($host);
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 28/41] sg-report-host-history: Rerganisation: Change loops
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (26 preceding siblings ...)
2020-07-31 11:38 ` [OSSTEST PATCH v2 27/41] sg-report-host-history: Rerganisation: Read old logs later Ian Jackson
@ 2020-07-31 11:38 ` Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 29/41] sg-report-host-history: Drop a redundznt AND clause Ian Jackson
` (12 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:38 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
Move the per-host code all into the same per-host loop. One effect is
to transpose the db_retry and host loops for mainquery.
No functional change.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
sg-report-host-history | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/sg-report-host-history b/sg-report-host-history
index 3f4670e5..2ca0e235 100755
--- a/sg-report-host-history
+++ b/sg-report-host-history
@@ -470,18 +470,10 @@ db_retry($dbh_tests, [], sub {
computeflightsrange();
});
-foreach (keys %hosts) {
- read_existing_logs($_);
-}
-
-db_retry($dbh_tests, [], sub {
- foreach my $host (sort keys %hosts) {
- mainquery($host);
- }
-});
-
foreach my $host (sort keys %hosts) {
+ read_existing_logs($host);
db_retry($dbh_tests, [], sub {
+ mainquery($host);
reporthost $host;
});
}
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 29/41] sg-report-host-history: Drop a redundznt AND clause
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (27 preceding siblings ...)
2020-07-31 11:38 ` [OSSTEST PATCH v2 28/41] sg-report-host-history: Rerganisation: Change loops Ian Jackson
@ 2020-07-31 11:38 ` Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 30/41] sg-report-host-history: Fork Ian Jackson
` (11 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:38 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
This condition is the same as $flightcond. (This has no effect on the
db performance since the query planner figures it out, but it is
confusing.)
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
sg-report-host-history | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sg-report-host-history b/sg-report-host-history
index 2ca0e235..f4352fc3 100755
--- a/sg-report-host-history
+++ b/sg-report-host-history
@@ -175,13 +175,12 @@ sub mainquery ($) {
AND val = ?
AND $flightcond
AND $restrictflight_cond
- AND flight > ?
ORDER BY flight DESC
LIMIT $limit * 2
END
print DEBUG "MAINQUERY $host...\n";
- $runvarq->execute($host, $minflight);
+ $runvarq->execute($host);
$hosts{$host} = $runvarq->fetchall_arrayref({});
print DEBUG "MAINQUERY $host got ".(scalar @{ $hosts{$host} })."\n";
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 30/41] sg-report-host-history: Fork
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (28 preceding siblings ...)
2020-07-31 11:38 ` [OSSTEST PATCH v2 29/41] sg-report-host-history: Drop a redundznt AND clause Ian Jackson
@ 2020-07-31 11:38 ` Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 31/41] schema: Add index to help cs-bisection-step Ian Jackson
` (10 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:38 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
Run each host's report in a separate child. This is considerably
faster.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
sg-report-host-history | 47 +++++++++++++++++++++++++++++++++++-------
1 file changed, 40 insertions(+), 7 deletions(-)
diff --git a/sg-report-host-history b/sg-report-host-history
index f4352fc3..dc694ebe 100755
--- a/sg-report-host-history
+++ b/sg-report-host-history
@@ -34,6 +34,7 @@ our $flightlimit;
our $htmlout = ".";
our $read_existing=1;
our $doinstall=1;
+our $maxjobs=10;
our @blessings;
open DEBUG, ">/dev/null";
@@ -44,7 +45,7 @@ csreadconfig();
while (@ARGV && $ARGV[0] =~ m/^-/) {
$_= shift @ARGV;
last if m/^--?$/;
- if (m/^--(limit)\=([1-9]\d*)$/) {
+ if (m/^--(limit|maxjobs)\=([1-9]\d*)$/) {
$$1= $2;
} elsif (m/^--time-limit\=([1-9]\d*)$/) {
$timelimit= $1;
@@ -469,12 +470,44 @@ db_retry($dbh_tests, [], sub {
computeflightsrange();
});
+undef $dbh_tests;
+
+our %children;
+our $worst = 0;
+
+sub wait_for_max_children ($) {
+ my ($lim) = @_;
+ while (keys(%children) > $lim) {
+ $!=0; $?=0; my $got = wait;
+ die "$! $got $?" unless exists $children{$got};
+ my $host = $children{$got};
+ delete $children{$got};
+ $worst = $? if $? > $worst;
+ if ($?) {
+ print STDERR "sg-report-flight[: [$got] failed for $host: $?\n";
+ } else {
+ print DEBUG "REAPED [$got] $host\n";
+ }
+ }
+}
+
foreach my $host (sort keys %hosts) {
- read_existing_logs($host);
- db_retry($dbh_tests, [], sub {
- mainquery($host);
- reporthost $host;
- });
+ wait_for_max_children($maxjobs);
+
+ my $pid = fork // die $!;
+ if (!$pid) {
+ opendb_tests();
+ read_existing_logs($host);
+ db_retry($dbh_tests, [], sub {
+ mainquery($host);
+ reporthost $host;
+ });
+ print DEBUG "JQ CACHE ".($jqtotal-$jqcachemisses)." / $jqtotal\n";
+ exit(0);
+ }
+ print DEBUG "SPAWNED [$pid] $host\n";
+ $children{$pid} = $host;
}
-print DEBUG "JQ CACHE ".($jqtotal-$jqcachemisses)." / $jqtotal\n";
+wait_for_max_children(0);
+exit $worst;
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 31/41] schema: Add index to help cs-bisection-step
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (29 preceding siblings ...)
2020-07-31 11:38 ` [OSSTEST PATCH v2 30/41] sg-report-host-history: Fork Ian Jackson
@ 2020-07-31 11:38 ` Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 32/41] adhoc-revtuple-generator: Fix an undef warning in a debug print Ian Jackson
` (9 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:38 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
cs-bisection step basis search involves looking for recent flights
that weren't broken. A flight is broken if it has broken steps.
Make an index for this to save it scanning the steps table.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
schema/steps-broken-index.sql | 7 +++++++
1 file changed, 7 insertions(+)
create mode 100644 schema/steps-broken-index.sql
diff --git a/schema/steps-broken-index.sql b/schema/steps-broken-index.sql
new file mode 100644
index 00000000..770747cc
--- /dev/null
+++ b/schema/steps-broken-index.sql
@@ -0,0 +1,7 @@
+-- ##OSSTEST## 010 Harmless
+--
+-- This index helps cs-bisection-flight check if flighss are broken.
+
+CREATE INDEX steps_broken_idx
+ ON steps (flight)
+ WHERE status='broken';
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 32/41] adhoc-revtuple-generator: Fix an undef warning in a debug print
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (30 preceding siblings ...)
2020-07-31 11:38 ` [OSSTEST PATCH v2 31/41] schema: Add index to help cs-bisection-step Ian Jackson
@ 2020-07-31 11:38 ` Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 33/41] cs-bisection-step: Generalise qtxt_common_rev_ok Ian Jackson
` (8 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:38 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
$parents might be undef here.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
New in v2.
---
adhoc-revtuple-generator | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/adhoc-revtuple-generator b/adhoc-revtuple-generator
index c8d6f4ad..ec33305a 100755
--- a/adhoc-revtuple-generator
+++ b/adhoc-revtuple-generator
@@ -463,7 +463,7 @@ sub coalesce {
$out->{$node}{Date}= $explode_date;
my $parents= $graphs[$explode_i]{ $node[$explode_i] }{Parents};
print DEBUG "#$explode_i $explode_isearliest".
- " $explode_date x".scalar(@$parents)."\n";
+ " $explode_date x".($parents ? scalar(@$parents) : "-")."\n";
foreach my $subparent (@$parents) {
$node[$explode_i]= $subparent;
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 33/41] cs-bisection-step: Generalise qtxt_common_rev_ok
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (31 preceding siblings ...)
2020-07-31 11:38 ` [OSSTEST PATCH v2 32/41] adhoc-revtuple-generator: Fix an undef warning in a debug print Ian Jackson
@ 2020-07-31 11:38 ` Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 34/41] cs-bisection-step: Move an AND Ian Jackson
` (7 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:38 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
* Make it into a subref which takes a $table argument.
* Change the two references into function calls using the @{...} syntax
* Move the definition earlier in the file
No change to the generated query.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
v2: New patch.
---
cs-bisection-step | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/cs-bisection-step b/cs-bisection-step
index 9a0fee39..5d4e179e 100755
--- a/cs-bisection-step
+++ b/cs-bisection-step
@@ -182,6 +182,14 @@ END
sub flight_rmap ($$) {
my ($flight, $need_urls) = @_;
+ my $qtxt_common_rev_ok = sub {
+ my ($table) = @_;
+ [<<END];
+ ($table.name LIKE E'built\\_revision\\_%' OR
+ $table.name LIKE E'revision\\_%')
+END
+ };
+
$dbh_tests->do(<<END, {});
CREATE TEMP TABLE tmp_build_info (
use varchar NOT NULL,
@@ -236,10 +244,6 @@ END
my $qtxt_common_tables = <<END;
FROM tmp_build_info AS rev
END
- my $qtxt_common_rev_condition = <<END;
- (rev.name LIKE E'built\\_revision\\_%' OR
- rev.name LIKE E'revision\\_%')
-END
my $sth= db_prepare(!$need_urls ? <<END_NOURLS : <<END_URLS);
SELECT
@@ -249,7 +253,7 @@ $qtxt_common_results
$qtxt_common_tables
WHERE
-$qtxt_common_rev_condition
+@{ $qtxt_common_rev_ok->('rev') }
ORDER by rev.name;
@@ -262,7 +266,7 @@ $qtxt_common_tables
CROSS JOIN tmp_build_info AS url
WHERE
-$qtxt_common_rev_condition
+@{ $qtxt_common_rev_ok->('rev') }
AND url.name LIKE E'tree\\_%'
AND url.use = rev.use
AND url.job = rev.job
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 34/41] cs-bisection-step: Move an AND
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (32 preceding siblings ...)
2020-07-31 11:38 ` [OSSTEST PATCH v2 33/41] cs-bisection-step: Generalise qtxt_common_rev_ok Ian Jackson
@ 2020-07-31 11:38 ` Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 35/41] cs-bisection-step: Break out qtxt_common_ok Ian Jackson
` (6 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:38 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
This obviously-fine change makes the next commit easier to review.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
v2: New patch.
---
cs-bisection-step | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/cs-bisection-step b/cs-bisection-step
index 5d4e179e..f11726aa 100755
--- a/cs-bisection-step
+++ b/cs-bisection-step
@@ -266,8 +266,8 @@ $qtxt_common_tables
CROSS JOIN tmp_build_info AS url
WHERE
-@{ $qtxt_common_rev_ok->('rev') }
- AND url.name LIKE E'tree\\_%'
+@{ $qtxt_common_rev_ok->('rev') } AND
+ url.name LIKE E'tree\\_%'
AND url.use = rev.use
AND url.job = rev.job
AND (rev.name = 'built_revision_' || substr(url.name,6) OR
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 35/41] cs-bisection-step: Break out qtxt_common_ok
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (33 preceding siblings ...)
2020-07-31 11:38 ` [OSSTEST PATCH v2 34/41] cs-bisection-step: Move an AND Ian Jackson
@ 2020-07-31 11:38 ` Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 36/41] cs-bisection-step: Use db_prepare a few times instead of ->do Ian Jackson
` (5 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:38 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
Make this bit of query into a subref which takes a $table argument.
No change to the generated query.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
v2: New patch.
---
cs-bisection-step | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/cs-bisection-step b/cs-bisection-step
index f11726aa..ba0c6424 100755
--- a/cs-bisection-step
+++ b/cs-bisection-step
@@ -190,6 +190,13 @@ sub flight_rmap ($$) {
END
};
+ my $qtxt_common_tree_ok = sub {
+ my ($table) = @_;
+ [<<END];
+ $table.name LIKE E'tree\\_%'
+END
+ };
+
$dbh_tests->do(<<END, {});
CREATE TEMP TABLE tmp_build_info (
use varchar NOT NULL,
@@ -267,7 +274,7 @@ $qtxt_common_tables
WHERE
@{ $qtxt_common_rev_ok->('rev') } AND
- url.name LIKE E'tree\\_%'
+@{ $qtxt_common_tree_ok->('url') }
AND url.use = rev.use
AND url.job = rev.job
AND (rev.name = 'built_revision_' || substr(url.name,6) OR
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 36/41] cs-bisection-step: Use db_prepare a few times instead of ->do
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (34 preceding siblings ...)
2020-07-31 11:38 ` [OSSTEST PATCH v2 35/41] cs-bisection-step: Break out qtxt_common_ok Ian Jackson
@ 2020-07-31 11:38 ` Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 37/41] cs-bisection-step: temporary table: Insert only rows we care about Ian Jackson
` (4 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:38 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
With $dbh_tests->do(...), we can only get a debug trace of the queries
by using DBI_TRACE which produces voluminous output. Using our own
db_prepare invokes our own debugging.
No functional change.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
v2: New patch.
---
cs-bisection-step | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/cs-bisection-step b/cs-bisection-step
index ba0c6424..1c165b78 100755
--- a/cs-bisection-step
+++ b/cs-bisection-step
@@ -197,7 +197,7 @@ END
END
};
- $dbh_tests->do(<<END, {});
+ db_prepare(<<END)->execute();
CREATE TEMP TABLE tmp_build_info (
use varchar NOT NULL,
name varchar NOT NULL,
@@ -206,7 +206,7 @@ END
)
END
- $dbh_tests->do(<<END, {}, $job, $flight);
+ db_prepare(<<END)->execute($job, $flight);
INSERT INTO tmp_build_info
SELECT t.name AS use,
@@ -230,7 +230,7 @@ END
END)
END
- $dbh_tests->do(<<END, {}, $job, $flight);
+ db_prepare(<<END)->execute($job, $flight);
INSERT INTO tmp_build_info
SELECT '' AS use,
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 37/41] cs-bisection-step: temporary table: Insert only rows we care about
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (35 preceding siblings ...)
2020-07-31 11:38 ` [OSSTEST PATCH v2 36/41] cs-bisection-step: Use db_prepare a few times instead of ->do Ian Jackson
@ 2020-07-31 11:38 ` Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 38/41] SQL: Change LIKE E'...\\_...' to LIKE '...\_...' Ian Jackson
` (3 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:38 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
Every use of this table has a WHERE or ON which invokes at least one
of these conditions. So put only those rows into the table.
This provides a significant speedup (which I haven't properly
measured).
No overall functional change.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
v2: New patch.
---
cs-bisection-step | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/cs-bisection-step b/cs-bisection-step
index 1c165b78..718c87b0 100755
--- a/cs-bisection-step
+++ b/cs-bisection-step
@@ -219,7 +219,9 @@ END
WHERE t.job = ?
AND t.flight = ?
- AND t.name LIKE '%buildjob'
+ AND t.name LIKE '%buildjob' AND
+(@{ $qtxt_common_rev_ok->('b') } OR
+ @{ $qtxt_common_tree_ok->('b') })
AND b.flight = (CASE WHEN t.val NOT LIKE '%.%'
THEN t.flight
ELSE cast(split_part(t.val, '.', 1) AS int)
@@ -239,7 +241,9 @@ END
job AS job
FROM runvars
WHERE job = ?
- AND flight = ?
+ AND flight = ? AND
+(@{ $qtxt_common_rev_ok->('runvars') } OR
+ @{ $qtxt_common_tree_ok->('runvars') })
END
my $qtxt_common_results = <<END;
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 38/41] SQL: Change LIKE E'...\\_...' to LIKE '...\_...'
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (36 preceding siblings ...)
2020-07-31 11:38 ` [OSSTEST PATCH v2 37/41] cs-bisection-step: temporary table: Insert only rows we care about Ian Jackson
@ 2020-07-31 11:38 ` Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 39/41] cs-bisection-step: Add a debug print when we run dot(1) Ian Jackson
` (2 subsequent siblings)
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:38 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
E'...' means to interpret \-escapes. But we don't want them: without
E, we can avoid some toothpick-doubling.
No functional change.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
v2: New patch.
---
cs-bisection-step | 8 ++++----
sg-report-job-history | 4 ++--
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/cs-bisection-step b/cs-bisection-step
index 718c87b0..a82cbfb8 100755
--- a/cs-bisection-step
+++ b/cs-bisection-step
@@ -185,15 +185,15 @@ sub flight_rmap ($$) {
my $qtxt_common_rev_ok = sub {
my ($table) = @_;
[<<END];
- ($table.name LIKE E'built\\_revision\\_%' OR
- $table.name LIKE E'revision\\_%')
+ ($table.name LIKE 'built\_revision\_%' OR
+ $table.name LIKE 'revision\_%')
END
};
my $qtxt_common_tree_ok = sub {
my ($table) = @_;
[<<END];
- $table.name LIKE E'tree\\_%'
+ $table.name LIKE 'tree\_%'
END
};
@@ -1220,7 +1220,7 @@ sub preparejob ($$$$) {
INTO TEMP bisection_runvars
FROM runvars
WHERE flight=? AND job=? AND synth='f'
- AND name NOT LIKE E'revision\\_%'
+ AND name NOT LIKE 'revision\_%'
AND name NOT LIKE '%host'
END
my (@trevisions) = split / /, $choose->{Rtuple};
diff --git a/sg-report-job-history b/sg-report-job-history
index d5f91ff1..22a28627 100755
--- a/sg-report-job-history
+++ b/sg-report-job-history
@@ -92,7 +92,7 @@ if (defined($flight)) {
our $revisionsq= db_prepare(<<END);
SELECT * FROM runvars
WHERE flight=? AND job=?
- AND name LIKE E'built\\_revision\\_\%'
+ AND name LIKE 'built\_revision\_%'
END
# (We report on non-main-revision jobs just as for main-revision ones.)
@@ -109,7 +109,7 @@ sub add_revisions ($$$$) {
our $buildsq= db_prepare(<<END);
SELECT * FROM runvars
WHERE flight=? AND job=?
- AND name LIKE E'\%buildjob'
+ AND name LIKE '%buildjob'
END
sub processjobbranch ($$) {
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 39/41] cs-bisection-step: Add a debug print when we run dot(1)
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (37 preceding siblings ...)
2020-07-31 11:38 ` [OSSTEST PATCH v2 38/41] SQL: Change LIKE E'...\\_...' to LIKE '...\_...' Ian Jackson
@ 2020-07-31 11:38 ` Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 40/41] cs-bisection-step: Lay out the revision tuple graph once Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 41/41] duration_estimator: Clarify recentflights query a bit Ian Jackson
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:38 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
Amongst other things this was useful for perf investigation.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
v2: New patch.
---
cs-bisection-step | 1 +
1 file changed, 1 insertion(+)
diff --git a/cs-bisection-step b/cs-bisection-step
index a82cbfb8..027032a1 100755
--- a/cs-bisection-step
+++ b/cs-bisection-step
@@ -1114,6 +1114,7 @@ END
if (eval {
foreach my $fmt (qw(ps png svg)) {
+ print DEBUG "RUNNING dot -T$fmt\n";
system_checked("dot", "-T$fmt", "-o$graphfile.$fmt",
"$graphfile.dot");
}
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 40/41] cs-bisection-step: Lay out the revision tuple graph once
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (38 preceding siblings ...)
2020-07-31 11:38 ` [OSSTEST PATCH v2 39/41] cs-bisection-step: Add a debug print when we run dot(1) Ian Jackson
@ 2020-07-31 11:38 ` Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 41/41] duration_estimator: Clarify recentflights query a bit Ian Jackson
40 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:38 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
The graph layout algorithm is not very fast, particularly if the
revision graph is big. In my test case this saves about 10s.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
v2: New patch.
---
cs-bisection-step | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/cs-bisection-step b/cs-bisection-step
index 027032a1..8544bac0 100755
--- a/cs-bisection-step
+++ b/cs-bisection-step
@@ -1113,10 +1113,15 @@ END
or die "$!";
if (eval {
+ print DEBUG "RUNNING dot -Txdot\n";
+ system_checked("dot", "-Txdot", "-o$graphfile.xdot",
+ "$graphfile.dot");
foreach my $fmt (qw(ps png svg)) {
- print DEBUG "RUNNING dot -T$fmt\n";
- system_checked("dot", "-T$fmt", "-o$graphfile.$fmt",
- "$graphfile.dot");
+ # neato rather than dot, because neato just uses positions
+ # etc. in the input whereas dot does (re)calculation work.
+ print DEBUG "RUNNING neato -n2 -T$fmt\n";
+ system_checked("neato", "-n2", "-T$fmt", "-o$graphfile.$fmt",
+ "$graphfile.xdot");
}
open SVGI, "$graphfile.svg" or die "$graphfile.svg $!";
open SVGO, ">", "$graphfile.svg.new" or die "$graphfile.svg.new $!";
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [OSSTEST PATCH v2 41/41] duration_estimator: Clarify recentflights query a bit
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
` (39 preceding siblings ...)
2020-07-31 11:38 ` [OSSTEST PATCH v2 40/41] cs-bisection-step: Lay out the revision tuple graph once Ian Jackson
@ 2020-07-31 11:38 ` Ian Jackson
2020-07-31 14:04 ` George Dunlap
40 siblings, 1 reply; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 11:38 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, George Dunlap
The condition on r.job is more naturally thought of as a join
condition than a where condition. (This is an inner join, so the
semantics are identical.)
Also, for clarity, swap the flight and job conditions round, so that
the ON clause is a series of r.thing = otherthing.
No functional change.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
CC: George Dunlap <George.Dunlap@citrix.com>
---
v2: New patch.
---
Osstest/Executive.pm | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index 8e4c5b9a..a69c624f 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -1153,10 +1153,10 @@ sub duration_estimator ($$;$$) {
FROM flights f
JOIN jobs j USING (flight)
JOIN runvars r
- ON f.flight=r.flight
+ ON r.flight=f.flight
+ AND r.job=j.job=
AND r.name=?
- WHERE j.job=r.job
- AND f.blessing=?
+ WHERE f.blessing=?
AND f.branch=?
AND j.job=?
AND r.val=?
--
2.20.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [OSSTEST PATCH v2 41/41] duration_estimator: Clarify recentflights query a bit
2020-07-31 11:38 ` [OSSTEST PATCH v2 41/41] duration_estimator: Clarify recentflights query a bit Ian Jackson
@ 2020-07-31 14:04 ` George Dunlap
0 siblings, 0 replies; 48+ messages in thread
From: George Dunlap @ 2020-07-31 14:04 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel
> On Jul 31, 2020, at 12:38 PM, Ian Jackson <ian.jackson@eu.citrix.com> wrote:
>
> The condition on r.job is more naturally thought of as a join
> condition than a where condition. (This is an inner join, so the
> semantics are identical.)
>
> Also, for clarity, swap the flight and job conditions round, so that
> the ON clause is a series of r.thing = otherthing.
>
> No functional change.
>
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [OSSTEST PATCH v2 08/41] sg-report-flight: Ask the db for flights of interest
2020-07-31 11:37 ` [OSSTEST PATCH v2 08/41] sg-report-flight: Ask the db for flights of interest Ian Jackson
@ 2020-07-31 14:17 ` George Dunlap
2020-07-31 15:43 ` Ian Jackson
0 siblings, 1 reply; 48+ messages in thread
From: George Dunlap @ 2020-07-31 14:17 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel
> On Jul 31, 2020, at 12:37 PM, Ian Jackson <ian.jackson@eu.citrix.com> wrote:
>
> Specifically, we narrow the initial query to flights which have at
> least some job with the built_revision_foo we are looking for.
>
> This condition is strictly broader than that implemented inside the
> flight search loop, so there is no functional change.
Assuming this is true, that job / runvar is filtered after extracting this information, then...
>
> Perf: runtime of my test case now ~300s-500s.
>
> Example query before (from the Perl DBI trace):
>
> SELECT * FROM (
> SELECT flight, blessing FROM flights
> WHERE (branch='xen-unstable')
> AND EXISTS (SELECT 1
> FROM jobs
> WHERE jobs.flight = flights.flight
> AND jobs.job = ?)
>
> AND ( (TRUE AND flight <= 151903) AND (blessing='real') )
> ORDER BY flight DESC
> LIMIT 1000
> ) AS sub
> ORDER BY blessing ASC, flight DESC
>
> With these bind variables:
>
> "test-armhf-armhf-libvirt"
>
> After:
>
> SELECT * FROM (
> SELECT DISTINCT flight, blessing
> FROM flights
> JOIN runvars r1 USING (flight)
>
> WHERE (branch='xen-unstable')
> AND ( (TRUE AND flight <= 151903) AND (blessing='real') )
> AND EXISTS (SELECT 1
> FROM jobs
> WHERE jobs.flight = flights.flight
> AND jobs.job = ?)
>
> AND r1.name LIKE 'built\_revision\_%'
> AND r1.name = ?
> AND r1.val= ?
>
> ORDER BY flight DESC
> LIMIT 1000
> ) AS sub
> ORDER BY blessing ASC, flight DESC
…I agree that this shoud introduce no other changes.
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [OSSTEST PATCH v2 19/41] Executive: Drop redundant AND clause
2020-07-31 11:37 ` [OSSTEST PATCH v2 19/41] Executive: Drop redundant AND clause Ian Jackson
@ 2020-07-31 14:17 ` George Dunlap
0 siblings, 0 replies; 48+ messages in thread
From: George Dunlap @ 2020-07-31 14:17 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel
> On Jul 31, 2020, at 12:37 PM, Ian Jackson <ian.jackson@eu.citrix.com> wrote:
>
> In "Executive: Use index for report__find_test" we changed an EXISTS
> subquery into a JOIN.
>
> Now, the condition r.flight=f.flight is redundant because this is the
> join column (from USING).
>
> No functional change.
>
> CC: George Dunlap <George.Dunlap@citrix.com>
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [OSSTEST PATCH v2 07/41] schema: Provide indices for sg-report-flight
2020-07-31 11:37 ` [OSSTEST PATCH v2 07/41] schema: Provide indices for sg-report-flight Ian Jackson
@ 2020-07-31 14:21 ` George Dunlap
2020-07-31 14:55 ` Ian Jackson
0 siblings, 1 reply; 48+ messages in thread
From: George Dunlap @ 2020-07-31 14:21 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel
> On Jul 31, 2020, at 12:37 PM, Ian Jackson <ian.jackson@eu.citrix.com> wrote:
>
> These indexes allow very fast lookup of "relevant" flights eg when
> trying to justify failures.
>
> In my ad-hoc test case, these indices (along with the subsequent
> changes to sg-report-flight and Executive.pm, reduce the runtime of
> sg-report-flight from 2-3ks (unacceptably long!) to as little as
> 5-7s seconds - a speedup of about 500x.
>
> (Getting the database snapshot may take a while first, but deploying
> this code should help with that too by reducing long-running
> transactions. Quoted perf timings are from snapshot acquisition.)
>
> Without these new indexes there may be a performance change from the
> query changes. I haven't benchmarked this so I am setting the schema
> updates to be Preparatory/Needed (ie, "Schema first" as
> schema/README.updates has it), to say that the index should be created
> before the new code is deployed.
>
> Testing: I have tested this series by creating experimental indices
> "trial_..." in the actual production instance. (Transactional DDL was
> very helpful with this.) I have verified with \d that schema update
> instructions in this commit generate indexes which are equivalent to
> the trial indices.
>
> Deployment: AFter these schema updates are applied, the trial indices
> are redundant duplicates and should be deleted.
>
> CC: George Dunlap <George.Dunlap@citrix.com>
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
I have no idea if building an index on a LIKE is a good idea or not, but it certainly seems to be useful, so:
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [OSSTEST PATCH v2 07/41] schema: Provide indices for sg-report-flight
2020-07-31 14:21 ` George Dunlap
@ 2020-07-31 14:55 ` Ian Jackson
0 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 14:55 UTC (permalink / raw)
To: George Dunlap; +Cc: xen-devel
George Dunlap writes ("Re: [OSSTEST PATCH v2 07/41] schema: Provide indices for sg-report-flight"):
>
>
> > On Jul 31, 2020, at 12:37 PM, Ian Jackson <ian.jackson@eu.citrix.com> wrote:
> >
> > These indexes allow very fast lookup of "relevant" flights eg when
> > trying to justify failures.
> >
> > In my ad-hoc test case, these indices (along with the subsequent
> > changes to sg-report-flight and Executive.pm, reduce the runtime of
> > sg-report-flight from 2-3ks (unacceptably long!) to as little as
> > 5-7s seconds - a speedup of about 500x.
> >
> > (Getting the database snapshot may take a while first, but deploying
> > this code should help with that too by reducing long-running
> > transactions. Quoted perf timings are from snapshot acquisition.)
> >
> > Without these new indexes there may be a performance change from the
> > query changes. I haven't benchmarked this so I am setting the schema
> > updates to be Preparatory/Needed (ie, "Schema first" as
> > schema/README.updates has it), to say that the index should be created
> > before the new code is deployed.
> >
> > Testing: I have tested this series by creating experimental indices
> > "trial_..." in the actual production instance. (Transactional DDL was
> > very helpful with this.) I have verified with \d that schema update
> > instructions in this commit generate indexes which are equivalent to
> > the trial indices.
> >
> > Deployment: AFter these schema updates are applied, the trial indices
> > are redundant duplicates and should be deleted.
...
>
> I have no idea if building an index on a LIKE is a good idea or not, but it certainly seems to be useful, so:
>
> Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Thanks.
This is a thing called a "partial index", where the index only covers
some subset of the rows. The subset is determined a condition on the
row contents.
Such an index can be a lot smaller than an index on the whole table
and also avoids slowing down updates that don't match the index
condition.
The idea is that when the query contains a condition that matches the
index condition, the query planner can use this small on-topic index
instead of wading through something large and irrelevant.
The query planner is not always very bright about what conditions are
subsets of what other conditions, and it runs without seeing the
contents of bind variables. So with LIKE, for example, it's generally
necessary to precisely replicate the index condition in the queries.
That's why some of the queries in this series have things like this:
AND r$ri.name LIKE 'built\_revision\_%'
AND r$ri.name = ?
where the Perl code passes in 'built_revison_something'.
I hope this explanation was interesting :-).
Ian.
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [OSSTEST PATCH v2 08/41] sg-report-flight: Ask the db for flights of interest
2020-07-31 14:17 ` George Dunlap
@ 2020-07-31 15:43 ` Ian Jackson
0 siblings, 0 replies; 48+ messages in thread
From: Ian Jackson @ 2020-07-31 15:43 UTC (permalink / raw)
To: George Dunlap; +Cc: xen-devel
George Dunlap writes ("Re: [OSSTEST PATCH v2 08/41] sg-report-flight: Ask the db for flights of interest"):
> > On Jul 31, 2020, at 12:37 PM, Ian Jackson <ian.jackson@eu.citrix.com> wrote:
> > Specifically, we narrow the initial query to flights which have at
> > least some job with the built_revision_foo we are looking for.
> >
> > This condition is strictly broader than that implemented inside the
> > flight search loop, so there is no functional change.
>
> Assuming this is true, that job / runvar is filtered after extracting this information, then...
...
> …I agree that this shoud introduce no other changes.
>
> Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Thanks.
Just to convince myself, I ran through the argument based on the perl
code. I found a lacuna.
1. The job of findaflight is to find a flight, and it doesn't have
significant side effects - just a return value.
2. If it returns a flight from the loop, $whynot must have been
undef. $whynot is never unset.
Consider some tree in %{ $specver{$thisthat} }.
3. If @revisions is 0 for that tree, $whynot is set. So one of the
two queries $revisionsq or $revisionsosstestq must have returned
some rows.
4. Furthermore, none of those rows must have passed the $wronginfo
grep. If they had, $whynot would have been set. Any row
whose val doesn't contain a colon, and which doesn't end up
in $wronginfo, had a val equal to the requested specver.
5. Colons in this field appear only in mercurial revisions. These are
now obsoelete - we have no mercurial trees. A consequence of this
commit is actually that we should explicitly abolish mercurial
support, at least pending a change to osstest to arrange for the
val column to contain only the hash part and not the number part.
6. Together, these conditons means that if $whynot wasn't set,
there must have been some row whose val matched the specver.
7. Both the $revisionsq and $revisionsosstestq queries take a flight
bound variable condition. This is bound by a value that came out
of @binfos. @binfos is made from %binfos, where the flight number
is the key. %binfos is populated by the @binfos_todo loop, where
it gets the flight number from a @binfos_todos entry - but it
filters them for $bflight == $tflight.
8. So some row must have matched the flight, and the specver, and
of course the name. This is precisely the new condition.
I think this means I should put a commit earlier in this series which
disables mercurial support until the colon version situation is
rationalised.
Ian.
^ permalink raw reply [flat|nested] 48+ messages in thread
end of thread, other threads:[~2020-07-31 15:44 UTC | newest]
Thread overview: 48+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-31 11:37 [OSSTEST PATCH v2 00/41] Performance work Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 01/41] Add cperl-indent-level to .dir-locals.el Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 02/41] SQL: Use "LIKE" rather than "like", etc Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 03/41] SQL: Fix incorrect LIKE pattern syntax (literals) Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 04/41] SQL: Fix incorrect LIKE pattern syntax (program variables) Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 05/41] sg-report-flight: Add a comment re same-flight search narrowing Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 06/41] sg-report-flight: Sort failures by job name as last resort Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 07/41] schema: Provide indices for sg-report-flight Ian Jackson
2020-07-31 14:21 ` George Dunlap
2020-07-31 14:55 ` Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 08/41] sg-report-flight: Ask the db for flights of interest Ian Jackson
2020-07-31 14:17 ` George Dunlap
2020-07-31 15:43 ` Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 09/41] sg-report-flight: Use WITH to use best index use for $flightsq Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 10/41] sg-report-flight: Use WITH clause to use index for $anypassq Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 11/41] sg-report-flight: Use the job row from the intitial query Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 12/41] Executive: Use index for report__find_test Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 13/41] duration_estimator: Ignore truncated jobs unless we know the step Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 14/41] duration_estimator: Introduce some _qtxt variables Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 15/41] duration_estimator: Explicitly provide null in general host q Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 16/41] duration_estimator: Return job column in first query Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 17/41] duration_estimator: Move $uptincl_testid to separate @x_params Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 18/41] duration_estimator: Move duration query loop into database Ian Jackson
2020-07-31 11:37 ` [OSSTEST PATCH v2 19/41] Executive: Drop redundant AND clause Ian Jackson
2020-07-31 14:17 ` George Dunlap
2020-07-31 11:37 ` [OSSTEST PATCH v2 20/41] schema: Add index for quick lookup by host Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 21/41] sg-report-host-history: Find flight limit by flight start date Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 22/41] sg-report-host-history: Drop per-job debug etc Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 23/41] Executive: Export opendb_tests Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 24/41] sg-report-host-history: Add a debug print after sorting jobs Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 25/41] sg-report-host-history: Do the main query per host Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 26/41] sg-report-host-history: Rerganisation: Make mainquery per-host Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 27/41] sg-report-host-history: Rerganisation: Read old logs later Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 28/41] sg-report-host-history: Rerganisation: Change loops Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 29/41] sg-report-host-history: Drop a redundznt AND clause Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 30/41] sg-report-host-history: Fork Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 31/41] schema: Add index to help cs-bisection-step Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 32/41] adhoc-revtuple-generator: Fix an undef warning in a debug print Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 33/41] cs-bisection-step: Generalise qtxt_common_rev_ok Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 34/41] cs-bisection-step: Move an AND Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 35/41] cs-bisection-step: Break out qtxt_common_ok Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 36/41] cs-bisection-step: Use db_prepare a few times instead of ->do Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 37/41] cs-bisection-step: temporary table: Insert only rows we care about Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 38/41] SQL: Change LIKE E'...\\_...' to LIKE '...\_...' Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 39/41] cs-bisection-step: Add a debug print when we run dot(1) Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 40/41] cs-bisection-step: Lay out the revision tuple graph once Ian Jackson
2020-07-31 11:38 ` [OSSTEST PATCH v2 41/41] duration_estimator: Clarify recentflights query a bit Ian Jackson
2020-07-31 14:04 ` George Dunlap
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).