Merge ~cjwatson/launchpad:archive-file-deeper-history into launchpad:master

[Colin Watson]

Retain more ArchiveFile history

Source and binary packages are removed from the published archive on disk a day after they are no longer referenced by current index files; package files in the librarian are typically expired a week after all their associated publishing history rows have been marked as removed from disk, although files in some archives (primary archives and certain named PPAs) never expire.

Archive index files, such as `Packages`, `Sources`, and `Release` files, currently work quite differently: old versions of these files are published in `by-hash` directories on disk so that clients can continue to make use of them for a grace period of a day, after which they are both removed from disk and removed from the database.

The latter approach is problematic for the forthcoming snapshot service, because it means we can only serve snapshots of index files for as long as they are still published on disk: not much more useful than what we already have!

This commit implements a much more useful policy. We now treat archive index files similarly to source and binary packages: they're removed from disk after a day as before, to prevent `by-hash` directories getting unreasonably large and causing bloat on mirrors, but now we merely set the `ArchiveFile.date_removed` column rather than deleting the row entirely. The `expire-archive-files` script now handles `ArchiveFile` as well as `SourcePackageReleaseFile` and `BinaryPackageFile` rows, which means that old snapshots of the primary archive and some other important archives will be retained in the database and the librarian.

This will of course cause the librarian to grow more quickly, but some back-of-the-envelope calculations suggest that even if we retain old archive index files in perpetuity then this will be on the order of an extra terabyte per year, which seems quite manageable.

DB MP: https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/436358

[William Grant (wgrant)]

Looks generally good. But a couple of concerns that I haven't totally thought through yet.

Are there potential races between markDeleted and unscheduleDeletion?

Does something (maybe just ArchiveAPI._translatePathByHash?) need to check whether the ArchiveFile has been expired to avoid crashing?

review: Approve (code)

[Colin Watson (cjwatson)]

I removed `unscheduleDeletion` in commit 34dddeb6e38bd31f8729a790c88e57f7738695d3, and we now just create new files rather than pre-optimizing that deduplication step, so that isn't a problem.

Expired `ArchiveFile`s aren't a problem, because (a) `getURL` just returns None and (b) `_translatePathByHash` already implicitly filters out expired rows by virtue of the join with `LibraryFileContent` to check the checksum. This is not 100% obvious at a glance though, and it was a bit inefficient due to inappropriate fallbacks in the case where it runs into a deleted file, so I've tightened this up a bit and added some explicit tests.

diff --git a/database/schema/security.cfg b/database/schema/security.cfg
index d7a4fbb..49f2daf 100644
--- a/database/schema/security.cfg
+++ b/database/schema/security.cfg
@@ -2032,6 +2032,7 @@ type=user
 [binaryfile-expire]
 groups=script
 public.archive                          = SELECT
+public.archivefile                      = SELECT
 public.binarypackagefile                = SELECT
 public.binarypackagepublishinghistory   = SELECT
 public.binarypackagerelease             = SELECT
diff --git a/lib/lp/archivepublisher/publishing.py b/lib/lp/archivepublisher/publishing.py
index 5778aa9..877ca78 100644
--- a/lib/lp/archivepublisher/publishing.py
+++ b/lib/lp/archivepublisher/publishing.py
@@ -1274,17 +1274,19 @@ class Publisher:
         by_hashes = ByHashes(self._config.distsroot, self.log)
         existing_live_files = {}
         existing_nonlive_files = {}
-        keep_files = set()
         reapable_files = set()
 
         def strip_dists(path):
             assert path.startswith("dists/")
             return path[len("dists/") :]
 
-        # Record all files from the database.
+        # Record all published files from the database.
         db_now = get_transaction_timestamp(IStore(ArchiveFile))
         for db_file in archive_file_set.getByArchive(
-            self.archive, container=container, eager_load=True
+            self.archive,
+            container=container,
+            only_published=True,
+            eager_load=True,
         ):
             file_key = (
                 strip_dists(db_file.path),
@@ -1375,16 +1377,18 @@ class Publisher:
         # Remove any files from disk that aren't recorded in the database.
         by_hashes.prune()
 
-        # And remove expired ArchiveFiles from the DB now that we've pruned
-        # them and their directories from disk.
-        delete_files = reapable_files - keep_files
-        if delete_files:
-            for container, path, sha256 in archive_file_set.delete(
-                delete_files
-            ):
+        # And mark expired ArchiveFiles as deleted in the DB now that we've
+        # pruned them and their directories from disk.
+        if reapable_files:
+            archive_file_set.markDeleted(reapable_files)
+            for db_file in reapable_files:
                 self.log.debug(
-                    "by-hash: Deleted %s for %s in %s"
-                    % (sha256, path, container)
+                    "by-hash: Marked %s for %s in %s as deleted"
+                    % (
+                        db_file.library_file.content.sha256,
+                        db_file.path,
+                        db_file.container,
+                    )
                 )
 
     def _writeReleaseFile(self, suite, release_data):
diff --git a/lib/lp/archivepublisher/tests/test_publisher.py b/lib/lp/archivepublisher/tests/test_publisher.py
index 33fc909..3aed43f 100644
--- a/lib/lp/archivepublisher/tests/test_publisher.py
+++ b/lib/lp/archivepublisher/tests/test_publisher.py
@@ -17,6 +17,7 @@ from datetime import datetime, timedelta
 from fnmatch import fnmatch
 from functools import partial
 from textwrap import dedent
+from typing import Optional, Sequence, Tuple
 from unittest import mock
 
 import pytz
@@ -3066,7 +3067,24 @@ class TestUpdateByHash(TestPublisherBase):
                 superseded_at + timedelta(days=BY_HASH_STAY_OF_EXECUTION)
             )
 
-    def assertHasSuiteFiles(self, patterns, *properties):
+    def assertHasSuiteFiles(
+        self,
+        patterns: Sequence[str],
+        *properties: Tuple[str, Optional[int], Optional[int], Optional[int]]
+    ) -> None:
+        """Assert that the database records certain archive files.
+
+        :param patterns: A sequence of `fnmatch` patterns for files under
+            `dists/breezy-autotest/` that we're interested in.
+        :param properties: A sequence of (`path`, `created_at`,
+            `superseded_at`, `removed_at`) tuples.  Each must match one of
+            the `ArchiveFile` rows matching `patterns`; `path` is relative
+            to `dists/breezy-autotest/`, while the `*_at` properties are
+            either None (indicating that the corresponding `date_*` column
+            should be None) or an index into `self.times` (indicating that
+            the corresponding `date_*` column should equal that time).
+        """
+
         def is_interesting(path):
             return any(
                 fnmatch(path, "dists/breezy-autotest/%s" % pattern)
@@ -3081,7 +3099,12 @@ class TestUpdateByHash(TestPublisherBase):
             if is_interesting(archive_file.path)
         ]
         matchers = []
-        for path, created_at, superseded_at in properties:
+        for path, created_at, superseded_at, removed_at in properties:
+            created_at = None if created_at is None else self.times[created_at]
+            superseded_at = (
+                None if superseded_at is None else self.times[superseded_at]
+            )
+            removed_at = None if removed_at is None else self.times[removed_at]
             scheduled_deletion_date_matcher = (
                 self._makeScheduledDeletionDateMatcher(superseded_at)
             )
@@ -3091,6 +3114,7 @@ class TestUpdateByHash(TestPublisherBase):
                     date_created=Equals(created_at),
                     date_superseded=Equals(superseded_at),
                     scheduled_deletion_date=scheduled_deletion_date_matcher,
+                    date_removed=Equals(removed_at),
                 )
             )
         self.assertThat(files, MatchesSetwise(*matchers))
@@ -3280,8 +3304,8 @@ class TestUpdateByHash(TestPublisherBase):
         flush_database_caches()
         self.assertHasSuiteFiles(
             ("Contents-*", "Release"),
-            ("Contents-i386", self.times[0], None),
-            ("Release", self.times[0], None),
+            ("Contents-i386", 0, None, None),
+            ("Release", 0, None, None),
         )
         releases = [get_release_contents()]
         self.assertThat(
@@ -3297,10 +3321,10 @@ class TestUpdateByHash(TestPublisherBase):
         flush_database_caches()
         self.assertHasSuiteFiles(
             ("Contents-*", "Release"),
-            ("Contents-i386", self.times[0], None),
-            ("Contents-hppa", self.times[1], None),
-            ("Release", self.times[0], self.times[1]),
-            ("Release", self.times[1], None),
+            ("Contents-i386", 0, None, None),
+            ("Contents-hppa", 1, None, None),
+            ("Release", 0, 1, None),
+            ("Release", 1, None, None),
         )
         releases.append(get_release_contents())
         self.assertThat(
@@ -3315,11 +3339,11 @@ class TestUpdateByHash(TestPublisherBase):
         flush_database_caches()
         self.assertHasSuiteFiles(
             ("Contents-*", "Release"),
-            ("Contents-i386", self.times[0], self.times[2]),
-            ("Contents-hppa", self.times[1], None),
-            ("Release", self.times[0], self.times[1]),
-            ("Release", self.times[1], self.times[2]),
-            ("Release", self.times[2], None),
+            ("Contents-i386", 0, 2, None),
+            ("Contents-hppa", 1, None, None),
+            ("Release", 0, 1, None),
+            ("Release", 1, 2, None),
+            ("Release", 2, None, None),
         )
         releases.append(get_release_contents())
         self.assertThat(
@@ -3333,12 +3357,12 @@ class TestUpdateByHash(TestPublisherBase):
         flush_database_caches()
         self.assertHasSuiteFiles(
             ("Contents-*", "Release"),
-            ("Contents-i386", self.times[0], self.times[2]),
-            ("Contents-hppa", self.times[1], None),
-            ("Release", self.times[0], self.times[1]),
-            ("Release", self.times[1], self.times[2]),
-            ("Release", self.times[2], self.times[3]),
-            ("Release", self.times[3], None),
+            ("Contents-i386", 0, 2, None),
+            ("Contents-hppa", 1, None, None),
+            ("Release", 0, 1, None),
+            ("Release", 1, 2, None),
+            ("Release", 2, 3, None),
+            ("Release", 3, None, None),
         )
         releases.append(get_release_contents())
         self.assertThat(
@@ -3365,10 +3389,13 @@ class TestUpdateByHash(TestPublisherBase):
         flush_database_caches()
         self.assertHasSuiteFiles(
             ("Contents-*", "Release"),
-            ("Contents-hppa", self.times[1], self.times[4]),
-            ("Release", self.times[2], self.times[3]),
-            ("Release", self.times[3], self.times[4]),
-            ("Release", self.times[4], None),
+            ("Contents-i386", 0, 2, 4),
+            ("Contents-hppa", 1, 4, None),
+            ("Release", 0, 1, 4),
+            ("Release", 1, 2, 4),
+            ("Release", 2, 3, None),
+            ("Release", 3, 4, None),
+            ("Release", 4, None, None),
         )
         releases.append(get_release_contents())
         self.assertThat(
@@ -3393,8 +3420,14 @@ class TestUpdateByHash(TestPublisherBase):
         flush_database_caches()
         self.assertHasSuiteFiles(
             ("Contents-*", "Release"),
-            ("Release", self.times[4], self.times[5]),
-            ("Release", self.times[5], None),
+            ("Contents-i386", 0, 2, 4),
+            ("Contents-hppa", 1, 4, 5),
+            ("Release", 0, 1, 4),
+            ("Release", 1, 2, 4),
+            ("Release", 2, 3, 5),
+            ("Release", 3, 4, 5),
+            ("Release", 4, 5, None),
+            ("Release", 5, None, None),
         )
         releases.append(get_release_contents())
         self.assertThat(suite_path("by-hash"), ByHashHasContents(releases[4:]))
@@ -3430,7 +3463,7 @@ class TestUpdateByHash(TestPublisherBase):
                 main_contents.add(f.read())
         self.assertHasSuiteFiles(
             ("main/source/Sources",),
-            ("main/source/Sources", self.times[0], None),
+            ("main/source/Sources", 0, None, None),
         )
 
         # Add a source package so that Sources is non-empty.
@@ -3447,8 +3480,8 @@ class TestUpdateByHash(TestPublisherBase):
         )
         self.assertHasSuiteFiles(
             ("main/source/Sources",),
-            ("main/source/Sources", self.times[0], self.times[1]),
-            ("main/source/Sources", self.times[1], None),
+            ("main/source/Sources", 0, 1, None),
+            ("main/source/Sources", 1, None, None),
         )
 
         # Delete the source package so that Sources is empty again.  The
@@ -3464,9 +3497,9 @@ class TestUpdateByHash(TestPublisherBase):
         )
         self.assertHasSuiteFiles(
             ("main/source/Sources",),
-            ("main/source/Sources", self.times[0], self.times[1]),
-            ("main/source/Sources", self.times[1], self.times[2]),
-            ("main/source/Sources", self.times[2], None),
+            ("main/source/Sources", 0, 1, None),
+            ("main/source/Sources", 1, 2, None),
+            ("main/source/Sources", 2, None, None),
         )
 
         # Make the first empty Sources file ready to prune.  This doesn't
@@ -3484,8 +3517,9 @@ class TestUpdateByHash(TestPublisherBase):
         )
         self.assertHasSuiteFiles(
             ("main/source/Sources",),
-            ("main/source/Sources", self.times[1], self.times[2]),
-            ("main/source/Sources", self.times[2], None),
+            ("main/source/Sources", 0, 1, 3),
+            ("main/source/Sources", 1, 2, None),
+            ("main/source/Sources", 2, None, None),
         )
 
     def setUpPruneableSuite(self):
@@ -3515,7 +3549,10 @@ class TestUpdateByHash(TestPublisherBase):
             for name in ("Release", "Sources.gz", "Sources.bz2"):
                 with open(suite_path("main", "source", name), "rb") as f:
                     main_contents.append(f.read())
-            self.advanceTime(delta=timedelta(hours=6))
+            # Advance time between each publisher run.  We don't advance
+            # time after the last one, since we'll do that below.
+            if sourcename != "baz":
+                self.advanceTime(delta=timedelta(hours=6))
         transaction.commit()
 
         # We have two condemned sets of index files and one uncondemned set.
@@ -3523,16 +3560,16 @@ class TestUpdateByHash(TestPublisherBase):
         # that it doesn't change.
         self.assertHasSuiteFiles(
             ("main/source/*", "Release"),
-            ("main/source/Sources.gz", self.times[0], self.times[1]),
-            ("main/source/Sources.gz", self.times[1], self.times[2]),
-            ("main/source/Sources.gz", self.times[2], None),
-            ("main/source/Sources.bz2", self.times[0], self.times[1]),
-            ("main/source/Sources.bz2", self.times[1], self.times[2]),
-            ("main/source/Sources.bz2", self.times[2], None),
-            ("main/source/Release", self.times[0], None),
-            ("Release", self.times[0], self.times[1]),
-            ("Release", self.times[1], self.times[2]),
-            ("Release", self.times[2], None),
+            ("main/source/Sources.gz", 0, 1, None),
+            ("main/source/Sources.gz", 1, 2, None),
+            ("main/source/Sources.gz", 2, None, None),
+            ("main/source/Sources.bz2", 0, 1, None),
+            ("main/source/Sources.bz2", 1, 2, None),
+            ("main/source/Sources.bz2", 2, None, None),
+            ("main/source/Release", 0, None, None),
+            ("Release", 0, 1, None),
+            ("Release", 1, 2, None),
+            ("Release", 2, None, None),
         )
         self.assertThat(suite_path("by-hash"), ByHashHasContents(top_contents))
         self.assertThat(
@@ -3574,13 +3611,16 @@ class TestUpdateByHash(TestPublisherBase):
         # generated.
         self.assertHasSuiteFiles(
             ("main/source/*", "Release"),
-            ("main/source/Sources.gz", self.times[1], self.times[2]),
-            ("main/source/Sources.gz", self.times[2], None),
-            ("main/source/Sources.bz2", self.times[1], self.times[2]),
-            ("main/source/Sources.bz2", self.times[2], None),
-            ("main/source/Release", self.times[0], None),
-            ("Release", self.times[1], self.times[2]),
-            ("Release", self.times[2], None),
+            ("main/source/Sources.gz", 0, 1, 3),
+            ("main/source/Sources.gz", 1, 2, None),
+            ("main/source/Sources.gz", 2, None, None),
+            ("main/source/Sources.bz2", 0, 1, 3),
+            ("main/source/Sources.bz2", 1, 2, None),
+            ("main/source/Sources.bz2", 2, None, None),
+            ("main/source/Release", 0, None, None),
+            ("Release", 0, 1, 3),
+            ("Release", 1, 2, None),
+            ("Release", 2, None, None),
         )
         self.assertThat(suite_path("by-hash"), ByHashHasContents(top_contents))
         self.assertThat(
@@ -3615,13 +3655,16 @@ class TestUpdateByHash(TestPublisherBase):
         # generated.
         self.assertHasSuiteFiles(
             ("main/source/*", "Release"),
-            ("main/source/Sources.gz", self.times[1], self.times[2]),
-            ("main/source/Sources.gz", self.times[2], None),
-            ("main/source/Sources.bz2", self.times[1], self.times[2]),
-            ("main/source/Sources.bz2", self.times[2], None),
-            ("main/source/Release", self.times[0], None),
-            ("Release", self.times[1], self.times[2]),
-            ("Release", self.times[2], None),
+            ("main/source/Sources.gz", 0, 1, 3),
+            ("main/source/Sources.gz", 1, 2, None),
+            ("main/source/Sources.gz", 2, None, None),
+            ("main/source/Sources.bz2", 0, 1, 3),
+            ("main/source/Sources.bz2", 1, 2, None),
+            ("main/source/Sources.bz2", 2, None, None),
+            ("main/source/Release", 0, None, None),
+            ("Release", 0, 1, 3),
+            ("Release", 1, 2, None),
+            ("Release", 2, None, None),
         )
         self.assertThat(suite_path("by-hash"), ByHashHasContents(top_contents))
         self.assertThat(
diff --git a/lib/lp/soyuz/interfaces/archivefile.py b/lib/lp/soyuz/interfaces/archivefile.py
index 512b07b..6c530b3 100644
--- a/lib/lp/soyuz/interfaces/archivefile.py
+++ b/lib/lp/soyuz/interfaces/archivefile.py
@@ -72,6 +72,15 @@ class IArchiveFile(Interface):
         readonly=False,
     )
 
+    date_removed = Datetime(
+        title=_(
+            "The date when this file was entirely removed from the published "
+            "archive."
+        ),
+        required=False,
+        readonly=False,
+    )
+
 
 class IArchiveFileSet(Interface):
     """Bulk operations on files in an archive."""
@@ -104,6 +113,7 @@ class IArchiveFileSet(Interface):
         path=None,
         sha256=None,
         condemned=None,
+        only_published=False,
         eager_load=False,
     ):
         """Get files in an archive.
@@ -119,6 +129,8 @@ class IArchiveFileSet(Interface):
             scheduled_deletion_date set; if False, return only files without
             a scheduled_deletion_date set; if None (the default), return
             both.
+        :param only_published: If True, return only files without a
+            `date_removed` set.
         :param eager_load: If True, preload related `LibraryFileAlias` and
             `LibraryFileContent` rows.
         :return: An iterable of matched files.
@@ -141,10 +153,12 @@ class IArchiveFileSet(Interface):
         :return: An iterable of matched container names.
         """
 
-    def delete(archive_files):
-        """Delete these archive files.
+    def markDeleted(archive_files):
+        """Mark these archive files as deleted.
+
+        This does not actually delete the rows from the database;
+        `lp.soyuz.scripts.expire_archive_files` will expire their
+        corresponding `LibraryFileAlias` rows as needed.
 
         :param archive_files: The `IArchiveFile`s to delete.
-        :return: An iterable of (container, path, sha256) for files that
-            were deleted.
         """
diff --git a/lib/lp/soyuz/model/archivefile.py b/lib/lp/soyuz/model/archivefile.py
index b7170dc..8272dd2 100644
--- a/lib/lp/soyuz/model/archivefile.py
+++ b/lib/lp/soyuz/model/archivefile.py
@@ -12,7 +12,7 @@ import os.path
 import re
 
 import pytz
-from storm.locals import And, DateTime, Int, Reference, Unicode
+from storm.locals import DateTime, Int, Reference, Unicode
 from zope.component import getUtility
 from zope.interface import implementer
 
@@ -20,7 +20,6 @@ from lp.services.database.bulk import load_related
 from lp.services.database.constants import UTC_NOW
 from lp.services.database.decoratedresultset import DecoratedResultSet
 from lp.services.database.interfaces import IPrimaryStore, IStore
-from lp.services.database.sqlbase import convert_storm_clause_to_string
 from lp.services.database.stormbase import StormBase
 from lp.services.database.stormexpr import RegexpMatch
 from lp.services.librarian.interfaces import ILibraryFileAliasSet
@@ -71,6 +70,10 @@ class ArchiveFile(StormBase):
         name="scheduled_deletion_date", tzinfo=pytz.UTC, allow_none=True
     )
 
+    date_removed = DateTime(
+        name="date_removed", tzinfo=pytz.UTC, allow_none=True
+    )
+
     def __init__(self, archive, container, path, library_file):
         """Construct an `ArchiveFile`."""
         super().__init__()
@@ -81,6 +84,7 @@ class ArchiveFile(StormBase):
         self.date_created = _now()
         self.date_superseded = None
         self.scheduled_deletion_date = None
+        self.date_removed = None
 
 
 @implementer(IArchiveFileSet)
@@ -116,6 +120,7 @@ class ArchiveFileSet:
         path_parent=None,
         sha256=None,
         condemned=None,
+        only_published=False,
         eager_load=False,
     ):
         """See `IArchiveFileSet`."""
@@ -145,6 +150,8 @@ class ArchiveFileSet:
                 clauses.append(ArchiveFile.scheduled_deletion_date != None)
             else:
                 clauses.append(ArchiveFile.scheduled_deletion_date == None)
+        if only_published:
+            clauses.append(ArchiveFile.date_removed == None)
         archive_files = IStore(ArchiveFile).find(ArchiveFile, *clauses)
 
         def eager_load(rows):
@@ -185,30 +192,13 @@ class ArchiveFileSet:
         )
 
     @staticmethod
-    def delete(archive_files):
+    def markDeleted(archive_files):
         """See `IArchiveFileSet`."""
-        # XXX cjwatson 2016-03-30 bug=322972: Requires manual SQL due to
-        # lack of support for DELETE FROM ... USING ... in Storm.
-        clauses = [
+        rows = IPrimaryStore(ArchiveFile).find(
+            ArchiveFile,
             ArchiveFile.id.is_in(
                 {archive_file.id for archive_file in archive_files}
             ),
-            ArchiveFile.library_file_id == LibraryFileAlias.id,
-            LibraryFileAlias.contentID == LibraryFileContent.id,
-        ]
-        where = convert_storm_clause_to_string(And(*clauses))
-        return list(
-            IPrimaryStore(ArchiveFile).execute(
-                """
-            DELETE FROM ArchiveFile
-            USING LibraryFileAlias, LibraryFileContent
-            WHERE """
-                + where
-                + """
-            RETURNING
-                ArchiveFile.container,
-                ArchiveFile.path,
-                LibraryFileContent.sha256
-            """
-            )
+            ArchiveFile.date_removed == None,
         )
+        rows.set(date_removed=_now())
diff --git a/lib/lp/soyuz/scripts/expire_archive_files.py b/lib/lp/soyuz/scripts/expire_archive_files.py
index c1c7e9f..13af377 100755
--- a/lib/lp/soyuz/scripts/expire_archive_files.py
+++ b/lib/lp/soyuz/scripts/expire_archive_files.py
@@ -13,6 +13,7 @@ from lp.services.librarian.model import LibraryFileAlias
 from lp.services.scripts.base import LaunchpadCronScript
 from lp.soyuz.enums import ArchivePurpose
 from lp.soyuz.model.archive import Archive
+from lp.soyuz.model.archivefile import ArchiveFile
 from lp.soyuz.model.files import BinaryPackageFile, SourcePackageReleaseFile
 from lp.soyuz.model.publishing import (
     BinaryPackagePublishingHistory,
@@ -98,19 +99,67 @@ class ArchiveExpirer(LaunchpadCronScript):
             ),
         )
 
-    def _determineExpirables(self, num_days, binary):
+    def _determineExpirables(self, num_days, table):
+        """Return expirable LibraryFileAlias IDs based on a given table."""
         stay_of_execution = Cast("%d days" % num_days, "interval")
         archive_types = (ArchivePurpose.PPA, ArchivePurpose.PARTNER)
+        eligible_clauses = []
+        denied_clauses = []
 
         LFA = LibraryFileAlias
-        if binary:
-            xPF = BinaryPackageFile
-            xPPH = BinaryPackagePublishingHistory
-            xPR_join = xPF.binarypackagerelease == xPPH.binarypackagereleaseID
+        if table == BinaryPackageFile:
+            BPF = BinaryPackageFile
+            BPPH = BinaryPackagePublishingHistory
+            lfa_column = BPF.libraryfile_id
+            date_removed_column = BPPH.dateremoved
+            eligible_clauses.extend(
+                [
+                    BPF.libraryfile == LFA.id,
+                    BPF.binarypackagerelease == BPPH.binarypackagereleaseID,
+                    BPPH.archive == Archive.id,
+                ]
+            )
+            denied_clauses.extend(
+                [
+                    BPF.binarypackagerelease == BPPH.binarypackagereleaseID,
+                    BPPH.archive == Archive.id,
+                ]
+            )
+        elif table == SourcePackageReleaseFile:
+            SPRF = SourcePackageReleaseFile
+            SPPH = SourcePackagePublishingHistory
+            lfa_column = SPRF.libraryfile_id
+            date_removed_column = SPPH.dateremoved
+            eligible_clauses.extend(
+                [
+                    SPRF.libraryfile == LFA.id,
+                    SPRF.sourcepackagerelease == SPPH.sourcepackagereleaseID,
+                    SPPH.archive == Archive.id,
+                ]
+            )
+            denied_clauses.extend(
+                [
+                    SPRF.sourcepackagerelease == SPPH.sourcepackagereleaseID,
+                    SPPH.archive == Archive.id,
+                ]
+            )
+        elif table == ArchiveFile:
+            lfa_column = ArchiveFile.library_file_id
+            date_removed_column = ArchiveFile.date_removed
+            eligible_clauses.extend(
+                [
+                    ArchiveFile.library_file == LFA.id,
+                    ArchiveFile.date_removed < UTC_NOW - stay_of_execution,
+                    ArchiveFile.archive == Archive.id,
+                ]
+            )
+            # Unlike BPF and SPRF, ArchiveFile rows don't currently share
+            # LFAs (although they may share LFCs after deduplication).
+            # However, we still use denied_clauses to implement
+            # archive-based expiry exceptions.
+            denied_clauses.append(ArchiveFile.archive == Archive.id)
         else:
-            xPF = SourcePackageReleaseFile
-            xPPH = SourcePackagePublishingHistory
-            xPR_join = xPF.sourcepackagerelease == xPPH.sourcepackagereleaseID
+            raise AssertionError("Unknown table: %r" % table)
         full_archive_name = Concatenate(
             Person.name, Concatenate("/", Archive.name)
         )
@@ -121,19 +170,16 @@ class ArchiveExpirer(LaunchpadCronScript):
         eligible = Select(
             LFA.id,
             where=And(
-                xPF.libraryfile == LFA.id,
-                xPR_join,
-                xPPH.dateremoved < UTC_NOW - stay_of_execution,
-                xPPH.archive == Archive.id,
+                *eligible_clauses,
+                date_removed_column < UTC_NOW - stay_of_execution,
                 Archive.purpose.is_in(archive_types),
                 LFA.expires == None,
             ),
         )
         denied = Select(
-            xPF.libraryfile_id,
+            lfa_column,
             where=And(
-                xPR_join,
-                xPPH.archive == Archive.id,
+                *denied_clauses,
                 Archive.owner == Person.id,
                 Or(
                     And(
@@ -148,21 +194,13 @@ class ArchiveExpirer(LaunchpadCronScript):
                         Not(full_archive_name.is_in(self.always_expire)),
                     ),
                     Not(Archive.purpose.is_in(archive_types)),
-                    xPPH.dateremoved > UTC_NOW - stay_of_execution,
-                    xPPH.dateremoved == None,
+                    date_removed_column > UTC_NOW - stay_of_execution,
+                    date_removed_column == None,
                 ),
             ),
         )
         return list(self.store.execute(Except(eligible, denied)))
 
-    def determineSourceExpirables(self, num_days):
-        """Return expirable libraryfilealias IDs."""
-        return self._determineExpirables(num_days=num_days, binary=False)
-
-    def determineBinaryExpirables(self, num_days):
-        """Return expirable libraryfilealias IDs."""
-        return self._determineExpirables(num_days=num_days, binary=True)
-
     def main(self):
         self.logger.info("Starting the PPA binary expiration")
         num_days = self.options.num_days
@@ -170,8 +208,13 @@ class ArchiveExpirer(LaunchpadCronScript):
 
         self.store = IStore(Archive)
 
-        lfa_ids = self.determineSourceExpirables(num_days)
-        lfa_ids.extend(self.determineBinaryExpirables(num_days))
+        lfa_ids = []
+        for table in (
+            BinaryPackageFile,
+            SourcePackageReleaseFile,
+            ArchiveFile,
+        ):
+            lfa_ids.extend(self._determineExpirables(num_days, table))
         batch_count = 0
         batch_limit = 500
         for id in lfa_ids:
diff --git a/lib/lp/soyuz/scripts/tests/test_expire_archive_files.py b/lib/lp/soyuz/scripts/tests/test_expire_archive_files.py
index 12403fd..63c85b4 100644
--- a/lib/lp/soyuz/scripts/tests/test_expire_archive_files.py
+++ b/lib/lp/soyuz/scripts/tests/test_expire_archive_files.py
@@ -54,7 +54,7 @@ class ArchiveExpiryTestBase(TestCaseWithFactory):
         script.main()
 
     def _setUpExpirablePublications(self, archive=None):
-        """Helper to set up two publications that are both expirable."""
+        """Helper to set up publications and indexes that are all expirable."""
         if archive is None:
             archive = self.archive
         pkg5 = self.stp.getPubSource(
@@ -78,7 +78,21 @@ class ArchiveExpiryTestBase(TestCaseWithFactory):
             self.archive2.distribution.currentseries, pub.pocket, self.archive2
         )
         other_binary.dateremoved = self.over_threshold_date
-        return pkg5, pub
+        af = self.factory.makeArchiveFile(
+            archive=archive,
+            container="release:",
+            path="dists/%s/Release" % pkg5.distroseries.getSuite(pkg5.pocket),
+            date_removed=self.over_threshold_date,
+        )
+        self.factory.makeArchiveFile(
+            archive=self.archive2,
+            container="release:",
+            path="dists/%s/Release"
+            % self.archive2.distribution.currentseries.getSuite(pub.pocket),
+            library_file=af.library_file,
+            date_removed=self.over_threshold_date,
+        )
+        return pkg5, pub, af
 
     def assertBinaryExpired(self, publication):
         self.assertNotEqual(
@@ -108,6 +122,18 @@ class ArchiveExpiryTestBase(TestCaseWithFactory):
             "lfa.expires should be None, but it's not.",
         )
 
+    def assertIndexExpired(self, archive_file):
+        self.assertIsNotNone(
+            archive_file.library_file.expires,
+            "lfa.expires should be set, but it's not.",
+        )
+
+    def assertIndexNotExpired(self, archive_file):
+        self.assertIsNone(
+            archive_file.library_file.expires,
+            "lfa.expires should be None, but it's not.",
+        )
+
 
 class ArchiveExpiryCommonTests:
     """Common source/binary expiration test cases.
@@ -126,10 +152,16 @@ class ArchiveExpiryCommonTests:
         [pub] = self.stp.getPubBinaries(
             pub_source=pkg1, dateremoved=None, archive=self.archive
         )
+        af = self.factory.makeArchiveFile(
+            archive=self.archive,
+            container="release:",
+            path="dists/%s/Release" % pkg1.distroseries.getSuite(pkg1.pocket),
+        )
 
         self.runScript()
         self.assertSourceNotExpired(pkg1)
         self.assertBinaryNotExpired(pub)
+        self.assertIndexNotExpired(af)
 
     def testNoExpirationWithDateUnderThreshold(self):
         """Test no expiring if dateremoved too recent."""
@@ -144,10 +176,17 @@ class ArchiveExpiryCommonTests:
             dateremoved=self.under_threshold_date,
             archive=self.archive,
         )
+        af = self.factory.makeArchiveFile(
+            archive=self.archive,
+            container="release:",
+            path="dists/%s/Release" % pkg2.distroseries.getSuite(pkg2.pocket),
+            date_removed=self.under_threshold_date,
+        )
 
         self.runScript()
         self.assertSourceNotExpired(pkg2)
         self.assertBinaryNotExpired(pub)
+        self.assertIndexNotExpired(af)
 
     def testExpirationWithDateOverThreshold(self):
         """Test expiring works if dateremoved old enough."""
@@ -162,10 +201,17 @@ class ArchiveExpiryCommonTests:
             dateremoved=self.over_threshold_date,
             archive=self.archive,
         )
+        af = self.factory.makeArchiveFile(
+            archive=self.archive,
+            container="release:",
+            path="dists/%s/Release" % pkg3.distroseries.getSuite(pkg3.pocket),
+            date_removed=self.over_threshold_date,
+        )
 
         self.runScript()
         self.assertSourceExpired(pkg3)
         self.assertBinaryExpired(pub)
+        self.assertIndexExpired(af)
 
     def testNoExpirationWithDateOverThresholdAndOtherValidPublication(self):
         """Test no expiry if dateremoved old enough but other publication."""
@@ -190,10 +236,23 @@ class ArchiveExpiryCommonTests:
             self.archive2.distribution.currentseries, pub.pocket, self.archive2
         )
         other_binary.dateremoved = None
+        af = self.factory.makeArchiveFile(
+            archive=self.archive,
+            container="release:",
+            path="dists/%s/Release" % pkg4.distroseries.getSuite(pkg4.pocket),
+            date_removed=self.over_threshold_date,
+        )
+        self.factory.makeArchiveFile(
+            archive=self.archive,
+            container="release:",
+            path="dists/%s/Release" % pkg4.distroseries.getSuite(pkg4.pocket),
+            library_file=af.library_file,
+        )
 
         self.runScript()
         self.assertSourceNotExpired(pkg4)
         self.assertBinaryNotExpired(pub)
+        self.assertIndexNotExpired(af)
 
     def testNoExpirationWithDateOverThresholdAndOtherPubUnderThreshold(self):
         """Test no expiring.
@@ -222,10 +281,24 @@ class ArchiveExpiryCommonTests:
             self.archive2.distribution.currentseries, pub.pocket, self.archive2
         )
         other_binary.dateremoved = self.under_threshold_date
+        af = self.factory.makeArchiveFile(
+            archive=self.archive,
+            container="release:",
+            path="dists/%s/Release" % pkg5.distroseries.getSuite(pkg5.pocket),
+            date_removed=self.over_threshold_date,
+        )
+        self.factory.makeArchiveFile(
+            archive=self.archive,
+            container="release:",
+            path="dists/%s/Release" % pkg5.distroseries.getSuite(pkg5.pocket),
+            library_file=af.library_file,
+            date_removed=self.under_threshold_date,
+        )
 
         self.runScript()
         self.assertSourceNotExpired(pkg5)
         self.assertBinaryNotExpired(pub)
+        self.assertIndexNotExpired(af)
 
     def testNoExpirationWithDateOverThresholdAndOtherPubOverThreshold(self):
         """Test expiring works.
@@ -233,14 +306,15 @@ class ArchiveExpiryCommonTests:
         Test expiring works if dateremoved old enough and other publication
         is over date threshold.
         """
-        source, binary = self._setUpExpirablePublications()
+        source, binary, index = self._setUpExpirablePublications()
         self.runScript()
         self.assertSourceExpired(source)
         self.assertBinaryExpired(binary)
+        self.assertIndexExpired(index)
 
     def testDryRun(self):
         """Test that when dryrun is specified, nothing is expired."""
-        source, binary = self._setUpExpirablePublications()
+        source, binary, index = self._setUpExpirablePublications()
         # We have to commit here otherwise when the script aborts it
         # will remove the test publications we just created.
         self.layer.txn.commit()
@@ -249,16 +323,20 @@ class ArchiveExpiryCommonTests:
         script.main()
         self.assertSourceNotExpired(source)
         self.assertBinaryNotExpired(binary)
+        self.assertIndexNotExpired(index)
 
     def testDoesNotAffectPrimary(self):
         """Test that expiry does not happen for non-PPA publications."""
         primary_archive = getUtility(IDistributionSet)[
             "ubuntutest"
         ].main_archive
-        source, binary = self._setUpExpirablePublications(primary_archive)
+        source, binary, index = self._setUpExpirablePublications(
+            primary_archive
+        )
         self.runScript()
         self.assertSourceNotExpired(source)
         self.assertBinaryNotExpired(binary)
+        self.assertIndexNotExpired(index)
 
 
 class TestPPAExpiry(ArchiveExpiryTestBase, ArchiveExpiryCommonTests):
@@ -280,7 +358,9 @@ class TestPPAExpiry(ArchiveExpiryTestBase, ArchiveExpiryCommonTests):
 
     def testNeverExpireWorks(self):
         """Test that never-expiring PPA owners are not expired."""
-        source, binary = self._setUpExpirablePublications(archive=self.archive)
+        source, binary, index = self._setUpExpirablePublications(
+            archive=self.archive
+        )
         script = self.getScript()
         script.never_expire = [
             self.archive.owner.name,
@@ -289,10 +369,13 @@ class TestPPAExpiry(ArchiveExpiryTestBase, ArchiveExpiryCommonTests):
         script.main()
         self.assertSourceNotExpired(source)
         self.assertBinaryNotExpired(binary)
+        self.assertIndexNotExpired(index)
 
     def testNeverExpireArchivesWorks(self):
         """Test that never-expiring individual PPAs are not expired."""
-        source, binary = self._setUpExpirablePublications(archive=self.archive)
+        source, binary, index = self._setUpExpirablePublications(
+            archive=self.archive
+        )
         script = self.getScript()
         script.never_expire = [
             "%s/%s" % (self.archive.owner.name, self.archive.name)
@@ -301,25 +384,28 @@ class TestPPAExpiry(ArchiveExpiryTestBase, ArchiveExpiryCommonTests):
         script.main()
         self.assertSourceNotExpired(source)
         self.assertBinaryNotExpired(binary)
+        self.assertIndexNotExpired(index)
 
     def testAlwaysExpireWorks(self):
         """Test that always-expiring private PPAs are expired anyway."""
         p3a = self.factory.makeArchive(private=True)
-        source, binary = self._setUpExpirablePublications(archive=p3a)
+        source, binary, index = self._setUpExpirablePublications(archive=p3a)
         script = self.getScript()
         script.always_expire = ["%s/%s" % (p3a.owner.name, p3a.name)]
         switch_dbuser(self.dbuser)
         script.main()
         self.assertSourceExpired(source)
         self.assertBinaryExpired(binary)
+        self.assertIndexExpired(index)
 
     def testPrivatePPAsNotExpired(self):
         """Test that private PPAs are not expired."""
         self.archive.private = True
-        source, binary = self._setUpExpirablePublications()
+        source, binary, index = self._setUpExpirablePublications()
         self.runScript()
         self.assertSourceNotExpired(source)
         self.assertBinaryNotExpired(binary)
+        self.assertIndexNotExpired(index)
 
 
 class TestPartnerExpiry(ArchiveExpiryTestBase, ArchiveExpiryCommonTests):
diff --git a/lib/lp/soyuz/tests/test_archivefile.py b/lib/lp/soyuz/tests/test_archivefile.py
index 8a7ea20..c4e2a57 100644
--- a/lib/lp/soyuz/tests/test_archivefile.py
+++ b/lib/lp/soyuz/tests/test_archivefile.py
@@ -251,22 +251,19 @@ class TestArchiveFile(TestCaseWithFactory):
             ),
         )
 
-    def test_delete(self):
+    def test_markDeleted(self):
         archive = self.factory.makeArchive()
         archive_files = [
             self.factory.makeArchiveFile(archive=archive) for _ in range(4)
         ]
-        expected_rows = [
-            (
-                archive_file.container,
-                archive_file.path,
-                archive_file.library_file.content.sha256,
-            )
-            for archive_file in archive_files[:2]
-        ]
         archive_file_set = getUtility(IArchiveFileSet)
-        rows = archive_file_set.delete(archive_files[:2])
-        self.assertContentEqual(expected_rows, rows)
+        archive_file_set.markDeleted(archive_files[:2])
+        flush_database_caches()
+        self.assertIsNotNone(archive_files[0].date_removed)
+        self.assertIsNotNone(archive_files[1].date_removed)
+        self.assertIsNone(archive_files[2].date_removed)
+        self.assertIsNone(archive_files[3].date_removed)
         self.assertContentEqual(
-            archive_files[2:], archive_file_set.getByArchive(archive)
+            archive_files[2:],
+            archive_file_set.getByArchive(archive, only_published=True),
         )
diff --git a/lib/lp/soyuz/xmlrpc/archive.py b/lib/lp/soyuz/xmlrpc/archive.py
index d5e5f69..f9a35a8 100644
--- a/lib/lp/soyuz/xmlrpc/archive.py
+++ b/lib/lp/soyuz/xmlrpc/archive.py
@@ -136,6 +136,8 @@ class ArchiveAPI(LaunchpadXMLRPCView):
         if checksum_type != "SHA256":
             return None
 
+        # This implicitly includes a check that the associated LFA isn't
+        # deleted, by way of joining with LFC to check the checksum.
         archive_file = (
             getUtility(IArchiveFileSet)
             .getByArchive(
@@ -167,7 +169,7 @@ class ArchiveAPI(LaunchpadXMLRPCView):
             )
             .one()
         )
-        if archive_file is None:
+        if archive_file is None or archive_file.library_file.deleted:
             return None
 
         log.info(
@@ -182,7 +184,7 @@ class ArchiveAPI(LaunchpadXMLRPCView):
         self, archive_reference: str, archive, path: PurePath
     ) -> Optional[str]:
         lfa = archive.getPoolFileByPath(path)
-        if lfa is None:
+        if lfa is None or lfa.deleted:
             return None
 
         log.info(
@@ -220,20 +222,17 @@ class ArchiveAPI(LaunchpadXMLRPCView):
                 return url
 
         # Consider other non-pool files.
-        if path.parts[0] != "pool":
+        elif path.parts[0] != "pool":
             url = self._translatePathNonPool(archive_reference, archive, path)
             if url is not None:
                 return url
-            log.info("%s: %s not found", archive_reference, path.as_posix())
-            raise faults.NotFound(
-                message="'%s' not found in '%s'."
-                % (path.as_posix(), archive_reference)
-            )
 
         # Consider pool files.
-        url = self._translatePathPool(archive_reference, archive, path)
-        if url is not None:
-            return url
+        else:
+            url = self._translatePathPool(archive_reference, archive, path)
+            if url is not None:
+                return url
+
         log.info("%s: %s not found", archive_reference, path.as_posix())
         raise faults.NotFound(
             message="'%s' not found in '%s'."
diff --git a/lib/lp/soyuz/xmlrpc/tests/test_archive.py b/lib/lp/soyuz/xmlrpc/tests/test_archive.py
index 1dffd43..5dd81a4 100644
--- a/lib/lp/soyuz/xmlrpc/tests/test_archive.py
+++ b/lib/lp/soyuz/xmlrpc/tests/test_archive.py
@@ -322,6 +322,26 @@ class TestArchiveAPI(TestCaseWithFactory):
             path,
         )
 
+    def test_translatePath_by_hash_checksum_expired(self):
+        archive = removeSecurityProxy(self.factory.makeArchive())
+        archive_file = self.factory.makeArchiveFile(
+            archive=archive,
+            container="release:jammy",
+            path="dists/jammy/InRelease",
+        )
+        path = (
+            "dists/jammy/by-hash/SHA256/%s"
+            % archive_file.library_file.content.sha256
+        )
+        removeSecurityProxy(archive_file.library_file).content = None
+        self.assertNotFound(
+            "translatePath",
+            "'%s' not found in '%s'." % (path, archive.reference),
+            "%s: %s not found" % (archive.reference, path),
+            archive.reference,
+            path,
+        )
+
     def test_translatePath_by_hash_checksum_found(self):
         archive = removeSecurityProxy(self.factory.makeArchive())
         now = get_transaction_timestamp(IStore(archive))
@@ -389,6 +409,19 @@ class TestArchiveAPI(TestCaseWithFactory):
             "nonexistent/path",
         )
 
+    def test_translatePath_non_pool_expired(self):
+        archive = removeSecurityProxy(self.factory.makeArchive())
+        path = "dists/focal/InRelease"
+        archive_file = self.factory.makeArchiveFile(archive=archive, path=path)
+        removeSecurityProxy(archive_file.library_file).content = None
+        self.assertNotFound(
+            "translatePath",
+            "'%s' not found in '%s'." % (path, archive.reference),
+            "%s: %s not found" % (archive.reference, path),
+            archive.reference,
+            path,
+        )
+
     def test_translatePath_non_pool_found(self):
         archive = removeSecurityProxy(self.factory.makeArchive())
         now = get_transaction_timestamp(IStore(archive))
@@ -471,6 +504,36 @@ class TestArchiveAPI(TestCaseWithFactory):
             path,
         )
 
+    def test_translatePath_pool_source_expired(self):
+        archive = removeSecurityProxy(self.factory.makeArchive())
+        spph = self.factory.makeSourcePackagePublishingHistory(
+            archive=archive,
+            status=PackagePublishingStatus.PUBLISHED,
+            sourcepackagename="test-package",
+            component="main",
+        )
+        sprf = self.factory.makeSourcePackageReleaseFile(
+            sourcepackagerelease=spph.sourcepackagerelease,
+            library_file=self.factory.makeLibraryFileAlias(
+                filename="test-package_1.dsc", db_only=True
+            ),
+        )
+        self.factory.makeSourcePackageReleaseFile(
+            sourcepackagerelease=spph.sourcepackagerelease,
+            library_file=self.factory.makeLibraryFileAlias(
+                filename="test-package_1.tar.xz", db_only=True
+            ),
+        )
+        removeSecurityProxy(sprf.libraryfile).content = None
+        path = "pool/main/t/test-package/test-package_1.dsc"
+        self.assertNotFound(
+            "translatePath",
+            "'%s' not found in '%s'." % (path, archive.reference),
+            "%s: %s not found" % (archive.reference, path),
+            archive.reference,
+            path,
+        )
+
     def test_translatePath_pool_source_found(self):
         archive = removeSecurityProxy(self.factory.makeArchive())
         spph = self.factory.makeSourcePackagePublishingHistory(
@@ -551,6 +614,30 @@ class TestArchiveAPI(TestCaseWithFactory):
             path,
         )
 
+    def test_translatePath_pool_binary_expired(self):
+        archive = removeSecurityProxy(self.factory.makeArchive())
+        bpph = self.factory.makeBinaryPackagePublishingHistory(
+            archive=archive,
+            status=PackagePublishingStatus.PUBLISHED,
+            sourcepackagename="test-package",
+            component="main",
+        )
+        bpf = self.factory.makeBinaryPackageFile(
+            binarypackagerelease=bpph.binarypackagerelease,
+            library_file=self.factory.makeLibraryFileAlias(
+                filename="test-package_1_amd64.deb", db_only=True
+            ),
+        )
+        removeSecurityProxy(bpf.libraryfile).content = None
+        path = "pool/main/t/test-package/test-package_1_amd64.deb"
+        self.assertNotFound(
+            "translatePath",
+            "'%s' not found in '%s'." % (path, archive.reference),
+            "%s: %s not found" % (archive.reference, path),
+            archive.reference,
+            path,
+        )
+
     def test_translatePath_pool_binary_found(self):
         archive = removeSecurityProxy(self.factory.makeArchive())
         bpph = self.factory.makeBinaryPackagePublishingHistory(
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index 62cd634..54ab372 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -3797,6 +3797,7 @@ class LaunchpadObjectFactory(ObjectFactory):
         library_file=None,
         date_superseded=None,
         scheduled_deletion_date=None,
+        date_removed=None,
     ):
         if archive is None:
             archive = self.makeArchive()
@@ -3818,6 +3819,8 @@ class LaunchpadObjectFactory(ObjectFactory):
             removeSecurityProxy(
                 archive_file
             ).scheduled_deletion_date = scheduled_deletion_date
+        if date_removed is not None:
+            removeSecurityProxy(archive_file).date_removed = date_removed
         return archive_file
 
     def makeBuilder(