Merge lp:~abentley/launchpad/daily-build-freshness into lp:launchpad/db-devel

[Aaron Bentley]

Store build manifests and flag stale recipes.

= Summary =
Provide supplementary data for daily builds.

== Proposed fix ==
Provide SourcePackageRecipe.is_stale to determine whether a recipe should be
rebuilt for daily builds.

Provide SourcePackageRecipeBuild.manifest so that we know what we built. This
can then be fed into bzr-builder's --if-changed-from option in order to avoid
unnecessary rebuilds.

== Pre-implementation notes ==
Preimplementation was with Thumper.

== Implementation details ==
None

== Tests ==
None

== Demo and Q/A ==
None

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  database/sampledata/current.sql
  database/sampledata/current-dev.sql
  database/schema/comments.sql
  database/schema/patch-2207-99-0.sql

[Björn Tillenius (bjornt)]

In general, the patch looks fine, but could you please expand on how the attributes will be set and used?

review: Needs Information (db)

[Aaron Bentley (abentley)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 05/28/2010 09:37 AM, Björn Tillenius wrote:
> Review: Needs Information db
> In general, the patch looks fine, but could you please expand on how the attributes will be set and used?

SourcePackageRecipe.is_stale
============================

When the Branch scanner sees a that the branch tip has changed for a
given branch, it will find all recipes which involve that branch, and
set is_stale to True for all of them.

When the daily builds script runs, it will look for recipes with
is_stale True and build_daily True. It will request builds for them.
Then it will set is_stale False for them.

SourcePackageRecipeBuild.manifest
=================================
When RecipeBuildBehaviour dispatches a build to a slave, it will look
for the last successful build of that type (same recipe and
distroseries) and include that build's manifest with its extra build
args. (Manifests are a form of recipe, so converting a manifest to text
is something we already do.)

The recipe building code in our buildd will be modified to use the
manifest if supplied.

When the build completes, the output manifest is (already) supplied to
RecipeBuildBehaviour. We will modify RecipeBuildBehaviour to set
SourcePackageRecipeBuild.manifest.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkv/07wACgkQ0F+nu1YWqI2LlgCdER2ysBDstTHQ0ZxJUV9HLg0n
jeoAn0YCgJQUUM8qdL1LlosSHimgDyaW
=5NhT
-----END PGP SIGNATURE-----

[Björn Tillenius (bjornt)]

On Fri, May 28, 2010 at 02:34:29PM -0000, Aaron Bentley wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 05/28/2010 09:37 AM, Björn Tillenius wrote:
> > Review: Needs Information db
> > In general, the patch looks fine, but could you please expand on how the attributes will be set and used?
>
> SourcePackageRecipe.is_stale
> ============================
>
> When the Branch scanner sees a that the branch tip has changed for a
> given branch, it will find all recipes which involve that branch, and
> set is_stale to True for all of them.
>
> When the daily builds script runs, it will look for recipes with
> is_stale True and build_daily True. It will request builds for them.
> Then it will set is_stale False for them.

Ok, thanks for the information. I have one comment, though. For the sake
of correctness, it seems like is_stale shouldn't be set after the build
has been requested, but rather after the build has completed.

BTW, did you consider any other approaches? For example, keeping track
of which revision a build was for, and instead of finding all
is_stale=True recipes, find the recipes where the revision of the latest
build for a recipe is different from the latest revision in that branch?

Will your suggested approach have any effect on branch scanner
performance?

--
Björn Tillenius | https://launchpad.net/~bjornt

[Aaron Bentley (abentley)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 05/31/2010 11:00 AM, Björn Tillenius wrote:
> Ok, thanks for the information. I have one comment, though. For the sake
> of correctness, it seems like is_stale shouldn't be set after the build
> has been requested, but rather after the build has completed.

There is no single build to refer to. A recipe may be built for many
distroseries. Even if there were a single build, it can become outdated
if a branch tip changes while it is building. But we don't know which
revisions it used to perform its build until the build is complete.
Also, if for some reason the build doesn't start in 24 hours, leaving
is_stale set to True would cause duplicate builds to be requested.

It seems to me that attempting to track the staleness of a recipe with
great precision makes us sensitive to race conditions and is not worth
the complexity. Instead, we should accept that we will occasionally
request duplicate builds, and handle that situation gracefully using
manifests as I described. Even without daily builds, it's easy for
users to accidentally request duplicate builds.

> BTW, did you consider any other approaches? For example, keeping track
> of which revision a build was for, and instead of finding all
> is_stale=True recipes, find the recipes where the revision of the latest
> build for a recipe is different from the latest revision in that branch?

Yes, I proposed something like that to thumper, and he proposed this
solution as a much simpler alternative.

Bear in mind that builds are one per distroseries, and that several
branches are involved in a recipe, and changing the tip of any of them
should trigger a build.

> Will your suggested approach have any effect on branch scanner
> performance?

I don't anticipate any observable difference. The DB queries should be
quite straightforward.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkwD1LYACgkQ0F+nu1YWqI3qEwCeIbg2wozuK9gZYfCB3mgmMh7x
MGoAn0k6pJzGmS1KhtQ8S0335mrsaiQT
=mtJA
-----END PGP SIGNATURE-----

[Björn Tillenius (bjornt)]

review: Approve (db)

[Stuart Bishop (stub)]

Generally fine.

I think we want some indexes:

CREATE INDEX sourcepackagerecipe__is_stale__build_daily__idx
ON SourcepackageRecipe(is_stale, build_daily);

CREATE INDEX sourcepackagerecipebuild__manifest__idx ON SourcepackageRecipeBuild(manifest);

patch-2207-61-0.sql

review: Approve (db)

=== modified file 'database/sampledata/current-dev.sql'
=== modified file 'database/sampledata/current.sql'
=== modified file 'database/schema/comments.sql'
--- database/schema/comments.sql	2010-05-27 22:18:16 +0000
+++ database/schema/comments.sql	2010-06-03 13:31:39 +0000
@@ -1350,6 +1350,7 @@
 COMMENT ON COLUMN SourcePackageRecipe.owner IS 'The person or team who can edit this recipe.';
 COMMENT ON COLUMN SourcePackageRecipe.name IS 'The name of the recipe in the web/URL.';
 COMMENT ON COLUMN SourcePackageRecipe.build_daily IS 'If true, this recipe should be built daily.';
+COMMENT ON COLUMN SourcePackageRecipe.is_stale IS 'True if this recipe has not been built since a branch was updated.';
 
 COMMENT ON COLUMN SourcePackageREcipe.daily_build_archive IS 'The archive to build into for daily builds.';
 
@@ -1371,6 +1372,7 @@
 COMMENT ON COLUMN SourcePackageRecipeBuild.date_first_dispatched IS 'The instant the build was dispatched the first time. This value will not get overridden if the build is retried.';
 COMMENT ON COLUMN SourcePackageRecipeBuild.requester IS 'Who requested the build.';
 COMMENT ON COLUMN SourcePackageRecipeBuild.recipe IS 'The recipe being processed.';
+COMMENT ON COLUMN SourcePackageRecipeBuild.manifest IS 'The evaluated recipe that was built.';
 COMMENT ON COLUMN SourcePackageRecipeBuild.archive IS 'The archive the source package will be built in and uploaded to.';
 COMMENT ON COLUMN SourcePackageRecipeBuild.pocket IS 'The pocket the source package will be built in and uploaded to.';
 COMMENT ON COLUMN SourcePackageRecipeBuild.dependencies IS 'The missing build dependencies, if any.';

=== added file 'database/schema/patch-2207-61-0.sql'
--- database/schema/patch-2207-61-0.sql	1970-01-01 00:00:00 +0000
+++ database/schema/patch-2207-61-0.sql	2010-06-03 13:31:39 +0000
@@ -0,0 +1,13 @@
+-- Copyright 2010 Canonical Ltd.  This software is licensed under the
+-- GNU Affero General Public License version 3 (see the file LICENSE).
+
+SET client_min_messages=ERROR;
+ALTER TABLE SourcePackageRecipe ADD COLUMN is_stale BOOLEAN NOT NULL DEFAULT TRUE;
+ALTER TABLE SourcePackageRecipeBuild ADD COLUMN manifest INTEGER REFERENCES SourcePackageRecipeData;
+
+CREATE INDEX sourcepackagerecipe__is_stale__build_daily__idx
+ON SourcepackageRecipe(is_stale, build_daily);
+
+CREATE INDEX sourcepackagerecipebuild__manifest__idx ON SourcepackageRecipeBuild(manifest);
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 61, 0);