perf(feed): un-block the search GIN + inner-join the state views (E2 S3, PERF-12/16)
PERF-12: the search ORed `unaccent(channels.title) ILIKE '%q%'` straight into the per-video predicate, which forced a full scan + detoast of every search_vector and a per-candidate channel fetch. Pre-resolve the matching channel ids (a few-hundred-row scan) and match `Video.channel_id.in_(ids)` instead, so the planner keeps the search_vector GIN as a BitmapOr branch (alongside ix_videos_channel_pub_desc). Empty match -> false() collapses the OR. PERF-16: show=watched|hidden|in_progress used an outer join + coalesce(status, 'new'), so the planner couldn't start from the user's small video_states set. Those views only return rows that HAVE a state, so inner-join video_states and filter the raw column (status is NOT NULL on a present row, so it's identical). 'unwatched'/'all' keep the outer join. Dropped the now-dead position_expr. New regression tests (test_feed_views): search returns both text- and channel- name matches; the four watch-state views return the exact expected sets. Measured on the prod-sized localdev reseed (prod planner, 0067/0068 applied): - search /feed/count q=magyar: 663,463 -> 19,206 buffers (1,406 -> 31 ms; OLD seq-scan+detoast -> NEW BitmapOr GIN+channel index) - search feed page: 2,908 -> 1,479 buffers (32.6 -> 1.4 ms) - show=watched (user with 64 watched): 40,661 -> 264 buffers (~154x) Gate green (336 pytest, incl. the 2 new equivalence tests).
This commit is contained in:
+30
-19
@@ -131,7 +131,6 @@ def filtered_query(
|
||||
either mode via the VideoState outer join."""
|
||||
state = aliased(VideoState)
|
||||
status_expr = func.coalesce(state.status, "new")
|
||||
position_expr = func.coalesce(state.position_seconds, 0)
|
||||
|
||||
query = select(*feed_columns(user, state)).join(
|
||||
Channel, Channel.id == Video.channel_id
|
||||
@@ -163,7 +162,14 @@ def filtered_query(
|
||||
and_(SearchFind.video_id == Video.id, SearchFind.user_id == user.id),
|
||||
)
|
||||
|
||||
query = query.outerjoin(
|
||||
# The state-gated views (watched/hidden/in_progress) only ever return videos that HAVE a
|
||||
# video_states row, so an INNER join lets the planner drive from the user's own small video_states
|
||||
# set (~100 rows) instead of scanning all videos and outer-joining (PERF-16). status is NOT NULL on
|
||||
# a present row (server_default 'new'), so the raw-column filters below match the coalesce form
|
||||
# exactly. 'unwatched'/'all' include row-less 'new' videos, so they keep the outer join.
|
||||
state_required = show in ("watched", "hidden", "in_progress")
|
||||
join_state = query.join if state_required else query.outerjoin
|
||||
query = join_state(
|
||||
state, and_(state.video_id == Video.id, state.user_id == user.id)
|
||||
)
|
||||
|
||||
@@ -246,15 +252,25 @@ def filtered_query(
|
||||
# channel searches work. `rank_expr` (weight-aware ts_rank) drives the "relevance" sort.
|
||||
rank_expr = None
|
||||
if q:
|
||||
# Resolve the channels whose NAME matches up front (there are only a few hundred channels, so
|
||||
# this is a tiny standalone scan) and match videos by channel_id. ORing an unindexed
|
||||
# `unaccent(channels.title) ILIKE '%q%'` straight into the per-video predicate defeated the
|
||||
# search_vector GIN — the planner detoasted every non-matching tsvector to test the `@@`
|
||||
# (~2.6s/search on the prod-sized set); channel_id.in_(...) keeps the GIN as a BitmapOr branch
|
||||
# (PERF-12). Empty match -> false() so the OR collapses to the text match alone.
|
||||
like = func.unaccent(f"%{q}%")
|
||||
matching_channel_ids = (
|
||||
db.execute(select(Channel.id).where(func.unaccent(Channel.title).ilike(like)))
|
||||
.scalars()
|
||||
.all()
|
||||
)
|
||||
channel_match = (
|
||||
Video.channel_id.in_(matching_channel_ids) if matching_channel_ids else false()
|
||||
)
|
||||
ts_str = to_tsquery_str(q)
|
||||
if ts_str:
|
||||
tsq = func.to_tsquery("public.unaccent_simple", ts_str)
|
||||
query = query.where(
|
||||
or_(
|
||||
Video.search_vector.op("@@")(tsq),
|
||||
func.unaccent(Channel.title).ilike(func.unaccent(f"%{q}%")),
|
||||
)
|
||||
)
|
||||
query = query.where(or_(Video.search_vector.op("@@")(tsq), channel_match))
|
||||
# Scale ts_rank (a float4) to an integer score so the keyset cursor compares it
|
||||
# EXACTLY — a raw float4 vs the float8 cursor value mismatches on round-trip and
|
||||
# breaks paging (the same page repeats). 1e6 granularity is ample; ties break on
|
||||
@@ -262,13 +278,7 @@ def filtered_query(
|
||||
rank_expr = cast(func.ts_rank(Video.search_vector, tsq) * 1000000, BigInteger)
|
||||
else:
|
||||
# No usable tokens (e.g. only punctuation): fall back to a plain substring match.
|
||||
like = func.unaccent(f"%{q}%")
|
||||
query = query.where(
|
||||
or_(
|
||||
func.unaccent(Video.title).ilike(like),
|
||||
func.unaccent(Channel.title).ilike(like),
|
||||
)
|
||||
)
|
||||
query = query.where(or_(func.unaccent(Video.title).ilike(like), channel_match))
|
||||
|
||||
if tags:
|
||||
# AND across tag categories (e.g. language AND topic narrows), OR within a
|
||||
@@ -315,14 +325,15 @@ def filtered_query(
|
||||
if show == "unwatched":
|
||||
query = query.where(status_expr.notin_(("watched", "hidden")))
|
||||
elif show == "in_progress":
|
||||
# Started but not finished: a stored resume position, not yet watched/hidden.
|
||||
# Started but not finished: a stored resume position, not yet watched/hidden. INNER-joined
|
||||
# (state_required) so the raw state columns are safe and the planner can use video_states.
|
||||
query = query.where(
|
||||
and_(position_expr > 0, status_expr.notin_(("watched", "hidden")))
|
||||
and_(state.position_seconds > 0, state.status.notin_(("watched", "hidden")))
|
||||
)
|
||||
elif show == "watched":
|
||||
query = query.where(status_expr == "watched")
|
||||
query = query.where(state.status == "watched")
|
||||
elif show == "hidden":
|
||||
query = query.where(status_expr == "hidden")
|
||||
query = query.where(state.status == "hidden")
|
||||
else: # all
|
||||
query = query.where(status_expr != "hidden")
|
||||
|
||||
|
||||
@@ -0,0 +1,84 @@
|
||||
"""Feed search + watch-state views (E2 S3), DB-lane.
|
||||
|
||||
Regression nets for two query rewrites that must stay behaviour-equivalent:
|
||||
- PERF-12: the search channel-name branch moved from an ORed `unaccent(channels.title) ILIKE` (which
|
||||
defeated the search_vector GIN) to a pre-resolved `channel_id IN (...)`. A search must STILL return
|
||||
both videos whose text matches AND videos whose CHANNEL name matches.
|
||||
- PERF-16: the watched/hidden/in_progress views moved from an outer join + `coalesce(state.status,'new')`
|
||||
to an inner join on video_states + the raw column. Each view must return exactly the same set.
|
||||
"""
|
||||
from datetime import datetime, timezone
|
||||
|
||||
from app.models import Channel, Subscription, Video, VideoState
|
||||
|
||||
|
||||
def _dt(day: int) -> datetime:
|
||||
return datetime(2024, 1, day, tzinfo=timezone.utc)
|
||||
|
||||
|
||||
def _video(vid: str, ch: str, title: str) -> Video:
|
||||
return Video(
|
||||
id=vid, channel_id=ch, published_at=_dt(1), title=title,
|
||||
is_short=False, live_status="none", via_search=False, via_explore=False,
|
||||
)
|
||||
|
||||
|
||||
def _feed_ids(client, query: str) -> set[str]:
|
||||
resp = client.get(f"/api/feed?{query}&limit=50")
|
||||
assert resp.status_code == 200, resp.text
|
||||
return {it["id"] for it in resp.json()["items"]}
|
||||
|
||||
|
||||
def test_search_matches_both_video_text_and_channel_name(db, as_user, make_user):
|
||||
# Two subscribed channels: one whose NAME matches the query (its video's text does not), and one
|
||||
# whose video TEXT matches (its name does not). The pre-resolved channel_id branch must still OR in
|
||||
# the name match, so both videos come back — and an unrelated video on a matching-name-less channel
|
||||
# stays out.
|
||||
user = make_user(email="search@example.com")
|
||||
db.add_all([
|
||||
Channel(id="UCkube", title="Kubernetes Weekly", subscriber_count=10),
|
||||
Channel(id="UCother", title="Some Other Channel", subscriber_count=10),
|
||||
])
|
||||
db.add_all([
|
||||
Subscription(user_id=user.id, channel_id="UCkube", priority=0, hidden=False),
|
||||
Subscription(user_id=user.id, channel_id="UCother", priority=0, hidden=False),
|
||||
])
|
||||
db.add_all([
|
||||
_video("vidChan", "UCkube", "Totally unrelated content"), # channel NAME matches
|
||||
_video("vidText", "UCother", "kubernetes deep dive"), # video TEXT matches
|
||||
_video("vidNone", "UCother", "cooking pasta at home"), # neither matches
|
||||
])
|
||||
db.commit()
|
||||
|
||||
ids = _feed_ids(as_user(user), "q=kubernetes&show=all")
|
||||
assert ids == {"vidChan", "vidText"}
|
||||
|
||||
|
||||
def test_watch_state_views_return_the_right_sets(db, as_user, make_user):
|
||||
user = make_user(email="views@example.com")
|
||||
db.add(Channel(id="UCv", title="Channel V", subscriber_count=10))
|
||||
db.add(Subscription(user_id=user.id, channel_id="UCv", priority=0, hidden=False))
|
||||
db.add_all([
|
||||
_video("vWatch", "UCv", "watched one"),
|
||||
_video("vHide", "UCv", "hidden one"),
|
||||
_video("vProg", "UCv", "in progress one"),
|
||||
_video("vNew", "UCv", "brand new one"),
|
||||
])
|
||||
db.flush()
|
||||
db.add_all([
|
||||
VideoState(user_id=user.id, video_id="vWatch", status="watched"),
|
||||
VideoState(user_id=user.id, video_id="vHide", status="hidden"),
|
||||
# in progress = a stored resume position while still "new" (not watched/hidden)
|
||||
VideoState(user_id=user.id, video_id="vProg", status="new", position_seconds=30),
|
||||
# vNew has NO state row -> "new"/unwatched
|
||||
])
|
||||
db.commit()
|
||||
|
||||
client = as_user(user)
|
||||
assert _feed_ids(client, "show=watched") == {"vWatch"}
|
||||
assert _feed_ids(client, "show=hidden") == {"vHide"}
|
||||
assert _feed_ids(client, "show=in_progress") == {"vProg"}
|
||||
# unwatched excludes watched + hidden, keeps new AND in-progress
|
||||
assert _feed_ids(client, "show=unwatched") == {"vProg", "vNew"}
|
||||
# "all" shows everything except hidden
|
||||
assert _feed_ids(client, "show=all") == {"vWatch", "vProg", "vNew"}
|
||||
Reference in New Issue
Block a user