launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/livefs-garbo into lp:launchpad with lp:~cjwatson/launchpad/livefs as a prerequisite.

Commit message:
Add a garbo job to remove old LiveFSFile records.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1247461 in Launchpad itself: "Move live filesystem building into Launchpad"
  https://bugs.launchpad.net/launchpad/+bug/1247461

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/livefs-garbo/+merge/217784

== Summary ==

Next stage of bug 1247461, following https://code.launchpad.net/~cjwatson/launchpad/livefs/+merge/217261.  This adds a garbo job so that we don't keep large livefs images around for long.

== Proposed fix ==

I explored various options including keeping the current image for each LiveFS, but that turned out to be cumbersome to get right: we'd have had to do something special to avoid images eventually piling up for old series that we don't build for any more.  In the end I decided to keep it simple for now: remove any images more than a day old, which will be more than enough time for cdimage to fetch them and build ISOs out of them or publish them somewhere persistent.  We can always tweak this later.

This will require cdimage to use its locally-cached copy if somebody tries to do an image build that intentionally doesn't rebuild the livefs.  But the rearrangements for this will be a good thing anyway: in particular they will arrange that we don't do an image build if the corresponding livefs build failed, which has been a long-standing problem in cdimage that causes confusion from time to time (at present we only fail the image build if livefs builds on all architectures fail, and otherwise carry on merrily using an old livefs on some architectures).

== LOC Rationale ==

+65; same rationale as https://code.launchpad.net/~cjwatson/launchpad/livefs/+merge/217261.

== Tests ==

bin/test -vvct LiveFSFilePruner

== Demo and Q/A ==

Build an image, wait a day (or hack the DB to lie about its date_finished), run garbo, and check that the appropriate LiveFSBuild's binary LiveFSFile artifacts are pruned.
-- 
https://code.launchpad.net/~cjwatson/launchpad/livefs-garbo/+merge/217784
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/livefs-garbo into lp:launchpad.

=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py	2014-03-11 06:14:19 +0000
+++ lib/lp/scripts/garbo.py	2014-04-30 15:55:05 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2014 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Database garbage collection."""
@@ -118,6 +118,7 @@
 from lp.services.session.model import SessionData
 from lp.services.verification.model.logintoken import LoginToken
 from lp.soyuz.model.archive import Archive
+from lp.soyuz.model.livefsbuild import LiveFSFile
 from lp.soyuz.model.publishing import SourcePackagePublishingHistory
 from lp.soyuz.model.reporting import LatestPersonSourcePackageReleaseCache
 from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
@@ -1363,6 +1364,28 @@
         transaction.commit()
 
 
+class LiveFSFilePruner(BulkPruner):
+    """A BulkPruner to remove old `LiveFSFile`s.
+
+    We remove binary files attached to `LiveFSBuild`s that are more than a
+    day old; these files are very large and are only useful for builds in
+    progress.  Text files are typically small (<1MiB) and useful for
+    retrospective analysis, so we preserve those indefinitely.
+    """
+    target_table_class = LiveFSFile
+    ids_to_prune_query = """
+        SELECT DISTINCT LiveFSFile.id
+        FROM LiveFSFile, LiveFSBuild, LibraryFileAlias
+        WHERE
+            LiveFSFile.livefsbuild = LiveFSBuild.id
+            AND LiveFSFile.libraryfile = LibraryFileAlias.id
+            AND LiveFSBuild.date_finished <
+                CURRENT_TIMESTAMP AT TIME ZONE 'UTC'
+                - CAST('1 day' AS interval)
+            AND LibraryFileAlias.mimetype != 'text/plain'
+        """
+
+
 class BaseDatabaseGarbageCollector(LaunchpadCronScript):
     """Abstract base class to run a collection of TunableLoops."""
     script_name = None  # Script name for locking and database user. Override.
@@ -1642,6 +1665,7 @@
         CodeImportEventPruner,
         CodeImportResultPruner,
         HWSubmissionEmailLinker,
+        LiveFSFilePruner,
         LoginTokenPruner,
         ObsoleteBugAttachmentPruner,
         OldTimeLimitedTokenDeleter,

=== modified file 'lib/lp/scripts/tests/test_garbo.py'
--- lib/lp/scripts/tests/test_garbo.py	2014-03-10 19:30:37 +0000
+++ lib/lp/scripts/tests/test_garbo.py	2014-04-30 15:55:05 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2014 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test the database garbage collector."""
@@ -41,6 +41,7 @@
     BugNotification,
     BugNotificationRecipient,
     )
+from lp.buildmaster.enums import BuildStatus
 from lp.code.bzr import (
     BranchFormat,
     RepositoryFormat,
@@ -87,6 +88,7 @@
     )
 from lp.services.database.interfaces import IMasterStore
 from lp.services.features.model import FeatureFlag
+from lp.services.features.testing import FeatureFixture
 from lp.services.identity.interfaces.account import AccountStatus
 from lp.services.identity.interfaces.emailaddress import EmailAddressStatus
 from lp.services.job.model.job import Job
@@ -109,6 +111,8 @@
 from lp.services.verification.model.logintoken import LoginToken
 from lp.services.worlddata.interfaces.language import ILanguageSet
 from lp.soyuz.enums import PackagePublishingStatus
+from lp.soyuz.interfaces.livefs import LIVEFS_FEATURE_FLAG
+from lp.soyuz.model.livefsbuild import LiveFSFile
 from lp.soyuz.model.reporting import LatestPersonSourcePackageReleaseCache
 from lp.testing import (
     feature_flags,
@@ -668,9 +672,9 @@
         switch_dbuser('testadmin')
         mp1 = self.factory.makeBranchMergeProposal()
         now = datetime.now(UTC)
-        mp1_diff_comment =  self.factory.makePreviewDiff(
+        mp1_diff_comment = self.factory.makePreviewDiff(
             merge_proposal=mp1, date_created=now - timedelta(hours=2))
-        mp1_diff_draft =  self.factory.makePreviewDiff(
+        mp1_diff_draft = self.factory.makePreviewDiff(
             merge_proposal=mp1, date_created=now - timedelta(hours=1))
         mp1_diff = self.factory.makePreviewDiff(merge_proposal=mp1)
         # Enabled 'inline_diff_comments' feature flag and attach comments
@@ -1317,6 +1321,43 @@
             'PopulateLatestPersonSourcePackageReleaseCache')
         self.assertEqual(spph_2.id, job_data['last_spph_id'])
 
+    def _test_LiveFSFilePruner(self, content_type, interval, expected_count=0):
+        # Garbo should (or should not, if `expected_count=1`) remove LiveFS
+        # files of MIME type `content_type` that finished more than
+        # `interval` days ago.
+        now = datetime.now(UTC)
+        switch_dbuser('testadmin')
+        self.useFixture(FeatureFixture({LIVEFS_FEATURE_FLAG: u'on'}))
+        store = IMasterStore(LiveFSFile)
+
+        db_build = self.factory.makeLiveFSBuild(
+            date_created=now - timedelta(days=interval, minutes=15),
+            status=BuildStatus.FULLYBUILT, duration=timedelta(minutes=10))
+        db_lfa = self.factory.makeLibraryFileAlias(content_type=content_type)
+        db_file = self.factory.makeLiveFSFile(
+            livefsbuild=db_build, libraryfile=db_lfa)
+        Store.of(db_file).flush()
+        self.assertEqual(1, store.find(LiveFSFile).count())
+
+        self.runDaily()
+
+        switch_dbuser('testadmin')
+        self.assertEqual(expected_count, store.find(LiveFSFile).count())
+
+    def test_LiveFSFilePruner_old_binary_files(self):
+        # LiveFS binary files attached to builds over a day old are pruned.
+        self._test_LiveFSFilePruner('application/octet-stream', 1)
+
+    def test_LiveFSFilePruner_old_text_files(self):
+        # LiveFS text files attached to builds over a day old are retained.
+        self._test_LiveFSFilePruner('text/plain', 1, expected_count=1)
+
+    def test_LiveFSFilePruner_recent_binary_files(self):
+        # LiveFS binary files attached to builds less than a day old are
+        # retained.
+        self._test_LiveFSFilePruner(
+            'application/octet-stream', 0, expected_count=1)
+
 
 class TestGarboTasks(TestCaseWithFactory):
     layer = LaunchpadZopelessLayer