У нас есть сервис, который собирает продавцам XML-фид для автозагрузки Авито. В воскресенье 27 сентября я выкатил миграцию данных. По моему расчёту она ставила новый признак ровно одному объявлению. После выкатки признак стоял у 321. Вреда не случилось, потому что я пересчитал помеченные строки раньше, чем фид успел пересобраться. Ниже код, запросы и мой главный промах: условие я считал в голове, а на проде не прогнал ни разу.
Откуда вообще взялся бэкфилл
Сборщик фида решает, откуда брать нишевые теги объявления: из файла, который продавец когда-то загрузил, или из настроек кабинета. От этого зависят телефон в объявлении, цена и ещё десяток полей. Решение принималось одной строкой в generate_feed.py:
ff = params.get("feed_fields") ff = ff if isinstance(ff, dict) else {} has_import = bool(ff)
Логика «если в params.feed_fields что-то лежит, значит, был импорт» работала, пока feed_fields писал только импортёр. Потом в карточке объявления появилась форма параметров, и она стала писать туда же. Объявление, у которого владелец руками выбрал «Гарантия: Есть», для сборщика превращалось в импортированное из файла. Мы нашли это на живом объявлении: его никто никогда не импортировал, а фид собирался для него по импортной ветке.
Починка напрашивалась сама. Импортёр ставит отдельный явный маркер params.feed_import_at, сборщик смотрит только на него. Со старыми импортами сложнее: они были сделаны до появления маркера, и без бэкфилла сборщик перестал бы считать их импортом. У одного такого объявления в файле был свой номер телефона. Без маркера в фид ушёл бы телефон кабинета по умолчанию, то есть покупатели звонили бы не тому человеку.
Значит, код и бэкфилл надо выкатывать одним PR.
Как я выбирал условие
Для бэкфилла нужен признак «это объявление когда-то пришло из файла». feed_fields не годится, с ним как раз и была проблема. Я прошёлся grep’ом по всем местам в apps/api и apps/worker, где пишутся ключи в params, и искал ключи с единственным писателем, импортёром.
Первый список оказался неточным. feed_legacy_id я считал импортным, а пишут его ещё четыре места: починка ссылки на фид, восстановление объявления оператором и две фоновые задачи. Его я выкинул. Остались шесть ключей, у каждого единственный писатель, feed/service.py::_ingest_items:
_IMPORTER_ONLY_KEYS = ( "feed_category_path", "feed_fields_raw", "feed_fields_xml", "avito_status_from_file", "avito_date_end_from_file", "contact_phone", ) KEY_EXISTS_PREDICATE = " OR ".join( f"params ? '{k}'" for k in _IMPORTER_ONLY_KEYS )
И сама миграция:
def upgrade() -> None: conn = op.get_bind() now_iso = datetime.now(timezone.utc).isoformat() conn.execute( text( "UPDATE ads SET params = params || jsonb_build_object(" " 'feed_import_at', CAST(:at AS text)," " 'feed_import_source', CAST(:src AS text)" ") " "WHERE deleted_at IS NULL " " AND NOT (params ? 'feed_import_at') " f" AND ({KEY_EXISTS_PREDICATE})" ), {"at": now_iso, "src": _SOURCE}, )
Здесь две вещи сделаны правильно, и обе потом пригодились. Бэкфилл пишет свой источник feed_import_source = 'backfill_feedimp01', поэтому его пометки отличаются от пометок настоящего импорта (у того upload или url). И downgrade() снимает ключи только там, где стоит этот источник.
В докстринге миграции я написал: «на проде ровно одно объявление получит маркер». Во всей базе непустой feed_fields был у двух объявлений. Одно из них пришло из файла, второе было заполнено формой. Из этого я вывел, что импортированных объявлений одно.
Где ошибка, видно уже здесь. Я посчитал, у кого непустой feed_fields. В миграции проверяется совсем другое: есть ли хоть один из шести ключей. Эти два условия я мысленно склеил и не заметил, что это разные множества.
Тест, который прошёл
Для миграции был интеграционный тест на настоящем Postgres (операторы jsonb ?, ||, - на моках не проверишь). Он загружает файл миграции по пути, запускает настоящий upgrade() на трёх строках и проверяет результат:
rows = ( ("imported", { "contact_phone": "+70000000000", "feed_fields": {"WorkExperience": "8"}, }), ("form_only", {"feed_fields": {"Guarantee": "Есть"}}), ("empty", {}), )
Ожидалось, что imported получит маркер, а form_only и empty нет. Так и вышло, CI зелёный, PR смержен в 13:31.
Задним числом видно, что три строки теста — это ровно та модель данных, которая была у меня в голове. Импортированное объявление в ней всегда несёт и импортный ключ, и нишевые поля. Строки «импортный ключ есть, feed_fields пустой» в тесте нет, потому что я не подозревал, что такие бывают. Тест честно проверил, что SQL делает то, что я задумал. Верно ли я задумал, он проверить не мог.
Что было на проде
После выкатки я посчитал помеченные строки:
SELECT count(*) FROM ads WHERE params->>'feed_import_source' = 'backfill_feedimp01';
Вышло 321. По кабинетам: 222, 98 и 1.
Разрезал все живые объявления по двум признакам, ради которых всё затевалось (запрос повторил позже в тот же день):
SELECT params ?| array['feed_category_path','feed_fields_raw','feed_fields_xml', 'avito_status_from_file','avito_date_end_from_file', 'contact_phone'] AS importer_key, (params->'feed_fields' IS NOT NULL AND params->'feed_fields' <> '{}'::jsonb) AS has_ff, params ? 'feed_import_at' AS marked, count(*) FROM ads WHERE deleted_at IS NULL GROUP BY 1, 2, 3 ORDER BY 1, 2, 3;
importer_key | has_ff | marked | count --------------+--------+--------+------- f | f | f | 4960 f | t | f | 1 t | f | f | 320 t | t | t | 1
Это состояние уже после отката, поэтому у 320 строк marked = f. До отката там стояло t.
Насколько я смог проследить, эти 320 объявлений достались нам при переезде со старой системы на n8n. Тот перенос складывал в params телефон и путь категории под теми же именами, что и наш импортёр. grep по коду этого показать не мог: он видит, кто пишет ключи сейчас, а кто писал их в базу до переезда, в коде уже не видно. Нишевых полей из файла у этих объявлений нет, и старый has_import правильно отправлял их по обычной ветке сборщика.
С пометкой они ушли бы по импортной. Телефон брался бы из старых перенесённых данных, а не из текущих настроек кабинета, и цена из файла имела бы приоритет над колонкой, которую владелец правит в карточке. Если продавец за это время сменил номер, после ближайшей пересборки фида покупатели звонили бы на старый. В одном кабинете таких объявлений было 222.
Как откатывал
downgrade() здесь не годился: он снял бы пометку и с того единственного объявления, которому она нужна. Откатывать миграцию целиком и выкатывать заново тоже не хотелось, потому что в том же релизе ехал код сборщика, который уже читает новый маркер.
Сделал руками. Сначала выгрузил все 321 помеченную строку в CSV на сервере, чтобы было к чему вернуться. Потом снял оба ключа, feed_import_at и feed_import_source, у строк с источником backfill_feedimp01, кроме одного нужного объявления. Повторный счёт показал 1.
Проверял потом не по счётчику, а по результату. Пересобрал штатной задачей фид кабинета, где лежит то самое объявление, и сравнил с предыдущей сборкой: 37 объявлений из 38 совпали байт в байт. 38-е побайтно не совпало, но набор «тег → значение» у него тот же.
Что поменял в миграции
На проде миграция уже применена, и Alembic сверяет ревизии, а не текст, так что повторно она там не запустится. Но на любой свежей базе (CI, dev, новый стенд) она бы снова пометила лишнее. Поэтому в файл ушло условие, которое я держал в голове с самого начала:
"WHERE deleted_at IS NULL " " AND NOT (params ? 'feed_import_at') " - f" AND ({KEY_EXISTS_PREDICATE})" + f" AND ({KEY_EXISTS_PREDICATE}) " + " AND params->'feed_fields' IS NOT NULL " + " AND params->'feed_fields' <> '{}'::jsonb"
revision и down_revision я не трогал. В тест добавил две строки, те самые из таблицы: импортный ключ есть, а feed_fields либо отсутствует, либо равен {}. Пометки у них быть не должно. Старое условие пометило бы обе.
Что не сработало и почему
Ревью смотрело на код, а в коде всё было корректно: условие в SQL в точности соответствовало тому, что написано в докстринге про ключи. Расхождение было между докстрингом и числом «ровно одно», и ни один ревьюер не мог его увидеть, не заглянув в прод.
grep по писателям я считал доказательством. Он и был доказательством, только для утверждения «кто пишет эти ключи в текущем коде». Для бэкфилла нужен был ответ на другой вопрос: у кого эти ключи лежат в базе. Код на него не отвечает.
И сам тест. Три строки фикстуры я придумал сам, поэтому они проверяли мою модель данных, а не реальную базу. Если бы фикстуру строил по таблице четырёх комбинаций, строка t | f появилась бы в ней сама.
Что теперь делаю перед мержем миграции данных
Беру WHERE из файла миграции буквально, копированием, и запускаю на проде как SELECT count(*) ... GROUP BY по кабинетам. Число кладу в описание PR рядом с ожидаемым. Если они не совпадают, мерж ждёт, пока я не пойму, какое из двух условий неверно.
Для этой миграции такой запрос занял бы секунду и показал 321 вместо 1 ещё до мержа.
После выкатки делаю ту же сверку второй раз, уже по маркеру источника. Именно она в этот раз и спасла. Но спасла потому, что фид собирается по расписанию, а не сразу после релиза, и у меня было окно. В следующий раз его может не быть.
Итог в числах: ждал 1, миграция пометила 321, после ручного отката осталась 1. Фид кабинета с тем самым объявлением совпал с предыдущей сборкой на 37 объявлениях из 38 байт в байт, у последнего совпал набор тегов.
