perf(db): the channel manager and the feed sidebar read an index, not the video heap
GET /api/channels summed every subscribed channel's videos by reading each video's heap page (681 MB on prod): 51-61 ms warm, but 2.25 s on a first open and 6.4 s under EXPLAIN once the table had left memory. /feed/summary and /feed/years did the same (~140 ms warm, ~800 ms cold). Migration 0095 replaces ix_videos_channel_pub_desc with the same key INCLUDING the columns those queries read, so they are Index Only Scans over a 25 MB index. A test holds the queries and the index's column list together. The admin Configuration page did a db.get per setting (two for an unset one): it now reads every override in one query (~40 ms of ~55 on prod).
This commit is contained in:
@@ -0,0 +1,47 @@
|
||||
"""videos: the per-channel index covers what the channel manager and the feed sidebar read (E31 S4d)
|
||||
|
||||
The channel manager (`GET /api/channels`) sums each subscribed channel's videos: count, last upload, total
|
||||
duration. The feed sidebar (`/feed/summary`, `/feed/years`) counts the reader's videos per channel and per
|
||||
year under the default filters: available, not via explore, not a short, not live. Every one of them found
|
||||
its rows through `ix_videos_channel_pub_desc` (channel_id, published_at DESC NULLS LAST, id) and then read
|
||||
each video's heap page for the rest: ~270k rows at ~2.5 KB each, 681 MB on prod. That stays in cache only
|
||||
while nothing else needs the memory. Measured on prod (2026-10-02): `/api/channels` 51–61 ms warm, but
|
||||
2.25 s and 1.96 s on a first open in the browser and 6.4 s under EXPLAIN with 30k blocks read from disk;
|
||||
`/feed/summary` and `/feed/years` ~140 ms warm, ~800 ms cold.
|
||||
|
||||
The same key with those columns INCLUDEd makes them Index Only Scans. On a prod-copy localdev, after a
|
||||
VACUUM, the manager's sums read 0 heap pages and the summary reads the 25 MB index instead of the heap.
|
||||
The visibility map is what lets them skip the heap; prod's autovacuum kept it complete (relallvisible ==
|
||||
relpages). The key columns are unchanged, so every reader of the old index (the per-channel feed, the FK
|
||||
cascade, the sync upsert, channel detail) rides this one. The old index is dropped.
|
||||
tests/test_channel_list_index_only.py holds the queries and this column list together.
|
||||
|
||||
CONCURRENTLY (autocommit_block) builds it without blocking a live prod. IF (NOT) EXISTS keeps the drift
|
||||
test's downgrade → re-upgrade round trip idempotent.
|
||||
"""
|
||||
|
||||
from alembic import op
|
||||
|
||||
revision: str = "0095_videos_channel_cover_index"
|
||||
down_revision: str | None = "0094_plex_genres_hu_everywhere"
|
||||
branch_labels: str | None = None
|
||||
depends_on: str | None = None
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
with op.get_context().autocommit_block():
|
||||
op.execute(
|
||||
"CREATE INDEX CONCURRENTLY IF NOT EXISTS ix_videos_channel_pub_cover "
|
||||
"ON videos (channel_id, published_at DESC NULLS LAST, id) "
|
||||
"INCLUDE (duration_seconds, unavailable_since, via_explore, is_short, live_status)"
|
||||
)
|
||||
op.execute("DROP INDEX CONCURRENTLY IF EXISTS ix_videos_channel_pub_desc")
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
with op.get_context().autocommit_block():
|
||||
op.execute(
|
||||
"CREATE INDEX CONCURRENTLY IF NOT EXISTS ix_videos_channel_pub_desc "
|
||||
"ON videos (channel_id, published_at DESC NULLS LAST, id ASC)"
|
||||
)
|
||||
op.execute("DROP INDEX CONCURRENTLY IF EXISTS ix_videos_channel_pub_cover")
|
||||
@@ -330,10 +330,12 @@ class Video(Base, TimestampMixin):
|
||||
__tablename__ = "videos"
|
||||
|
||||
id: Mapped[str] = mapped_column(String(16), primary_key=True) # video id
|
||||
# No standalone channel_id index: the composite `ix_videos_channel_pub_desc`
|
||||
# (channel_id, published_at DESC NULLS LAST, id) from migration 0067 leads with channel_id, so it
|
||||
# serves every channel_id lookup (FK cascade, sync upsert, channel-detail) AND the per-channel feed
|
||||
# sort. Like the search_vector GIN, this composite is migration-defined, not an inline `index=True`.
|
||||
# No standalone channel_id index: the composite `ix_videos_channel_pub_cover`
|
||||
# (channel_id, published_at DESC NULLS LAST, id) from migration 0095 leads with channel_id, so it
|
||||
# serves every channel_id lookup (FK cascade, sync upsert, channel-detail) and the per-channel feed
|
||||
# sort. Its INCLUDEd columns make the channel manager's sums and the feed sidebar's counts Index Only
|
||||
# Scans (no heap): a column those queries start reading belongs in it (test_channel_list_index_only).
|
||||
# Like the search_vector GIN, this composite is migration-defined, not an inline `index=True`.
|
||||
channel_id: Mapped[str] = mapped_column(ForeignKey("channels.id", ondelete="CASCADE"))
|
||||
title: Mapped[str | None] = mapped_column(Text) # normalized for display (see app.titles)
|
||||
original_title: Mapped[str | None] = mapped_column(Text) # raw YouTube title, preserved
|
||||
|
||||
@@ -6,7 +6,7 @@ from datetime import datetime, timezone
|
||||
|
||||
from fastapi import APIRouter, BackgroundTasks, Depends
|
||||
from pydantic import BaseModel
|
||||
from sqlalchemy import func, select, update
|
||||
from sqlalchemy import Select, func, select, update
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from app.collation import collated, sorted_words
|
||||
@@ -40,6 +40,22 @@ router = APIRouter(prefix="/api/channels", tags=["channels"])
|
||||
log = logs.get_logger(__name__)
|
||||
|
||||
|
||||
def channel_aggregates(channel_ids: list[str]) -> Select:
|
||||
"""Per-channel stored count, last upload and total duration, in one grouped pass. Every column it
|
||||
reads is in `ix_videos_channel_pub_cover` (migration 0095), so Postgres answers it from the index
|
||||
alone; a test holds that (test_channel_list_index_only)."""
|
||||
return (
|
||||
select(
|
||||
Video.channel_id,
|
||||
func.count(Video.id),
|
||||
func.max(Video.published_at),
|
||||
func.coalesce(func.sum(Video.duration_seconds), 0),
|
||||
)
|
||||
.where(Video.channel_id.in_(channel_ids))
|
||||
.group_by(Video.channel_id)
|
||||
)
|
||||
|
||||
|
||||
@router.get("")
|
||||
def list_channels(user: User = Depends(current_user), db: Session = Depends(get_db)) -> list[dict]:
|
||||
rows = db.execute(
|
||||
@@ -49,24 +65,15 @@ def list_channels(user: User = Depends(current_user), db: Session = Depends(get_
|
||||
.order_by(Subscription.priority.desc(), collated(Channel.title, lang_for_user(user)))
|
||||
).all()
|
||||
|
||||
# Per-channel aggregates over the shared catalog in ONE grouped pass: stored count, last
|
||||
# upload, total duration. Cheap — a single grouped scan, so computed on read rather than
|
||||
# denormalised (measure-first: reads aren't frequent/hot enough to warrant cached columns).
|
||||
# Per-channel aggregates over the shared catalog (channel_aggregates), computed on read rather than
|
||||
# denormalised: an Index Only Scan over a ~20 MB index, where it used to read every video's heap page
|
||||
# (681 MB on prod, a 2–6 s first open once something else had pushed it out of memory, E31 S4d).
|
||||
# (The Normal/Shorts/Live breakdown moved to the channel-detail page in R14, so it is no longer
|
||||
# computed here — the manager table dropped its N/S/L column.)
|
||||
channel_ids = [c.id for _, c in rows]
|
||||
agg: dict[str, dict] = {}
|
||||
if channel_ids:
|
||||
for cid, n, last_up, total_dur in db.execute(
|
||||
select(
|
||||
Video.channel_id,
|
||||
func.count(Video.id),
|
||||
func.max(Video.published_at),
|
||||
func.coalesce(func.sum(Video.duration_seconds), 0),
|
||||
)
|
||||
.where(Video.channel_id.in_(channel_ids))
|
||||
.group_by(Video.channel_id)
|
||||
).all():
|
||||
for cid, n, last_up, total_dur in db.execute(channel_aggregates(channel_ids)).all():
|
||||
agg[cid] = {
|
||||
"stored": n,
|
||||
"last_video_at": iso(last_up),
|
||||
|
||||
@@ -10,6 +10,7 @@ stored in the DB and stay env-only).
|
||||
|
||||
from dataclasses import dataclass
|
||||
|
||||
from sqlalchemy import select
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from app import logs, security
|
||||
@@ -307,7 +308,11 @@ def _raw_override(db: Session, key: str) -> str | None:
|
||||
per key so the operator gets a real signal without a per-request traceback. Recovery is documented in
|
||||
docs/self-hosting.md: the key must never change once secrets are stored. (B-core-2)
|
||||
"""
|
||||
row = db.get(SystemConfig, key)
|
||||
return _raw_of(db.get(SystemConfig, key), key)
|
||||
|
||||
|
||||
def _raw_of(row: SystemConfig | None, key: str) -> str | None:
|
||||
"""`_raw_override` over a row already read (describe() reads them all in one query)."""
|
||||
if row is None or row.value is None:
|
||||
return None
|
||||
if not row.encrypted:
|
||||
@@ -327,8 +332,11 @@ def _raw_override(db: Session, key: str) -> str | None:
|
||||
|
||||
def get(db: Session, key: str):
|
||||
"""Effective value: the DB override (typed) if set, else the env/config default."""
|
||||
s = _BY_KEY[key]
|
||||
raw = _raw_override(db, key)
|
||||
return _effective(_BY_KEY[key], _raw_override(db, key))
|
||||
|
||||
|
||||
def _effective(s: ConfigSpec, raw: str | None):
|
||||
"""`get` over an override already read."""
|
||||
if raw is None:
|
||||
return _default(s)
|
||||
if raw == "" and not s.allow_empty:
|
||||
@@ -420,8 +428,12 @@ def describe(db: Session) -> dict:
|
||||
"""Admin-UI metadata + current values, grouped. Secret values are never echoed —
|
||||
only whether an override is set and whether an env default exists."""
|
||||
groups: dict[str, list[dict]] = {}
|
||||
# Every override in one query: a db.get per key was one round trip each (two for a key with no
|
||||
# row, which the session cannot cache), ~40 ms of the page's ~55 on prod (E31 S4d).
|
||||
rows = {r.key: r for r in db.scalars(select(SystemConfig))}
|
||||
for s in SPECS:
|
||||
is_set = db.get(SystemConfig, s.key) is not None
|
||||
row = rows.get(s.key)
|
||||
is_set = row is not None
|
||||
item: dict = {
|
||||
"key": s.key,
|
||||
"type": s.type,
|
||||
@@ -436,7 +448,7 @@ def describe(db: Session) -> dict:
|
||||
if s.secret:
|
||||
item["default_is_set"] = bool(_default(s))
|
||||
else:
|
||||
item["value"] = get(db, s.key)
|
||||
item["value"] = _effective(s, _raw_of(row, s.key))
|
||||
item["default"] = _default(s)
|
||||
groups.setdefault(s.group, []).append(item)
|
||||
sections = [
|
||||
|
||||
@@ -0,0 +1,83 @@
|
||||
"""The channel manager's sums and the feed sidebar's counts read the index alone (E31 S4d).
|
||||
|
||||
`GET /api/channels` counts each subscribed channel's videos and sums their durations; `/feed/summary`
|
||||
and `/feed/years` count the reader's videos per channel and per year under the default filters. They
|
||||
used to read every video's heap page for that (681 MB on prod), a 2–6 s first open once something else
|
||||
had pushed the table out of memory. Migration 0095 makes `ix_videos_channel_pub_cover` carry every
|
||||
column those queries read, so Postgres answers them with Index Only Scans. This test holds the queries
|
||||
and the index together: a column added to one and not the other fails it. DB lane.
|
||||
"""
|
||||
|
||||
import re
|
||||
|
||||
from sqlalchemy import text
|
||||
from sqlalchemy.dialects import postgresql
|
||||
|
||||
from app.feedquery import filtered_query
|
||||
from app.models import Video
|
||||
from app.routes.channels import channel_aggregates
|
||||
|
||||
INDEX = "ix_videos_channel_pub_cover"
|
||||
|
||||
# The feed's own defaults (routes/feed.py _feed_params): what the sidebar asks on a plain open.
|
||||
FEED_DEFAULTS = {
|
||||
"tags": [],
|
||||
"tag_modes": {"language": "or", "topic": "or", "other": "or"},
|
||||
"untagged": False,
|
||||
"channel_ids": [],
|
||||
"q": None,
|
||||
"min_duration": None,
|
||||
"max_duration": None,
|
||||
"max_age_days": None,
|
||||
"published_after": None,
|
||||
"published_before": None,
|
||||
"show_normal": True,
|
||||
"include_shorts": False,
|
||||
"include_live": False,
|
||||
"show": "unwatched",
|
||||
"scope": "my",
|
||||
"library_source": "organic",
|
||||
}
|
||||
|
||||
|
||||
def _index_columns(db) -> list[str]:
|
||||
# Key columns first, then the INCLUDE ones, in the index's own order.
|
||||
return list(
|
||||
db.execute(
|
||||
text(
|
||||
"SELECT a.attname FROM pg_index i "
|
||||
"JOIN pg_class c ON c.oid = i.indexrelid "
|
||||
"CROSS JOIN LATERAL unnest(i.indkey) WITH ORDINALITY AS k(attnum, n) "
|
||||
"JOIN pg_attribute a ON a.attrelid = i.indrelid AND a.attnum = k.attnum "
|
||||
"WHERE c.relname = :name ORDER BY k.n"
|
||||
),
|
||||
{"name": INDEX},
|
||||
).scalars()
|
||||
)
|
||||
|
||||
|
||||
def _video_columns(stmt) -> set[str]:
|
||||
# The columns of `videos` the statement as SENT reads: the compiled SQL, not the expression tree,
|
||||
# which still carries the projection a with_only_columns replaced.
|
||||
sql = str(stmt.compile(dialect=postgresql.dialect()))
|
||||
return set(re.findall(r"\bvideos\.(\w+)", sql))
|
||||
|
||||
|
||||
def _assert_covered(db, read: set[str]) -> None:
|
||||
held = _index_columns(db)
|
||||
assert held, f"{INDEX} is missing"
|
||||
# It leads with channel_id, so it also serves every per-channel lookup the old index did.
|
||||
assert held[0] == "channel_id"
|
||||
assert read <= set(held), f"read but not in {INDEX}: {sorted(read - set(held))}"
|
||||
|
||||
|
||||
def test_the_channel_sums_read_only_columns_the_index_holds(db):
|
||||
_assert_covered(db, _video_columns(channel_aggregates(["UC_x"])))
|
||||
|
||||
|
||||
def test_the_feed_sidebar_counts_read_only_columns_the_index_holds(db, make_user):
|
||||
# feed_summary groups the default filter by channel, feed_years by publish date.
|
||||
base, _rank = filtered_query(db, make_user(), **FEED_DEFAULTS)
|
||||
_assert_covered(
|
||||
db, _video_columns(base.with_only_columns(Video.channel_id, Video.published_at))
|
||||
)
|
||||
@@ -0,0 +1,39 @@
|
||||
"""The admin Configuration page reads every override in one query (E31 S4d).
|
||||
|
||||
`sysconfig.describe()` did a `db.get` per key to tell whether an override was set, and `get()` did one
|
||||
more for its value: ~55 keys, and two round trips for a key with no row, since the session cannot cache
|
||||
an absence. That was ~40 ms of the page's ~55 on prod. It now reads the table once. DB lane.
|
||||
"""
|
||||
|
||||
from sqlalchemy import event
|
||||
|
||||
from app import sysconfig
|
||||
|
||||
|
||||
def _statements(db, fn) -> int:
|
||||
seen: list[str] = []
|
||||
engine = db.get_bind()
|
||||
listener = lambda *a, **k: seen.append(a[2]) # noqa: E731 — (conn, cursor, statement, …)
|
||||
event.listen(engine, "before_cursor_execute", listener)
|
||||
try:
|
||||
fn()
|
||||
finally:
|
||||
event.remove(engine, "before_cursor_execute", listener)
|
||||
return len(seen)
|
||||
|
||||
|
||||
def test_describe_reads_the_overrides_once(db):
|
||||
sysconfig.set_value(db, "quota_daily_budget", 5000)
|
||||
db.commit()
|
||||
assert _statements(db, lambda: sysconfig.describe(db)) == 1
|
||||
|
||||
|
||||
def test_describe_shows_the_stored_value_and_the_default(db):
|
||||
sysconfig.set_value(db, "quota_daily_budget", 5000)
|
||||
db.commit()
|
||||
items = {i["key"]: i for g in sysconfig.describe(db)["groups"].values() for i in g}
|
||||
assert items["quota_daily_budget"]["is_set"] is True
|
||||
assert items["quota_daily_budget"]["value"] == 5000
|
||||
other = next(i for k, i in items.items() if k != "quota_daily_budget" and not i["secret"])
|
||||
assert other["is_set"] is False
|
||||
assert other["value"] == other["default"]
|
||||
@@ -67,19 +67,23 @@ def test_the_env_accepts_any_letter_case_and_rejects_an_unknown_value():
|
||||
|
||||
|
||||
class _Row:
|
||||
def __init__(self, value):
|
||||
self.value, self.encrypted = value, False
|
||||
def __init__(self, key, value):
|
||||
self.key, self.value, self.encrypted = key, value, False
|
||||
|
||||
|
||||
class _FakeDb:
|
||||
"""Just enough of a Session for sysconfig's reads: one stored override row per key."""
|
||||
|
||||
def __init__(self, **rows):
|
||||
self.rows = {k: _Row(v) for k, v in rows.items()}
|
||||
self.rows = {k: _Row(k, v) for k, v in rows.items()}
|
||||
|
||||
def get(self, _model, key):
|
||||
return self.rows.get(key)
|
||||
|
||||
def scalars(self, _stmt):
|
||||
# describe() reads every override at once (E31 S4d).
|
||||
return list(self.rows.values())
|
||||
|
||||
|
||||
def test_a_stored_value_reads_back_canonical_and_a_stale_one_falls_back_to_the_default():
|
||||
assert sysconfig.get_str(_FakeDb(plex_transcode_hw="OFF"), "plex_transcode_hw") == "off"
|
||||
|
||||
Reference in New Issue
Block a user