diff --git a/backend/app/downloads/gc.py b/backend/app/downloads/gc.py index 9cfd052f..3759dd10 100644 --- a/backend/app/downloads/gc.py +++ b/backend/app/downloads/gc.py @@ -282,33 +282,32 @@ def run_download_gc(db: Session) -> dict: kept_assets = select(DownloadJob.asset_id).where( DownloadJob.kept.is_(True), DownloadJob.asset_id.is_not(None) ) - candidate_ids = ( - db.execute( - select(MediaAsset.id) - .where( - MediaAsset.status == "ready", - MediaAsset.source_kind != "upload", - MediaAsset.id.not_in(kept_assets), - ) - .order_by(MediaAsset.last_access_at.asc().nulls_first()) + candidates = db.execute( + select(MediaAsset.id, MediaAsset.last_access_at) + .where( + MediaAsset.status == "ready", + MediaAsset.source_kind != "upload", + MediaAsset.id.not_in(kept_assets), ) - .scalars() - .all() - ) + .order_by(MediaAsset.last_access_at.asc().nulls_first()) + ).all() db.rollback() - for asset_id in candidate_ids: + for asset_id, seen_access in candidates: # Counted afresh before each: a file deleted since (this one, an upload, a candidate # further down) has freed its space, and no other user's file goes for that space. if _on_disk(db) <= cap: break # Locked and re-read like an expiry: since the SELECT a holder may have kept it, - # every holder deleted it, or someone asked for it again (a fresh cache hit). + # every holder deleted it, or someone asked for it again (a fresh cache hit). A hit + # is a CHANGED stamp, not one later than this pass's clock: a hit stamped just + # before the pass began but committed after the SELECT is still a hit, and a clock + # that steps back would make a fresh one look old. asset, holders = _relock_evictable(db, asset_id) if ( asset is None or asset.status != "ready" or any(j.kept for j in holders) - or (asset.last_access_at is not None and asset.last_access_at > now) + or asset.last_access_at != seen_access ): db.commit() continue diff --git a/backend/tests/test_file_folders.py b/backend/tests/test_file_folders.py index b13b6f79..46813297 100644 --- a/backend/tests/test_file_folders.py +++ b/backend/tests/test_file_folders.py @@ -739,6 +739,28 @@ def test_a_file_asked_for_again_while_the_cache_is_over_its_cap_stays( assert (_root / "old.mp4").exists() +@pytest.mark.parametrize("stamped", ["before_the_pass", "by_a_clock_that_stepped_back"]) +def test_a_cache_hit_stamped_earlier_than_the_pass_still_keeps_the_file( + db, make_user, _root, monkeypatch, stamped +): + # A hit is a stamp that changed since the pass's SELECT. Compared with the pass's own clock, a + # hit stamped before the pass began (committed after its SELECT), or by a clock that had stepped + # back, read as old and the file the user just asked for went. + jold, jnew = _two_over_the_cap(db, make_user, _root) + seen = NOW - timedelta(days=9) + stamp = seen + (timedelta(hours=1) if stamped == "before_the_pass" else timedelta(days=9)) + + def asked_again(other, asset_id): + if asset_id == jold.asset_id: + other.get(MediaAsset, asset_id).last_access_at = stamp - timedelta(seconds=1) + + _meanwhile(monkeypatch, asked_again, step="_relock_evictable") + _gc(db) + db.expire_all() + assert db.get(DownloadJob, jold.id) is not None + assert (_root / "old.mp4").exists() + + def test_a_file_asked_for_while_its_cached_copy_is_freed_gets_a_fresh_one(db, make_user): # The enqueue read the cached copy unlocked: freed meanwhile, its job pointed at a row that was gone # (a foreign-key failure, a 500).