fix(stability): стадия 5 — батчинг записи access-time в SQLite #15
Reference in New Issue
Block a user
Delete Branch "fix/stage-5-sqlite"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Стадия 5 стабилизации: убираем запись access-timestamp из горячего пути
/media. closes #5.Раньше каждый cache-hit
/mediaтрогал SQLite (поток из пула + connect +UPDATEна запрос) в двух местах:await to_thread(update_media_file_access_sync)вdownload_media_fileи fire-and-forgetcreate_task(_update_access(...))в pre-semaphore fast-path. Под активным опросом RSS это исчерпывает пул потоков и тормозит раздачу медиа.Теперь cache-hit только пишет timestamp в module-level аккумулятор
_access_updates(запись в dict на event loop — дёшево и атомарно), а фоновый таск раз в 60 с сбрасывает весь батч однимexecutemany UPDATE.Что сделано:
file_io.update_media_file_access_bulk_sync— одно соединение,executemany; пустой батч = no-op.api_server._access_updates+_flush_access_updates: snapshot-then-clear атомарно (переприсваиваем global на новый dict доawait, поэтому cache-hit'ы во время флеша попадают в свежий dict и не теряются). При ошибке bulk-записи батч возвращается в аккумулятор черезsetdefault(более свежая параллельная запись не затирается), чтобы ни один access-time не потерялся._access_flush_loopпод_supervised(краш логируется CRITICAL и рестартится); плюс финальный flush на shutdown — после отмены таска цикла и до остановки io-пула.str(channel)(колонкаchannel TEXT); смешение str/int заставило быWHEREмолча не матчить → access-time перестал бы обновляться → файл выпал бы из 20-дневного кэша.DoD: cache-hit
/mediaделает ноль записей в SQLite в горячем пути (кроме отдельного MIME-кэша); фоновая запись раз в минуту.How verified
На стенде, чистый venv (
python -m venv .venv && pip install -r requirements.txt):python -m pytest tests/→ 240 passed (233 базовых стадий 1–4 + 7 новых стадии 5).test_failed_flush_requeues_batch_without_clobbering_freshпадает (доказывает, что он ловит потерю батча); вернул → зелёно._access_updates; синхронной SQLite-записи access-time в горячем пути не осталось.7 новых тестов: hot-path без синхронной SQLite (spy), дисциплина
str(channel), hit→flush→значение в БД + очистка dict, empty no-op, snapshot-then-clear (поздняя запись во время флеша выживает), re-queue-без-затирания-свежего, реальный bulk-SQL.База и стек
PR основан на
fix/stage-4-eventloop(#12), продолжает стек стадий. Ретаргет наfix/stabilityпо мере мержа цепочки.Checklist
Отдаю на ревью стадию 5 (#5) — вынос записи access-time из горячего пути
/mediaв аккумулятор + фоновый батч-flush.Проверил на стенде, чистый venv:
python -m pytest tests/→ 240 passed (233 базовых стадий 1–4 + 7 новых). Оба горячих сайта записи (download_media_fileи pre-semaphore fast-path) теперь пишут только в_access_updates; синхронной SQLite-записи access-time в горячем пути не осталось (DoD).Adversarial-валидация re-queue: вырезал ветку возврата батча →
test_failed_flush_requeues_batch_without_clobbering_freshпадает (значит, тест реально ловит потерю батча при ошибке bulk-UPDATE); вернул → зелёно.Внутренний цикл: 1 проход код + 1 внутреннее ревью (APPROVE WITH SUGGESTIONS). По итогам ревью добавил устойчивость: при ошибке bulk-записи батч возвращается в аккумулятор через
setdefault(свежая параллельная запись не затирается) — раньше падение флеша теряло до 60 с access-time и могло со временем выселить ещё используемый файл из кэша. Ключевые инварианты, за которыми проследил: snapshot-then-clear синхронно доawait,global _access_updatesпри переприсваивании,str(channel)на обоих сайтах и во флеше, порядок shutdown (финальный flush после отмены таска и до остановки io-пула).База —
fix/stage-4-eventloop(#12), стек стадий; ретаргет наfix/stabilityпо мере мержа цепочки.Ревью — #15 (fix(stability): стадия 5 — батчинг записи access-time в SQLite), round 1. Вердикт: CHANGES
Отличная, аккуратная concurrency-работа — веер 9 аспектов, 7 LGTM. Дизайн аккумулятора АДВЕРСАРИАЛЬНО ПОДТВЕРЖДЁН корректным (не по прозе): stability атаковал 7 векторов — все чисто: snapshot-then-clear атомарен (
pending = _access_updates; _access_updates = {}без единогоawaitмежду чтением и переприсваиванием → хиты во время флеша целиком попадают в новый dict, не теряются); re-queue race-free (setdefaultбез await до мутации, свежая запись не затирается, timestamp не регрессирует); рост ограничен (ключи = закэшированные файлы, чистится раз в 60с); shutdown-порядок верный (финальный flush ПОСЛЕ отмены loop-таска и ДОio_executor.shutdown→ executor жив, нет двойного флеша); оба hot-path-write доказанно на loop'е (не в тред-пуле стадии-4). coherence: DoD достигнут (оба hot-сайта только в аккумулятор; единственная другая hot-path SQLite-запись — вынесенный условный MIME-кэш). Bulk-SQL байт-в-байт с single (SET added=? WHERE channel=? AND post_id=? AND file_unique_id=?, аргументы переставлены корректно). security (параметризовано, рост ограничен за HMAC-гейтом), regressions (eviction-эффект ≤60с против 20-дневного окна — пренебрежимо), architecture, conventions — LGTM.Объективка ЗЕЛЁНАЯ — ВОСПРОИЗВЁЛ сам (venv,
pytest tests/→ 240 passed = 233+7); 7 новых тестов мутационно кусают (test-coverage проверил: сдвиг clear после await / удаление re-queue / setdefault→[key]= / повторная sync-запись / своп tuple-колонок — все дают КРАСНЫЙ). CHANGES — из-за 2 low-хвостов (не бага): непокрытый DoD-путь + фактически неверный коммент. Эскалаций нет.📋 Do (2 low) + ⛔ DROP + что сверено
Do — apply these, then re-review
[test-coverage · low] Прямой guard на ГЛАВНЫЙ hot-path (DoD-путь) не покрыт —
api_server.py:1139(get_mediapre-semaphore cache-hit).DoD PR'а — «cache-hit /media делает ноль SQLite-записей в горячем пути». Spy-тест
test_cache_hit_records_accumulator_no_sqliteзамокиваетapi_server.update_media_file_access_syncи ассертит, что она не зовётся — но гоняет толькоdownload_media_file. Самый горячий изменённый сайт — pre-semaphore fast-path вget_media(ровно тот путь, ради которого PR и делается) — прямого «zero-SQLite-write» guard'а не имеет. Регрессия, вернувшая sync/create_task-запись ИМЕННО в веткуget_media, пройдёт зелёной (download_media_file-spy её не ловит). Строка идентична покрытой, но сайт отдельный. Fix: spy-тест черезget_mediapre-semaphore cache-hit, зеркалоtest_cache_hit_records_accumulator_no_sqlite, ассертящий ноль синхронных SQLite-записей.[documentation · low] Коммент даёт ФАКТИЧЕСКИ НЕВЕРНЫЙ механизм —
api_server.py:~148(блок_access_updates).Пишет: смешение str/int ключей «would make the UPDATE's WHERE silently never match → access-time stops advancing → file falls out of the 20-day cache». Эмпирически проверил — это неверно: колонка
channel— TEXT-affinity (file_io.py:44), а bound-параметр без affinity получает TEXT-affinity столбца → int123коэрцится в'123'и СОВПАДАЕТ со строкой, записанной какstr(channel). (Прогнал:UPDATE ... WHERE channel=?с int-параметром 123 против TEXT-строки '123' → matched & updated.) Код корректен (оба сайтаstr(channel)), неверен лишь заявленный failure-mode — мейнтейнер, доверившись, унесёт ложный факт о SQLite. Fix: переформулировать без «silently never match» — например «ключимstr(channel), чтобы не полагаться на неявную affinity-коэрцию SQLite; тип держим консистентным со значением при вставке».⛔ DROP — кодеру НЕ делать · калибровочный лог (оператору)
[below-threshold]low[test-coverage]update_media_file_access_bulk_sync: «non-existent row = no-op» явно не ассертится (empty-batch — да). Врождённое поведение SQLite UPDATE-no-match, риск низкий. Опц.: один unseeded-ключ, ассерт row count не изменился.[below-threshold]low[simplification/documentation] Коммент_access_flush_loop«(_supervised still restarts us if this ever raises out.)» слегка вводит в заблуждение: внутреннийexcept Exceptionглотает всё → из loop'а наружу ничего не вылетает,_supervised-рестарт для этого цикла фактически не срабатывает (только cancel на shutdown). Не мёртвый код, лишь неточность.[below-threshold]low[stability/coherence] Два shutdown-нюанса (не блокеры, совпадают со старой семантикой): (a) cancel внутриto_thread-await →CancelledErrorминуетexcept Exception-re-queue, но батч уже отдан треду (soft state); (b) финальный flush без таймаута, ноbusy_timeout=5000ограничивает ожидание локов. Не стоит щитования.Сверено (9 аспектов + мои проверки, голова
a0458874, дифф three-dot от merge-base8d213902=утв. голова стадии-4): objective gate ВОСПРОИЗВЁЛ сам (240 passed); stability адверсариально по 7 векторам чисто (snapshot-race / re-queue / рост / loss / shutdown-порядок / _supervised / hot-write-на-loop); 7 тестов мутационно кусают; bulk-SQL байт-в-байт с single (сверил исходник); str(channel) на обоих сайтах = старому поведению; SQLite-affinity коэрцию проверил эмпирически (int-ключ ВСЁ РАВНО матчит — отсюда doc-finding); DoD достигнут (hot-path только аккумулятор + вынесенный условный MIME). Эскалаций нет.Round 1 — оба low-финдинга поправлены (
d0801ef). Багов не было, дизайн аккумулятора ты адверсариально подтвердил — спасибо за мутационную проверку.1. [test-coverage] Прямой guard на главный hot-path (
get_mediapre-semaphore) — добавлен.Новый тест
test_get_media_pre_semaphore_cache_hit_no_sqliteзеркалитdownload_media_file-spy, но через самget_media: мокаю HMAC-гейт (verify_media_digest) иprepare_file_response, спаюupdate_media_file_access_syncи ассертюcalled == []+ ключ в_access_updates. Проверил, что он site-specific: вырезал запись в аккумулятор только в веткеget_media→ новый тест краснеет, аdownload_media_file-тест остаётся зелёным (ровно та регрессия, что раньше прошла бы незамеченной, теперь ловится).2. [documentation] Неверный failure-mode в комменте — переформулировал.
Ты прав: колонка
channel— TEXT-affinity, bound-int коэрцится и матчит, «silently never match» — неверно. Убрал это утверждение; коммент теперь: ключимstr(channel), чтобы держать тип консистентным со значением при вставке и не полагаться на неявную affinity-коэрцию SQLite (код на обоих сайтах и так был корректен).DROP-хвосты не трогал (non-existent-row no-op, неточность про
_supervised-рестарт, shutdown-нюансы) — как помечено.Объективка: чистый venv,
python -m pytest tests/→ 241 passed (было 240 + 1 новый guard). Правки — только тест + коммент, продакшн-логика не менялась, поэтому отдельный внутренний цикл не гонял; вместо него — адверсариальная мутация нового теста (выше). База прежняя —fix/stage-4-eventloop.Ревью — #15 (fix(stability): стадия 5 — батчинг записи access-time в SQLite), round 2. Вердикт: PASS ✅
Оба low-пункта round-1 закрыты и проверены; concurrency-дизайн (адверсариально подтверждён в round-1) не тронут — единственное изменение прод-кода — комментарий, так что новых багов внести неоткуда.
test_get_media_pre_semaphore_cache_hit_no_sqliteреально проходит через pre-semaphore cache-hit веткуget_media(настоящий cache-файл, digest-гейт и FileResponse замоканы), spy'итupdate_media_file_access_syncи ассертитcalled == [](ноль синхронных SQLite-записей) + ключ в_access_updates+ возврат через cache-hit путь. Мутационно проверил сам: вживил sync-запись в ту веткуget_media→ тест ПАДАЕТ (AssertionError наcalled == []); откатил чисто. Тест кусает.Объективка ЗЕЛЁНАЯ — ВОСПРОИЗВЁЛ сам (venv):
pytest tests/→ 241 passed (240 + 1 новый), stage5 8 passed. Регрессий нет.Что сверено
Сверено (голова
d0801ef0, дельта round-1→2a0458874..d0801ef0): дельта = 1 spy-тест (get_mediaDoD-путь) + 1 переформулированный комментарий; прод-логика (аккумулятор/flush/re-queue/shutdown) не менялась. objective gate ВОСПРОИЗВЁЛ сам (241 passed); новый тест мутационно кусает (вживлённая sync-запись в ветку get_media → красный, откат чист); doc-коммент теперь фактически верен (сверил с эмпирикой SQLite-affinity из round-1). Оба r1-DROP (non-existent-row-assert, _supervised-коммент) остались below-threshold, не требовались. Эскалаций нет.