Files
ripe-cidr-collector/docs/plan-review-fixes-5-10.md
ayurishchevandClaude Sonnet 5 d6b69d0842 Stage A of review fixes: single read snapshot, redacted /health, batched diff
/addresses reads the cursor and data in one session and one snapshot,
/health exposes only time and backup file name of the last restore,
get_changes checks presence in batches instead of per value (6.8 s -> 88 ms
on a 12k-entry journal). Adds the plan for review fixes 5-10.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-21 10:06:31 +03:00

7.7 KiB
Raw Permalink Blame History

План: исправления 5-10 по результатам ревью

Источник: docs/review-2026-09-21.md. Находки 1-4 закрыты (summary-input-hardening.md, summary-loss-guard.md). Здесь - низкие по серьёзности находки 5-10 и связанный пробел тестов (задание backup, /health). Влияние проверено по графу и grep: get_cidrs/get_fqdn_ips вызываются только из get_addresses; run_job вызывают планировщик, check_collect_requests и один тест.

Два этапа (каждый со своими тестами и коммитом)

Этап А: путь чтения (находки 5, 6, 10)

# Исправление Файлы
5 Одна сессия и один снимок в /addresses. Новый контекст db.read_transaction(conn) (BEGIN ... COMMIT, единый снимок WAL). Внутри одной сессии: проверка ensure_data_ready, курсор journal_head и данные по всем запрошенным типам; форматирование ответа - после закрытия сессии. Функции get_cidrs и get_fqdn_ips удаляются. Итог: одно соединение вместо трёх, а курсор в заголовке X-Changes-Cursor точно соответствует выданным данным (комментарий «курсор читаем до данных» становится не нужен). db.py, api_server.py
6 /health не отдаёт внутренние пути. last_restore в ответе - только {"at", "backup"} (backup - имя файла копии без каталога); полная запись (карантин, текст ошибки) остаётся в last_restore.json для оператора. api_server.py
10 Проверка наличия в get_changes пакетами. Вместо запроса на каждое значение - SELECT kind, value FROM addresses WHERE value IN (...) порциями по 500 (в пределах лимита переменных SQLite), пары (kind, value) сверяются в Python. Результат тот же, число запросов падает с N до N/500. db.py

Тесты этапа А (новых 1, дополняется 1 существующий): /health не содержит путей (запись last_restore.json подставляется тестом); в test_change_journal добавляется пакет из 1200 значений (переход через границу порции), результат сверяется с ожидаемым.

Этап Б: демон, образ, внешний источник (находки 7, 8, 9)

# Исправление Файлы
7 Состояние заданий под одной блокировкой. _state_lock становится RLock, все изменения job_state (run_job, schedule_jobs, sync_schedule) выполняются with _state_lock; write_status вызывается после короткого обновления, как сейчас. Поведение не меняется, гонка между записью статуса и обновлением состояния исчезает. collector_daemon.py
8 Dockerfile не зависит от списка модулей. COPY *.py ./ (в корне лежат только модули приложения; тесты и прочее - в подкаталогах или исключены .dockerignore) и проверка на этапе сборки RUN python -c "import api_server, collector_daemon, db, healthcheck": забытый или сломанный модуль ломает сборку, а не запуск. Dockerfile
9 Повторы и sourceapp для RIPEstat. Сессия requests с urllib3.Retry: до 3 повторов при сбоях соединения и кодах 429/500/502/503/504, экспоненциальная пауза (уважается Retry-After); в запрос добавляется sourceapp (по умолчанию ripe-cidr-collector, переопределяется ключом ripestat_sourceapp в config.json, например с контактом). Сессия создаётся на один запуск сбора. После исчерпания повторов поведение прежнее: источник пропускается, ничего не удаляется. Худший случай на один ASN: около 50 с (4 попытки по 10 с и паузы), в расписании */15 это допустимо. cidr_collector.py, README.md

Тесты этапа Б (новых 2): задание backup в демоне (run_backup создаёт копию и уважает backup_keep; закрывает пробел из ревью); сессия RIPEstat (в запросе есть sourceapp, включённые повторы и коды, сбой после повторов даёт None).

Итого

Новых тестов 3, всего 27 (плюс расширение существующего). README: ripestat_sourceapp, поведение повторов, /health.

Порядок и проверка

  1. Этап А -> тесты в контейнере -> вручную: параллельные запросы /addresses во время записи сборщика (данные и курсор согласованы), /health без путей, большой since=0 на базе с несколькими тысячами изменений (время ответа до и после) -> коммит.
  2. Этап Б -> тесты в контейнере -> вручную: сборка образа (проверка импорта проходит; намеренно удалённый модуль ломает сборку), сбой RIPEstat имитируется локальным сервером, который дважды отвечает 503, затем 200 (повторы срабатывают, в лог пишется предупреждение), демон и API в отдельных процессах, Docker Compose -> коммит.
  3. Итоги docs/summary-review-fixes-5-10.md, статусы в отчёте ревью, README.

Не входит

  • Архитектурные пункты ревью (вынос путей в settings.py, разделение api_server.py и cidr_collector.py): отдельная доработка после наблюдаемости, чтобы не смешивать с исправлениями поведения.
  • Ограничение частоты POST /collect (риск 8 анализа) и повторные запросы DNS.

Риски и откат

  • Этап А: изменения только на чтении, результат проверяется тестами и сравнением ответов до и после (ответы /addresses и /addresses/diff должны совпасть побайтно на одной базе).
  • Этап Б: повторы увеличивают худшее время одного запуска сбора (описано выше); блокировка RLock безопасна для существующих вызовов.
  • Откат - возврат к предыдущему коммиту этапа; схема данных не затрагивается.