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

[RFC OSSTEST] 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 and this is quicker even than using memoisation on the
ap-fetch invocations. This 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.

RFC firstly because I'm not 100% sure about making this unconditional
in standalone-generate-dump-flight-runvars and secondly because of the
addition of sqlite_use_immediate_transaction => 0 which TBH I don't
really understand but resolves errors like:

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

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.

Even with this I see occasional:

DBD::SQLite::db do failed: UNIQUE constraint failed: jobs.flight, jobs.job [for Statement "        INSERT INTO jobs VALUES (?,'test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm','test-debianhvm','queued')
"] at ./cs-job-create line 65.

But statistically a couple of tries usually gets me past that.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 Osstest/JobDB/Standalone.pm             | 1 +
 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 | 2 ++
 7 files changed, 21 insertions(+)

diff --git a/Osstest/JobDB/Standalone.pm b/Osstest/JobDB/Standalone.pm
index 431ba5a..9874e7d 100644
--- a/Osstest/JobDB/Standalone.pm
+++ b/Osstest/JobDB/Standalone.pm
@@ -50,6 +50,7 @@ sub open ($) {
         AutoCommit => 1,
         RaiseError => 1,
         ShowErrorStatement => 1,
+        sqlite_use_immediate_transaction => 0,
         })
         or die "could not open standalone db $dbi";
 
diff --git a/ap-common b/ap-common
index 91425a9..cff9d3f 100644
--- a/ap-common
+++ b/ap-common
@@ -145,3 +145,12 @@ info_linux_tree () {
 
 	return 0
 }
+
+check_ap_fetch_placeholders () {
+	if [ "$AP_FETCH_PLACEHOLDERS" = x ] ; 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..d15a28e 100755
--- a/standalone-generate-dump-flight-runvars
+++ b/standalone-generate-dump-flight-runvars
@@ -42,6 +42,8 @@ if [ "x$AP_FETCH_MEMO_KEEP" = x ]; then
 fi
 export AP_FETCH_PFX='./memoise tmp/apmemo'
 
+export AP_FETCH_PLACEHOLDERS=1
+
 # 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.
 # That would avoid a lot of needless write contention over the single
-- 
2.5.3

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

[Ian Jackson]

Ian Campbell writes ("[RFC OSSTEST] ap-fetch-*: Support $AP_FETCH_PLACEHOLDERS envvar which outputs a placeholder"):
> 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 and this is quicker even than using memoisation on the
> ap-fetch invocations. This 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.
> 
> RFC firstly because I'm not 100% sure about making this unconditional
> in standalone-generate-dump-flight-runvars

Maybe it should be the default, but it is sometimes useful to see the
effect of changes to ap-fetch* in the runvars.  Eg, my smoke test
series would have needed not to be using these placeholders.

> and secondly because of the
> addition of sqlite_use_immediate_transaction => 0 which TBH I don't
> really understand but resolves errors like:

I can see why that helps but I don't think it's right.

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

This is very odd.  These jobs ought to have been deleted by
Osstest::JobDB::Standalone::flight_create.

This merits further investigation :-/.

Ian.

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

[Ian Campbell]

On Thu, 2015-10-08 at 17:00 +0100, Ian Jackson wrote:
> Ian Campbell writes ("[RFC OSSTEST] ap-fetch-*: Support
> $AP_FETCH_PLACEHOLDERS envvar which outputs a placeholder"):
> > 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 and this is quicker even than using memoisation on the
> > ap-fetch invocations. This 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.
> > 
> > RFC firstly because I'm not 100% sure about making this unconditional
> > in standalone-generate-dump-flight-runvars
> 
> Maybe it should be the default, but it is sometimes useful to see the
> effect of changes to ap-fetch* in the runvars.  Eg, my smoke test
> series would have needed not to be using these placeholders.

Right, I can easily (I hope) make that aspect work then.

> > and secondly because of the
> > addition of sqlite_use_immediate_transaction => 0 which TBH I don't
> > really understand but resolves errors like:
> 
> I can see why that helps but I don't think it's right.

I suspected as much.

> >     DBD::SQLite::db do failed: UNIQUE constraint failed: jobs.flight,
> > jobs.job [for Statement "        INSERT INTO jobs VALUES (?,'build-i386
> > -xsm','build','queued')
> 
> This is very odd.  These jobs ought to have been deleted by
> Osstest::JobDB::Standalone::flight_create.
> 
> This merits further investigation :-/.

Right :-/

Ian.

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

[Ian Jackson]

Ian Campbell writes ("Re: [RFC OSSTEST] ap-fetch-*: Support $AP_FETCH_PLACEHOLDERS envvar which outputs a placeholder"):
> On Thu, 2015-10-08 at 17:00 +0100, Ian Jackson wrote:
> > Maybe [placeholders] should be the default, but it is sometimes
> > useful to see the effect of changes to ap-fetch* in the runvars.
> > Eg, my smoke test series would have needed not to be using these
> > placeholders.
> 
> Right, I can easily (I hope) make that aspect work then.

I'll wait for a respin of this and then I will see about ...

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

... trying to repro and debug this.

Ian.