peter/siftlode
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(reader): the Takeout import keeps a later upload, an un-mark and its memory bound

Browse Source 
- An ad-hoc request made while its target's run is under way is the run after it: the scheduler
  dropped a second fire of a running id, so a second Takeout file was never imported and its key
  stayed queued until a restart.
- Un-marking or resetting a video is remembered (video_unmarks): an import of an older YouTube view
  no longer marks it watched again; un-marks take the user's Takeout lock.
- One user's failure in the Shorts job's Takeout pass is theirs: the job keeps its own result, the
  others are settled, and a failed lookup spends none of the daily share.
- The upload reads the user's id before giving the read connection back.
- A watched row waits on a discovery video (search or explore) until the catalog keeps it, so a
  mark never pins it against the cleanup jobs.
- The history is decoded a window at a time, a Watch later file over 2 MiB is refused unread, and
  two uploads at once across all users (takeout_server_busy).
- A Watch later list goes in as one presence read and one insert.
This commit is contained in:
peter
2026-10-23 22:07:11 +02:00
parent 9f580fd8c8
commit 9c5f81fd94
17 changed files with 667 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/tests/test_takeout.py
+255 -3
View File
@@ -7,12 +7,15 @@ import tracemalloc
from datetime import datetime, timedelta, timezone
import pytest
from fastapi import Depends
from sqlalchemy import select, text
from sqlalchemy.exc import OperationalError
from sqlalchemy.orm import Session
from app import jobruns, quota, scheduler
from app.db import SessionLocal
from app.catalog.models import Channel, Video
from app.auth import current_user
from app.db import SessionLocal, get_db
from app.catalog.models import Channel, Subscription, Video
from app.models import JobRun, Notification, User
from app.runstatus import RunStatus, RunTrigger
from app.reader import queries, service, takeout
@@ -145,6 +148,42 @@ def test_the_history_is_read_an_entry_at_a_time_not_as_one_tree():
assert peak < 5 * len(body), f"parsing held {peak / len(body):.1f} times the file"
def test_a_history_with_an_emoji_costs_less_than_its_own_size_to_read(monkeypatch):
"""Decoded whole, one emoji makes the text four bytes a character: a 32 MiB history held some
128 MiB of it on top of its bytes. Read a window at a time, it holds a window (a few times its
bytes as text) whatever the file's size — a small one here, so a 9 MB file can show it."""
monkeypatch.setattr(takeout, "_WINDOW", 64 * 1024)
view = _view("aaaaaaaaaaa", title="Watched " + "😀" * 40)
body = b"[" + b",".join([json.dumps(view, ensure_ascii=False).encode()] * 20_000) + b"]"
tracemalloc.start()
try:
parsed = takeout.parse(body)
_, peak = tracemalloc.get_traced_memory()
finally:
tracemalloc.stop()
assert parsed.entries == 20_000
assert peak < len(body) / 2, f"parsing held {peak / len(body):.1f} times the file"
@pytest.mark.parametrize("split", [1, 7, 100, 4096])
def test_the_history_reads_the_same_whatever_its_window(monkeypatch, split):
"""An entry, a character or a number cut by the window's edge is read whole."""
monkeypatch.setattr(takeout, "_WINDOW", split)
views = [_view(f"{c * 11}", title="ő😀" * 3) for c in "abcdef"]
body = " [\n" + ",\n".join(json.dumps(v, ensure_ascii=False) for v in views) + "\n]\n"
parsed = takeout.parse(body.encode())
assert (parsed.entries, sorted(parsed.videos)) == (6, [c * 11 for c in "abcdef"])
numbers = takeout._json_list(b"[123456789, 2]", 0)
assert list(numbers) == [123456789, 2]
def test_a_watch_later_file_far_larger_than_any_list_is_refused_unread():
body = b"Video ID,Time\n" + b"x" * (takeout.CSV_MAX_BYTES)
with pytest.raises(takeout.TakeoutError) as caught:
takeout.parse(body)
assert caught.value.reason == "too_many"
def test_the_id_lists_are_asked_in_chunks(db, make_user, monkeypatch):
user = make_user("chunks@example.com")
ids = [f"chunk{n:06d}" for n in range(5)]
@@ -253,6 +292,33 @@ def test_a_known_video_is_marked_watched_at_its_takeout_time_and_the_rest_wait_o
assert (states["older000000"].status, states["older000000"].position_seconds) == ("watched", 40)
@pytest.mark.parametrize("undo", ["unmark", "clear_state"])
def test_a_video_the_user_un_marked_here_is_not_marked_again_by_an_older_view(db, make_user, undo):
"""Un-marking (or resetting) a video leaves no state row, and the import inserted a watched one
over the user's own choice. A view on YouTube after the un-mark is a new view, and counts."""
user = make_user(f"{undo}@example.com")
for vid in ("undone00000", "again000000"):
_video(db, vid)
db.commit()
for vid in ("undone00000", "again000000"):
service.mark(db, user, vid, "watched")
getattr(service, undo)(db, user, vid)
db.commit()
assert _states(db, user) == {}
later = datetime.now(timezone.utc) + timedelta(minutes=5)
service.stage_takeout(
db,
user,
TakeoutKind.WATCHED,
{"undone00000": (T0, None), "again000000": (later, None)},
)
db.commit()
assert takeout.run_import(user.id)["watched"] == 1
assert set(_states(db, user)) == {"again000000"}
assert _rows(db, user)["undone00000"].outcome == "kept"
def test_a_second_upload_keeps_the_later_time_and_what_the_import_settled(db, make_user):
user = make_user("again@example.com")
_video(db, "plain000000")
@@ -305,6 +371,40 @@ def test_the_watch_later_list_joins_the_users_own_oldest_first_after_what_it_hol
assert db.execute(select(Playlist).where(Playlist.user_id == user.id)).scalars().all() == [pl]
@pytest.mark.parametrize("n", [3, 40])
def test_a_watch_later_list_goes_in_in_statements_that_do_not_grow_with_it(db, make_user, n):
"""A video at a time, a full list of 5000 was some 20,000 statements under the user's lock."""
from sqlalchemy import event
user = make_user(f"many{n}@example.com")
ids = [f"wl{i:09d}" for i in range(n)]
for vid in ids:
_video(db, vid)
db.commit()
service.stage_takeout(
db,
user,
TakeoutKind.WATCH_LATER,
{v: (T0 + timedelta(minutes=i), None) for i, v in enumerate(ids)},
)
db.commit()
rows = queries.takeout_waiting(db, user, TakeoutKind.WATCH_LATER)
service.watch_later(db, user) # the list made beforehand: its own insert is not this count's
db.flush()
statements = []
engine = db.get_bind()
listener = lambda *a: statements.append(a[2]) # noqa: E731 — the event's own signature
event.listen(engine, "before_cursor_execute", listener)
try:
assert service.apply_takeout_watch_later(db, user, rows, set())["watch_later_added"] == n
finally:
event.remove(engine, "before_cursor_execute", listener)
db.commit()
assert len(statements) <= 9, statements
held = [i.video_id for i in queries.items(db, service.watch_later(db, user).id)]
assert held == ids
def test_the_job_fetches_the_unknown_watch_later_videos_and_says_what_waits(
db, make_user, monkeypatch
):
@@ -407,6 +507,38 @@ def test_the_shorts_job_puts_in_the_watch_later_videos_a_run_could_not(db, make_
assert takeout.apply_waiting(db) == {}
def test_one_users_failure_in_the_shorts_job_leaves_the_others_and_the_probe_alone(
db, make_user, monkeypatch
):
"""A Watch later lookup that raised (a revoked token, a refused quota) escaped the Shorts job:
its run was recorded as failed with its own counts lost, and every user after this one waited."""
first, second = make_user("first@example.com"), make_user("second@example.com")
_video(db, "ready000000")
db.commit()
service.stage_takeout(db, first, TakeoutKind.WATCH_LATER, {"unknown0000": (T0, None)})
service.stage_takeout(db, second, TakeoutKind.WATCHED, {"ready000000": (T0, None)})
db.commit()
def refused(session, yt, ids):
raise RuntimeError("invalid_grant")
monkeypatch.setattr(takeout, "ensure_videos", refused)
monkeypatch.setattr(takeout, "get_service_user", lambda session: first)
monkeypatch.setattr(scheduler, "run_shorts", lambda session: {"probed": 4})
scheduler._shorts_job()
db.expire_all()
run = db.execute(select(JobRun).where(JobRun.job == "shorts")).scalar_one()
assert run.status == "ok"
assert run.result == {"probed": 4, "takeout_watched": 1, "takeout_failed": 1}
assert _states(db, second)["ready000000"].status == "watched"
assert _rows(db, first)["WL:unknown0000"].outcome is None
# A lookup that failed spent none of the user's daily share.
assert not takeout._lookups.exhausted(str(first.id))
assert takeout._lookups.room(str(first.id)) == 2 * takeout.WATCH_LATER_MAX
def test_a_watch_later_row_staged_while_youtube_is_asked_is_left_for_the_next_run(
db, make_user, monkeypatch
):
@@ -469,6 +601,9 @@ def test_staging_and_settling_take_the_users_one_lock(db, make_user):
"watch later": lambda s, u: service.apply_takeout_watch_later(
s, u, [(1, "aaaaaaaaaaa")], set()
),
# Between the import's read of the un-marks and its marking, one would be overwritten.
"unmark": lambda s, u: service.unmark(s, u, "aaaaaaaaaaa"),
"reset": lambda s, u: service.clear_state(s, u, "aaaaaaaaaaa"),
}
with SessionLocal() as holder:
service.lock_takeout(holder, holder.get(User, user.id))
@@ -570,6 +705,39 @@ def test_a_signed_in_upload_has_its_body_read(db, make_user, as_user, queued):
assert len(pulled) == 4
def test_the_upload_arrives_with_no_connection_held(
db, make_user, as_user, override_dep, queued, monkeypatch
):
"""The read connection is given back before the body: reading the user's id after that reloaded
the user and took one straight back out, `idle in transaction` for the whole upload."""
user = make_user("idle@example.com")
client = as_user(user)
def signed_in(session: Session = Depends(get_db)) -> User:
# As the real sign-in check: a SELECT on the request's session, which the release ends —
# and expires what it loaded. (`as_user`'s own resolver reads nothing.)
session.expire_all()
return session.get(User, user.id)
override_dep(current_user, signed_in)
sessions, held = [], []
release, capped = feed_routes.release_read_connection, feed_routes.capped_body
def releasing(session):
sessions.append(session)
release(session)
async def reading(request, limit):
held.extend(s.in_transaction() for s in sessions)
return await capped(request, limit)
monkeypatch.setattr(feed_routes, "release_read_connection", releasing)
monkeypatch.setattr(feed_routes, "capped_body", reading)
resp = client.post("/api/takeout", content=_history(_view("aaaaaaaaaaa")))
assert resp.status_code == 200, resp.text
assert held == [False]
def test_uploads_past_the_hourly_limit_are_refused_after_their_body_arrived(
db, make_user, as_user, queued
):
@@ -600,6 +768,20 @@ def test_a_second_upload_while_one_of_the_users_is_still_arriving_is_refused(
assert feed_routes._takeout_uploading == {user.id}
def test_a_third_upload_across_users_while_two_arrive_is_refused_after_its_body(
db, make_user, as_user, queued, monkeypatch
):
"""Each upload's bytes are held whole while it arrives: with no bound across users, a few at
once could take the process past its memory."""
monkeypatch.setattr(feed_routes, "_takeout_uploading", {9001, 9002})
body, pulled = _counting_body()
resp = as_user(make_user("third@example.com")).post("/api/takeout", content=body)
assert resp.status_code == 429
assert resp.json()["detail"]["reason"] == "takeout_server_busy"
assert len(pulled) == 4
assert queued == [] and feed_routes._takeout_uploading == {9001, 9002}
def test_an_upload_that_would_keep_too_many_videos_is_refused(
db, make_user, as_user, queued, monkeypatch
):
@@ -679,12 +861,49 @@ def test_a_queued_import_is_told_apart_from_the_last_one_until_its_run_is_record
assert scheduler.ad_hoc_queued("takeout_import", target)
assert as_user(user).get("/api/takeout").json()["queued"] is True
scheduler._ad_hoc("takeout_import", dict, user.id, target)
scheduler._ad_hoc_fired("takeout_import", target)
assert not scheduler.ad_hoc_queued("takeout_import", target)
assert as_user(user).get("/api/takeout").json()["queued"] is False
assert added == [f"takeout_import@{target}"]
def test_an_upload_while_its_import_starts_stays_queued_for_the_run_after(
db, make_user, monkeypatch
):
"""A request landing while the run before it writes its row is the run after that one: still
queued, so the page does not take the run starting now for it."""
user = make_user("starting@example.com")
added, ran = [], []
class Held:
def add_job(self, *args, **kwargs):
added.append(kwargs["id"])
monkeypatch.setattr(scheduler, "_scheduler", Held())
target = takeout.run_target(user.id)
start = jobruns.start
def starting(*args, **kwargs):
scheduler.run_ad_hoc(
"takeout_import", lambda: ran.append(2) or {}, actor_id=user.id, target=target
)
return start(*args, **kwargs)
scheduler.run_ad_hoc(
"takeout_import", lambda: ran.append(1) or {}, actor_id=user.id, target=target
)
monkeypatch.setattr(scheduler.jobruns, "start", starting)
scheduler._ad_hoc_fired("takeout_import", target)
assert ran == [1]
assert scheduler.ad_hoc_queued("takeout_import", target)
assert added == [f"takeout_import@{target}"] * 2 # the run after it, asked for as it ended
monkeypatch.setattr(scheduler.jobruns, "start", start)
scheduler._ad_hoc_fired("takeout_import", target)
assert ran == [1, 2]
assert not scheduler.ad_hoc_queued("takeout_import", target)
# --- the videos that wait ----------------------------------------------------------------------
@@ -711,6 +930,39 @@ def test_waiting_videos_are_marked_once_the_shorts_probe_has_cleared_them(db, ma
assert takeout.apply_waiting(db) == {}
def test_a_discovery_video_is_not_marked_until_the_catalog_keeps_it(db, make_user):
"""A mark is a VideoState, which the cleanup jobs read as a reader keeping the video: on a video
only someone's search or explore brought in, it kept that video (and its channel) for good."""
user = make_user("discover@example.com")
for vid, flag in (("search00000", "via_search"), ("explore0000", "via_explore")):
_video(db, vid)
db.flush()
setattr(db.get(Video, vid), flag, True)
_video(db, "plain000000")
db.commit()
service.stage_takeout(
db,
user,
TakeoutKind.WATCHED,
{v: (T0, None) for v in ("search00000", "explore0000", "plain000000")},
)
db.commit()
assert takeout.apply_waiting(db) == {"takeout_watched": 1}
assert set(_states(db, user)) == {"plain000000"}
assert {v for v, r in _rows(db, user).items() if r.outcome is None} == {
"search00000",
"explore0000",
}
# Subscribed to: the catalog keeps the channel's videos of its own, the search one included.
db.add(Subscription(user_id=user.id, channel_id=CHANNEL))
db.get(Video, "explore0000").via_explore = False # as subscribing does (catalog.service)
db.commit()
assert takeout.apply_waiting(db) == {"takeout_watched": 2}
assert set(_states(db, user)) == {"plain000000", "search00000", "explore0000"}
def test_the_shorts_job_settles_the_waiting_rows_its_probe_made_ready(db, make_user):
user = make_user("job-shorts@example.com")
_video(db, "ready000000") # probed already: the job's probe has nothing left to do