[PULP-2346] Use correlated EXISTS membership lookups and add relative_path index

CHANGES/+optimize-content-membership-queries.bugfix

@@ -0,0 +1 @@
+Sped up content app lookups (the `index.html` probe, directory listings and exact file lookups) by checking repository membership per matching row instead of across the whole repository version, backed by a new index on `ContentArtifact.relative_path`.

pulpcore/app/migrations/0162_contentartifact_relative_path_pattern_index.py

@@ -0,0 +1,100 @@
+import re
+
+import django.contrib.postgres.indexes
+from django.db import migrations, models
+
+_INDEX_NAME = "core_ca_relpath_pattern_idx"
+_INDEX_MARKER = "pulpcore:0162:core_ca_relpath_pattern_idx"
+_CREATE_INDEX_SQL = (
+    'CREATE INDEX CONCURRENTLY IF NOT EXISTS "core_ca_relpath_pattern_idx" '
+    'ON "core_contentartifact" USING btree ("relative_path" text_pattern_ops)'
+)
+_DROP_INDEX_SQL = 'DROP INDEX CONCURRENTLY IF EXISTS "core_ca_relpath_pattern_idx"'
+# pg_get_indexdef output for the expected index, e.g.
+# CREATE INDEX core_ca_relpath_pattern_idx ON public.core_contentartifact USING btree
+# (relative_path text_pattern_ops)
+_EXPECTED_INDEX_DEF = re.compile(
+    r'^CREATE INDEX "?core_ca_relpath_pattern_idx"? ON (?:"?\w+"?\.)?"?core_contentartifact"? '
+    r'USING btree \("?relative_path"? text_pattern_ops\)$'
+)
+_COMMENT_INDEX_SQL = (
+    'COMMENT ON INDEX "core_ca_relpath_pattern_idx" '
+    "IS 'pulpcore:0162:core_ca_relpath_pattern_idx'"
+)
+
+
+def _index_info(cursor):
+    cursor.execute(
+        """
+        SELECT
+            pg_index.indisvalid,
+            obj_description(indexrel.oid, 'pg_class'),
+            pg_get_indexdef(indexrel.oid)
+        FROM pg_class AS indexrel
+        JOIN pg_index ON pg_index.indexrelid = indexrel.oid
+        JOIN pg_namespace AS namespace ON namespace.oid = indexrel.relnamespace
+        WHERE indexrel.relname = %s AND namespace.nspname = current_schema()
+        """,
+        [_INDEX_NAME],
+    )
+    return cursor.fetchone()
+
+
+def create_relative_path_index(apps, schema_editor):
+    """Create the index, reusing an equivalent one that already exists (e.g. created out of band)."""
+    with schema_editor.connection.cursor() as cursor:
+        info = _index_info(cursor)
+        if info:
+            valid, _, definition = info
+            if not valid:
+                raise RuntimeError(
+                    f"Index {_INDEX_NAME!r} exists but is invalid; refusing to drop it automatically."
+                )
+            if not _EXPECTED_INDEX_DEF.match(definition):
+                raise RuntimeError(
+                    f"Index {_INDEX_NAME!r} exists with an unexpected definition ({definition!r}); "
+                    "refusing to reuse or drop it automatically."
+                )
+            return
+
+        # IF NOT EXISTS keeps concurrent migration runs from creating a second physical index.
+        cursor.execute(_CREATE_INDEX_SQL)
+        # The comment records ownership so the reverse migration only drops what it created.
+        cursor.execute(_COMMENT_INDEX_SQL)
+
+
+def drop_relative_path_index(apps, schema_editor):
+    """Drop only an index created by this migration, leaving pre-existing ones intact."""
+    with schema_editor.connection.cursor() as cursor:
+        info = _index_info(cursor)
+        if info and info[1] == _INDEX_MARKER:
+            cursor.execute(_DROP_INDEX_SQL)
+
+
+class Migration(migrations.Migration):
+    """Add the index used by the content handler's relative_path prefix and equality lookups."""
+
+    atomic = False  # CREATE/DROP INDEX CONCURRENTLY cannot run in a transaction.
+
+    dependencies = [
+        ("core", "0161_upstreampulp_remote_policy"),
+    ]
+
+    operations = [
+        migrations.SeparateDatabaseAndState(
+            state_operations=[
+                migrations.AddIndex(
+                    model_name="contentartifact",
+                    index=models.Index(
+                        django.contrib.postgres.indexes.OpClass(
+                            "relative_path", name="text_pattern_ops"
+                        ),
+                        name=_INDEX_NAME,
+                    ),
+                ),
+            ],
+            database_operations=[
+                migrations.RunPython(create_relative_path_index, drop_relative_path_index),
+            ],
+        ),
+    ]

pulpcore/app/models/content.py

@@ -16,7 +16,7 @@
 
 from django.conf import settings
 from django.contrib.postgres.fields import HStoreField
-from django.contrib.postgres.indexes import GinIndex
+from django.contrib.postgres.indexes import GinIndex, OpClass
 from django.core import validators
 from django.db import IntegrityError, models, transaction
 from django.forms.models import model_to_dict
@@ -663,7 +663,13 @@ class ContentArtifact(BaseModel, QueryMixin):
 
     class Meta:
         unique_together = ("content", "relative_path")
-        indexes = [models.Index(fields=["relative_path"])]
+        indexes = [
+            models.Index(fields=["relative_path"]),
+            models.Index(
+                OpClass("relative_path", name="text_pattern_ops"),
+                name="core_ca_relpath_pattern_idx",
+            ),
+        ]
 
     @staticmethod
     def sort_key(ca):

pulpcore/content/handler.py

@@ -46,6 +46,7 @@
     models,
     transaction,
 )
+from django.db.models import Exists, OuterRef  # noqa: E402
 from jinja2 import Template  # noqa: E402
 
 from pulpcore.app import mime_types  # noqa: E402
@@ -712,7 +713,12 @@ def list_directory_blocking():
 
             if repo_version or publication.pass_through:
                 cas = ContentArtifact.objects.select_related("artifact").filter(
-                    content__in=content_repo_ver.content, relative_path__startswith=path
+                    Exists(
+                        content_repo_ver._content_relationships().filter(
+                            content_id=OuterRef("content_id")
+                        )
+                    ),
+                    relative_path__startswith=path,
                 )
                 for ca in cas:
                     name = file_or_directory_name(path, ca.relative_path)
@@ -898,9 +904,14 @@ async def _match_and_stream(self, path, request):
                             "artifact", "artifact__pulp_domain"
                         )
                         .filter(
-                            content__in=publication.repository_version.content,
+                            Exists(
+                                publication.repository_version._content_relationships().filter(
+                                    content_id=OuterRef("content_id")
+                                )
+                            ),
+                            relative_path=original_rel_path,
                         )
-                        .aget(relative_path=original_rel_path)
+                        .aget()
                     )
 
                 except MultipleObjectsReturned:
@@ -938,9 +949,14 @@ async def _match_and_stream(self, path, request):
             # Look for index.html or list the directory
             index_path = "{}index.html".format(rel_path)
 
-            contentartifact_exists = await ContentArtifact.objects.filter(
-                content__in=repo_version.content, relative_path=index_path
-            ).aexists()
+            # Existence-only probe: select just the pk and stop at the first match.
+            index_matches = ContentArtifact.objects.filter(
+                Exists(
+                    repo_version._content_relationships().filter(content_id=OuterRef("content_id"))
+                ),
+                relative_path=index_path,
+            ).values_list("pk", flat=True)[:1]
+            contentartifact_exists = bool([pk async for pk in index_matches])
             if contentartifact_exists:
                 original_rel_path = index_path
                 headers = self.response_headers(original_rel_path, distro)
@@ -961,9 +977,20 @@ async def _match_and_stream(self, path, request):
                     )
 
             try:
-                ca = await ContentArtifact.objects.select_related(
-                    "artifact", "artifact__pulp_domain"
-                ).aget(content__in=repo_version.content, relative_path=original_rel_path)
+                ca = (
+                    await ContentArtifact.objects.select_related(
+                        "artifact", "artifact__pulp_domain"
+                    )
+                    .filter(
+                        Exists(
+                            repo_version._content_relationships().filter(
+                                content_id=OuterRef("content_id")
+                            )
+                        ),
+                        relative_path=original_rel_path,
+                    )
+                    .aget()
+                )
 
             except MultipleObjectsReturned:
                 log.error(

pulpcore/tests/unit/content/test_handler.py

@@ -308,7 +308,11 @@ async def test_list_directory_metadata(monkeypatch, use_publication):
         SimpleNamespace(objects=remote_artifact_manager),
     )
 
-    repo_version = SimpleNamespace(content=[], _content_relationships=lambda: [])
+    class Relationships(list):
+        def filter(self, *args, **kwargs):
+            return Mock()
+
+    repo_version = SimpleNamespace(content=[], _content_relationships=lambda: Relationships())
 
     if use_publication:
         publication_manager = Mock()