[RFC OSSTEST v2] ap-fetch-*: Support $AP_FETCH_PLACEHOLDERS envvar which outputs a placeholder

[RFC OSSTEST v2] ap-fetch-*: Support $AP_FETCH_PLACEHOLDERS envvar which outputs a placeholder

[Ian Campbell]

And use this in standalone-generate-dump-flight-runvars. In general I
don't think we are interested in the specific revision_* runvars when
using this tool but it can be avoided using AP_FETCH_PLACEHOLDERS=n
when calling standalone-generate-dump-flight-runvars.

This is quicker even than using memoisation on the ap-fetch
invocations and produces output like:

libvirt                    build-amd64                                        revision_xen            ap-fetch-version-baseline:xen-unstable

By doing this the diffs of before and after changes to e.g.
make-flight don't pickup noise if a something/someone does a push in
the middle.

The memoisation bits of standalone-generate-dump-flight-runvars are
disable if AP_FETCH_PLACEHOLDERS=y.

Still RFC because of these sqlite errors

    DBD::SQLite::db do failed: UNIQUE constraint failed: jobs.flight, jobs.job [for Statement "        INSERT INTO jobs VALUES (?,'build-i386-xsm','build','queued')

or

    DBD::SQLite::db do failed: database is locked [for Statement " DELETE FROM runvars WHERE flight = ?  "] at Osstest/JobDB/Standalone.pm line 67.
    DBD::SQLite::db do failed: database is locked [for Statement " DELETE FROM runvars WHERE flight = ?  "] at Osstest/JobDB/Standalone.pm line 67.

Which consistently take out the use of standalone-generate-dump-flight-runvars
with this patch. I think probably because ap-fetch-* now complete
instantly which makes the standalone-generate-dump-flight-runvars far
more thunderous on the DB.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v2:
  - Fix typo which would activate this mode unless
    $AP_FETCH_PLACEHOLDERS=x.
  - Require $AP_FETCH_PLACEHOLDERS=y (not just non-empty) and update
    standalone-generate-runvars to only set AP_FETCH_PLACEHOLDERS=y if
    it is unset.
  - Drop sqlite_use_immediate_transaction => 0,
  - Only memoize if not using placeholders (in particular don't blow
    away the cache)

The SQL errors seem to reproduce less reliably in this iteration than
before. I haven't got a FC why.
---
 ap-common                               |  9 +++++++++
 ap-fetch-version                        |  2 ++
 ap-fetch-version-baseline               |  3 +++
 ap-fetch-version-baseline-late          |  2 ++
 ap-fetch-version-old                    |  2 ++
 standalone-generate-dump-flight-runvars | 10 ++++++++--
 6 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/ap-common b/ap-common
index 91425a9..5b6e088 100644
--- a/ap-common
+++ b/ap-common
@@ -145,3 +145,12 @@ info_linux_tree () {
 
 	return 0
 }
+
+check_ap_fetch_placeholders () {
+	if [ "x$AP_FETCH_PLACEHOLDERS" != xy ] ; then
+		return 0
+	fi
+
+	echo "$(basename $0):$branch"
+	exit 0
+}
diff --git a/ap-fetch-version b/ap-fetch-version
index 6fa7588..f884bd3 100755
--- a/ap-fetch-version
+++ b/ap-fetch-version
@@ -25,6 +25,8 @@ branch=$1
 select_xenbranch
 . ./ap-common
 
+check_ap_fetch_placeholders
+
 if info_linux_tree "$branch"; then
 	repo_tree_rev_fetch_git linux \
 		$TREE_LINUX_THIS $TAG_LINUX_THIS $LOCALREV_LINUX
diff --git a/ap-fetch-version-baseline b/ap-fetch-version-baseline
index 2e42508..c9da82c 100755
--- a/ap-fetch-version-baseline
+++ b/ap-fetch-version-baseline
@@ -22,6 +22,9 @@ set -e -o posix
 branch=$1
 
 . ./cri-lock-repos
+. ./ap-common
+
+check_ap_fetch_placeholders
 
 : ${BASE_TREE_LINUX:=git://xenbits.xen.org/people/ianc/linux-2.6.git}
 : ${BASE_TAG_LINUX:=xen/next-2.6.32}
diff --git a/ap-fetch-version-baseline-late b/ap-fetch-version-baseline-late
index 9856ec9..dff8b05 100755
--- a/ap-fetch-version-baseline-late
+++ b/ap-fetch-version-baseline-late
@@ -27,6 +27,8 @@ new=$2
 select_xenbranch
 . ./ap-common
 
+check_ap_fetch_placeholders
+
 case "$branch" in
 
 linux-next)
diff --git a/ap-fetch-version-old b/ap-fetch-version-old
index 66d51f8..99f276a 100755
--- a/ap-fetch-version-old
+++ b/ap-fetch-version-old
@@ -25,6 +25,8 @@ branch=$1
 select_xenbranch
 . ./ap-common
 
+check_ap_fetch_placeholders
+
 : ${BASE_TAG_LINUX2639:=tested/2.6.39.x}
 : ${BASE_LOCALREV_LINUX:=daily-cron.$branch.old}
 : ${BASE_LOCALREV_LIBVIRT:=daily-cron.$branch.old}
diff --git a/standalone-generate-dump-flight-runvars b/standalone-generate-dump-flight-runvars
index d113927..a1907b0 100755
--- a/standalone-generate-dump-flight-runvars
+++ b/standalone-generate-dump-flight-runvars
@@ -36,11 +36,17 @@ if [ $# = 0 ]; then
    set `./mg-list-all-branches`
 fi
 
-if [ "x$AP_FETCH_MEMO_KEEP" = x ]; then
+: ${AP_FETCH_PLACEHOLDERS:=y}
+export AP_FETCH_PLACEHOLDERS
+
+
+if [ "x$AP_FETCH_PLACEHOLDERS" != xy ]; then
+    if [ "x$AP_FETCH_MEMO_KEEP" = x ]; then
 	rm -rf tmp/apmemo
 	mkdir tmp/apmemo
+    fi
+    export AP_FETCH_PFX='./memoise tmp/apmemo'
 fi
-export AP_FETCH_PFX='./memoise tmp/apmemo'
 
 # In the future it might be nice for this script to arrange to use a
 # separate standalone.db, in tmp/ probably, for each different branch.
-- 
2.5.3

Re: [RFC OSSTEST v2] ap-fetch-*: Support $AP_FETCH_PLACEHOLDERS envvar which outputs a placeholder

[Ian Jackson]

Ian Campbell writes ("[RFC OSSTEST v2] ap-fetch-*: Support $AP_FETCH_PLACEHOLDERS envvar which outputs a placeholder"):
> Still RFC because of these sqlite errors
> 
>     DBD::SQLite::db do failed: UNIQUE constraint failed: jobs.flight, jobs.job [for Statement "        INSERT INTO jobs VALUES (?,'build-i386-xsm','build','queued')

After having tried to repro this without success, and discussed
various implausible theories, we think this error is due to the
following:

You ran ./standalone-generate-dump-flight-runvars but then realised
you wanted to restart it.  So you pressed ^C.  But this does not
actually kill all the children (this is indeed a problem with this
script).  You then reran the script.  Now it is racing with other
copies of make-flight, cs-job-create, etc., which can create the same
jobs in the same flights as your current run.

>     DBD::SQLite::db do failed: database is locked [for Statement " DELETE FROM runvars WHERE flight = ?  "] at Osstest/JobDB/Standalone.pm line 67.
>     DBD::SQLite::db do failed: database is locked [for Statement " DELETE FROM runvars WHERE flight = ?  "] at Osstest/JobDB/Standalone.pm line 67.

And we think this is while you were trying to solve these problems by
setting the timeout, which you say you tried setting to zero in the
belief (justified by documentation but not apparently true) that that
means `infinite'.

> Which consistently take out the use of
> standalone-generate-dump-flight-runvars with this patch. I think
> probably because ap-fetch-* now complete instantly which makes the
> standalone-generate-dump-flight-runvars far more thunderous on the
> DB.

In any case, I have tried this several times both with and without
eatmydata and it seems to work fine for me.

I may prepare a patch to make standalone-generate-dump-flight-runvars
handle ^C properly.

Ian.

Re: [RFC OSSTEST v2] ap-fetch-*: Support $AP_FETCH_PLACEHOLDERS envvar which outputs a placeholder

[Ian Campbell]

On Fri, 2015-10-09 at 17:06 +0100, Ian Jackson wrote:
> 
[...]

Agree with that analysis, thanks.

> In any case, I have tried this several times both with and without
> eatmydata and it seems to work fine for me.

Me too, today.

> I may prepare a patch to make standalone-generate-dump-flight-runvars
> handle ^C properly.

That would be nice.

I shall send a v3 without the RFC stuff in a moment.

Ian.