OPDS views perf: Stage 2 — re-enable route caching

codex/urls/const.py

@@ -4,4 +4,11 @@
 BROWSER_TIMEOUT = 60 * 5
 COVER_MAX_AGE = 60 * 60 * 24 * 7
 PAGE_MAX_AGE = COVER_MAX_AGE
-OPDS_TIMEOUT = 0  # BROWSER_TIMEOUT
+# 60 s — long enough to amortize the full feed pipeline across a tab
+# refresh / reader app re-fetch, short enough that bookmark-position
+# changes show up before the next poll. Per-route ``vary_on_headers``
+# scopes the cache key so per-user feeds don't leak across sessions
+# (sub-plan 01 #1). Was previously 0 (cache_page no-op) for an
+# unattributed reason — the disable rationale is reconstructed in
+# tasks/opds-views-perf/stage0.md.
+OPDS_TIMEOUT = 60

codex/urls/opds/__init__.py

@@ -1 +1,22 @@
 """OPDS urls."""
+
+from django.views.decorators.cache import cache_page
+from django.views.decorators.vary import vary_on_headers
+
+from codex.urls.const import OPDS_TIMEOUT
+
+
+def opds_cached(view):
+    """
+    Wrap an OPDS feed/manifest/start view with cache_page + vary headers.
+
+    OPDS accepts Basic, Bearer, and Session auth, so the cache key must
+    vary on Cookie and Authorization to keep per-user feeds from leaking
+    across users / auth schemes (mirrors ``codex/urls/opds/binary.py``'s
+    cover-route composition; sub-plan 01 #1).
+
+    Used by ``v1.py``, ``v2.py``, and ``root.py``'s ``/opds/v2.0`` entry.
+    Progression and binary routes are NOT wrapped — see their respective
+    modules for the rationale.
+    """
+    return cache_page(OPDS_TIMEOUT)(vary_on_headers("Cookie", "Authorization")(view))

codex/urls/opds/root.py

@@ -3,6 +3,7 @@
 from django.urls import include, path, re_path
 from django.views.generic.base import RedirectView
 
+from codex.urls.opds import opds_cached
 from codex.views.opds.v2.feed import OPDS2StartView
 
 app_name = "opds"
@@ -15,9 +16,13 @@
     ),
     path("bin/", include("codex.urls.opds.binary")),
     path("v1.2/", include("codex.urls.opds.v1")),
+    # The trailing-slash variant ``/opds/v2.0/`` is mounted via
+    # ``codex.urls.opds.v2``; this no-trailing-slash entry mirrors
+    # the same cache_page + vary_on_headers composition so the start
+    # page benefits from caching at both URLs (sub-plan 01 #1).
     path(
         "v2.0",
-        OPDS2StartView.as_view(),
+        opds_cached(OPDS2StartView.as_view()),
         {"group": "r", "pks": (0,), "page": 1},
         name="start",
     ),

codex/urls/opds/v1.py

@@ -1,9 +1,8 @@
 """codex:opds:v1 URL Configuration."""
 
 from django.urls import path
-from django.views.decorators.cache import cache_page
 
-from codex.urls.const import OPDS_TIMEOUT
+from codex.urls.opds import opds_cached
 from codex.views.opds.opensearch.v1 import OpenSearch1View
 from codex.views.opds.v1.feed import OPDS1FeedView, OPDS1StartView
 
@@ -15,18 +14,18 @@
     # Browser
     path(
         "<group:group>/<int_list:pks>/<int:page>",
-        cache_page(OPDS_TIMEOUT)(OPDS1FeedView.as_view()),
+        opds_cached(OPDS1FeedView.as_view()),
         name="feed",
     ),
     path(
         "opensearch/v1.1",
-        cache_page(OPDS_TIMEOUT)(OpenSearch1View.as_view()),
+        opds_cached(OpenSearch1View.as_view()),
         name="opensearch_v1",
     ),
     # Start
     path(
         "",
-        cache_page(OPDS_TIMEOUT)(OPDS1StartView.as_view()),
+        opds_cached(OPDS1StartView.as_view()),
         {"group": "r", "pks": (0,), "page": 1},
         name="start",
     ),

codex/urls/opds/v2.py

@@ -1,38 +1,43 @@
-"""codex:opds:v1 URL Configuration."""
+"""codex:opds:v2 URL Configuration."""
 
 from django.urls import path
-from django.views.decorators.cache import cache_page
 from django.views.generic import RedirectView
 
-from codex.urls.const import OPDS_TIMEOUT
+from codex.urls.opds import opds_cached
 from codex.views.opds.v2.feed import OPDS2FeedView, OPDS2StartView
 from codex.views.opds.v2.manifest import OPDS2ManifestView
 from codex.views.opds.v2.progression import OPDS2ProgressionView
 
 app_name = "v2"
 
+
 urlpatterns = [
     #
     # Browser
     path(
         "c/<int_list:pks>/1",
-        cache_page(OPDS_TIMEOUT)(OPDS2ManifestView.as_view()),
+        opds_cached(OPDS2ManifestView.as_view()),
         {"group": "c", "page": 1},
         name="manifest",
     ),
+    # Progression GET / PUT is correctness-sensitive — a PUT mutates
+    # the bookmark, and a GET within the cache window would otherwise
+    # return the pre-PUT position (multi-device sync would also see
+    # stale data). cache_page only caches GETs, but the freshness
+    # cost outweighs the ~9-query saving.
     path(
         "<group:group>/<int:pk>/position",
-        cache_page(OPDS_TIMEOUT)(OPDS2ProgressionView.as_view()),
+        OPDS2ProgressionView.as_view(),
         name="position",
     ),
     path(
         "<group:group>/<int_list:pks>/<int:page>",
-        cache_page(OPDS_TIMEOUT)(OPDS2FeedView.as_view()),
+        opds_cached(OPDS2FeedView.as_view()),
         name="feed",
     ),
     path(
         "",
-        cache_page(OPDS_TIMEOUT)(OPDS2StartView.as_view()),
+        opds_cached(OPDS2StartView.as_view()),
         {"group": "r", "pks": (0,), "page": 1},
         name="start",
     ),

tasks/opds-views-perf/99-summary.md

@@ -85,7 +85,7 @@ land.
 
 | #   | Change | Sub-plan | Impact | Effort | Risk | Status |
 | --- | ------ | -------- | ------ | ------ | ---- | ------ |
-| 1   | **Re-enable OPDS route caching.** Set `OPDS_TIMEOUT > 0` (suggest 60 s). Confirm `Vary: Cookie, Authorization` is set on feed routes (binary routes already have it — `codex/urls/opds/binary.py:36-37`). Investigation phase: source the original disable rationale from git history first. | 01 #1 | **Very high** (every feed request currently re-runs the full pipeline) | M | M-H | ⏳ Open |
+| 1   | **Re-enable OPDS route caching.** Set `OPDS_TIMEOUT > 0` (suggest 60 s). Confirm `Vary: Cookie, Authorization` is set on feed routes (binary routes already have it — `codex/urls/opds/binary.py:36-37`). Investigation phase: source the original disable rationale from git history first. | 01 #1 | **Very high** (every feed request currently re-runs the full pipeline) | M | M-H | ✅ Stage 2 |
 | 2   | **Batch `_publication_credits` into a single query.** Replace the 11-query loop in `v2/manifest.py:194-199` with one `Credit.objects.filter(comic__in=obj.ids)` + Python-side partition by role name. | 05 #1 | **High** (saves 10 queries per manifest hit) | S-M | L | ✅ Stage 1 |
 | 3   | **Fix N+1 in `_publication_belongs_to_story_arcs`.** Change `.only("story_arc", "number")` → `.select_related("story_arc")` (or `.values("story_arc__pk", "story_arc__name", "number")`) in `v2/manifest.py:122-144`. | 05 #2 | **Medium-High** (textbook N+1 on every manifest hit; cost scales with story-arc count) | XS | L | ✅ Stage 0 |
 | 4   | **Skip preview-pipeline re-runs on start page.** `v2/feed/publications.py:240-269` re-instantiates a feed view + runs the full ACL/filter/annotation pipeline per `PREVIEW_GROUPS` link spec. Either batch into a single union query or memoize ACL+annotation results at the view level. | 02 #2, 04 #3 | **Medium-High** (saves ~4 full pipeline runs per start-page hit) | M-L | M | ⏳ Open |

tasks/opds-views-perf/stage2-after.json

@@ -0,0 +1,182 @@
+{
+  "series_pk_used": 325,
+  "comic_pk_used": 10785,
+  "flows": [
+    {
+      "name": "v1_start",
+      "description": "v1 OPDS root (Atom navigation feed at /opds/v1.2/).",
+      "kind": "url",
+      "url": "/opds/v1.2/",
+      "cold": {
+        "status_code": 200,
+        "num_sql_queries": 14,
+        "time_taken_ms": 53.375
+      },
+      "warm": {
+        "status_code": 200,
+        "num_sql_queries": 0,
+        "time_taken_ms": 1.727
+      }
+    },
+    {
+      "name": "v1_root_browse",
+      "description": "v1 root browse (Atom feed at the canonical group URL).",
+      "kind": "url",
+      "url": "/opds/v1.2/r/0/1",
+      "cold": {
+        "status_code": 200,
+        "num_sql_queries": 15,
+        "time_taken_ms": 128.87599999999998
+      },
+      "warm": {
+        "status_code": 200,
+        "num_sql_queries": 0,
+        "time_taken_ms": 1.74
+      }
+    },
+    {
+      "name": "v1_series_acquisition",
+      "description": "v1 acquisition feed for the busiest series \u2014 exercises the per-entry stream / download / cover link generation in v1/entry/links.py. ``topGroup=s`` is required because the between-flow settings reset defaults topGroup to ``p`` (Publisher) and ``s/325/1`` would 302 to root otherwise.",
+      "kind": "url",
+      "url": "/opds/v1.2/s/325/1?topGroup=s",
+      "cold": {
+        "status_code": 200,
+        "num_sql_queries": 18,
+        "time_taken_ms": 50.524
+      },
+      "warm": {
+        "status_code": 200,
+        "num_sql_queries": 0,
+        "time_taken_ms": 2.7
+      }
+    },
+    {
+      "name": "v1_acquisition_with_metadata",
+      "description": "v1 acquisition feed with ?opdsMetadata=1 \u2014 fires the 9-query M2M fan-out (authors / contributors / category_groups) per entry. Headline number for sub-plan 03 #1.",
+      "kind": "url",
+      "url": "/opds/v1.2/s/325/1?topGroup=s&opdsMetadata=1",
+      "cold": {
+        "status_code": 200,
+        "num_sql_queries": 16,
+        "time_taken_ms": 69.201
+      },
+      "warm": {
+        "status_code": 200,
+        "num_sql_queries": 0,
+        "time_taken_ms": 2.004
+      }
+    },
+    {
+      "name": "v1_opensearch",
+      "description": "Static opensearch description doc (sub-plan 06 #5).",
+      "kind": "url",
+      "url": "/opds/v1.2/opensearch/v1.1",
+      "cold": {
+        "status_code": 200,
+        "num_sql_queries": 3,
+        "time_taken_ms": 7.494
+      },
+      "warm": {
+        "status_code": 200,
+        "num_sql_queries": 0,
+        "time_taken_ms": 1.939
+      }
+    },
+    {
+      "name": "v2_start",
+      "description": "v2 OPDS start page at /opds/v2.0 \u2014 fires the preview-pipeline rerun for every PREVIEW_GROUPS link spec (sub-plan 02 #2).",
+      "kind": "url",
+      "url": "/opds/v2.0",
+      "cold": {
+        "status_code": 200,
+        "num_sql_queries": 53,
+        "time_taken_ms": 513.1790000000001
+      },
+      "warm": {
+        "status_code": 200,
+        "num_sql_queries": 0,
+        "time_taken_ms": 1.77
+      }
+    },
+    {
+      "name": "v2_root_browse",
+      "description": "v2 JSON feed at the root group URL.",
+      "kind": "url",
+      "url": "/opds/v2.0/r/0/1",
+      "cold": {
+        "status_code": 200,
+        "num_sql_queries": 15,
+        "time_taken_ms": 117.556
+      },
+      "warm": {
+        "status_code": 200,
+        "num_sql_queries": 0,
+        "time_taken_ms": 2.412
+      }
+    },
+    {
+      "name": "v2_series_publications",
+      "description": "v2 publications feed for the busiest series \u2014 per-publication _thumb / link assembly hot path. ``topGroup=s`` for the same reason as v1_series_acquisition.",
+      "kind": "url",
+      "url": "/opds/v2.0/s/325/1?topGroup=s",
+      "cold": {
+        "status_code": 200,
+        "num_sql_queries": 18,
+        "time_taken_ms": 49.785999999999994
+      },
+      "warm": {
+        "status_code": 200,
+        "num_sql_queries": 0,
+        "time_taken_ms": 1.686
+      }
+    },
+    {
+      "name": "v2_manifest",
+      "description": "v2 single-comic manifest. Headline number for sub-plan 05: 11-query credit fan-out + 7-query M2M subject loop + story_arcs N+1 + per-page reading_order reverse() calls.",
+      "kind": "url",
+      "url": "/opds/v2.0/c/10785/1",
+      "cold": {
+        "status_code": 200,
+        "num_sql_queries": 24,
+        "time_taken_ms": 90.137
+      },
+      "warm": {
+        "status_code": 200,
+        "num_sql_queries": 0,
+        "time_taken_ms": 2.3640000000000003
+      }
+    },
+    {
+      "name": "v2_progression_get",
+      "description": "Progression GET for the busiest comic. Returns 204 when the test user has no bookmark \u2014 still exercises the ACL filter + FilteredRelation annotation pipeline (sub-plan 06 #2).",
+      "kind": "url",
+      "url": "/opds/v2.0/c/10785/position",
+      "cold": {
+        "status_code": 204,
+        "num_sql_queries": 9,
+        "time_taken_ms": 14.328999999999999
+      },
+      "warm": {
+        "status_code": 204,
+        "num_sql_queries": 8,
+        "time_taken_ms": 10.055
+      }
+    },
+    {
+      "name": "auth_doc_v1",
+      "description": "Static OPDS authentication document (sub-plan 06 #5).",
+      "kind": "url",
+      "url": "/opds/auth/v1",
+      "cold": {
+        "status_code": 200,
+        "num_sql_queries": 2,
+        "time_taken_ms": 6.2090000000000005
+      },
+      "warm": {
+        "status_code": 200,
+        "num_sql_queries": 0,
+        "time_taken_ms": 1.759
+      }
+    }
+  ]
+}