fix(tradein): откат транзакции в backfill + неотрицательный счётчик квот #2538

Merged

lekss361 merged 1 commit from fix/tradein-audit-backfill-quota into main

2026-07-26 22:21:09 +00:00

lekss361 commented

2026-07-26 21:42:39 +00:00

Owner

1. Одна плохая строка рвала остаток батча

cian_history_backfill.py:148 — в цикле по листингам не было db.rollback() после сбоя сохранения. Сессия оставалась в failed-transaction state, и все последующие листинги батча (до 49 штук) падали каскадом. В логе это выглядело как 45 независимых ошибок вместо одной.

В этом же файле для домов такой откат уже был, с комментарием про отравленную сессию — то есть класс дефекта команда распознала, просто для листингов пропустила.

Достижимость подтверждена: change_time берётся сырым из скрейпа и подставляется в CAST(:ct AS timestamptz) без валидации; ЦИАН отдаёт два разных формата.

2. Счётчик квот уходил в минус — причина оказалась не та, что я думал

Я предполагал, что есть путь, уменьшающий счётчик лишний раз. Проверка это опровергла: декремента в коде нет вообще, increment() делает только used+1 под условием WHERE used < lim (это фикс гонки #747) и физически не может уйти ниже нуля.

Настоящая причина нашлась в истории репозитория: отрицательные значения оставил отменённый ручной SQL-хак из рунбука. Миграция 185 тогда починила только user2 — у остальных аккаунтов тот же дефект остался. На проде сейчас у praktika за июнь стоит -3 при 42 фактических оценках.

Миграция 189 сбрасывает оставшиеся отрицательные значения в ноль и добавляет ограничение CHECK (used >= 0), чтобы будущий ручной UPDATE не мог это повторить. Идемпотентна.

Важно понимать, что это чинит порчу данных, а не логику подсчёта. Расхождение счётчика с фактом остаётся: у user2 за июль 18 против 20 реальных. Отдельный вопрос, нужно ли доводить счётчик до точного соответствия или он и не должен быть источником правды — сейчас факт всегда можно посчитать по trade_in_estimates.

Test plan

Полный набор дважды: 2668 passed, 8 skipped
ruff check чисто, проверка на ловушку :x::type чисто
Новые тесты: откат при сбое сохранения листинга, миграция 189
Миграция против живой базы не применялась — только статические проверки текста SQL

## 1. Одна плохая строка рвала остаток батча `cian_history_backfill.py:148` — в цикле по листингам не было `db.rollback()` после сбоя сохранения. Сессия оставалась в failed-transaction state, и все последующие листинги батча (до 49 штук) падали каскадом. В логе это выглядело как 45 независимых ошибок вместо одной. В этом же файле для домов такой откат уже был, с комментарием про отравленную сессию — то есть класс дефекта команда распознала, просто для листингов пропустила. Достижимость подтверждена: `change_time` берётся сырым из скрейпа и подставляется в `CAST(:ct AS timestamptz)` без валидации; ЦИАН отдаёт два разных формата. ## 2. Счётчик квот уходил в минус — причина оказалась не та, что я думал Я предполагал, что есть путь, уменьшающий счётчик лишний раз. **Проверка это опровергла:** декремента в коде нет вообще, `increment()` делает только `used+1` под условием `WHERE used < lim` (это фикс гонки #747) и физически не может уйти ниже нуля. Настоящая причина нашлась в истории репозитория: отрицательные значения оставил **отменённый ручной SQL-хак из рунбука**. Миграция 185 тогда починила **только `user2`** — у остальных аккаунтов тот же дефект остался. На проде сейчас у `praktika` за июнь стоит `-3` при 42 фактических оценках. Миграция 189 сбрасывает оставшиеся отрицательные значения в ноль и добавляет ограничение `CHECK (used >= 0)`, чтобы будущий ручной `UPDATE` не мог это повторить. Идемпотентна. Важно понимать, что это чинит **порчу данных, а не логику подсчёта**. Расхождение счётчика с фактом остаётся: у `user2` за июль 18 против 20 реальных. Отдельный вопрос, нужно ли доводить счётчик до точного соответствия или он и не должен быть источником правды — сейчас факт всегда можно посчитать по `trade_in_estimates`. ## Test plan - [x] Полный набор дважды: 2668 passed, 8 skipped - [x] `ruff check` чисто, проверка на ловушку `:x::type` чисто - [x] Новые тесты: откат при сбое сохранения листинга, миграция 189 - [ ] Миграция против живой базы не применялась — только статические проверки текста SQL

lekss361 added 1 commit 2026-07-26 21:42:40 +00:00

fix(tradein/backfill): rollback on listings save failure + nonnegative used quota

CI / changes (pull_request) Successful in 8s

Details

CI Trade-In / changes (pull_request) Successful in 9s

Details

CI Trade-In / frontend-checks (pull_request) Has been skipped

Details

CI / backend-tests (pull_request) Has been skipped

Details

CI / frontend-tests (pull_request) Has been skipped

Details

CI / openapi-codegen-check (pull_request) Has been skipped

Details

CI Trade-In / backend-tests (pull_request) Successful in 4m57s

Details

9c59586be6

1. cian_history_backfill.py listings block was missing the same
   db.rollback() the houses block already has: save_detail_enrichment()
   runs several unprotected db.execute() and only commits at the end.
   A single bad row (e.g. a non-standard Cian change_time hitting
   CAST(:ct AS timestamptz)) leaves the session in a failed-transaction
   state, and every subsequent listing in the batch (up to 49) then
   fails with PendingRollbackError -- one real failure looked like N
   independent ones in the logs.

2. account_estimate_usage.used had no floor. Prod audit: praktika
   2026-06 shows used=-3 against 42 real estimates. Migration 185
   already fixed this class of bug for user2 (negative used from a
   retired SQL-runbook bonus hack) but only reset that one username.
   No decrement path exists anywhere in app.services.account_quota --
   increment() only ever does used+1 under a WHERE used < lim guard
   (#747) -- so the minus is external (manual UPDATE), not an app bug.
   Migration 189 resets all remaining negative used rows and adds
   CHECK (used >= 0) so a future manual UPDATE can't reintroduce it.