/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>
7.7 KiB
План: исправления 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.
Порядок и проверка
- Этап А -> тесты в контейнере -> вручную: параллельные запросы
/addressesво время записи сборщика (данные и курсор согласованы),/healthбез путей, большойsince=0на базе с несколькими тысячами изменений (время ответа до и после) -> коммит. - Этап Б -> тесты в контейнере -> вручную: сборка образа (проверка импорта проходит; намеренно удалённый модуль ломает сборку), сбой RIPEstat имитируется локальным сервером, который дважды отвечает 503, затем 200 (повторы срабатывают, в лог пишется предупреждение), демон и API в отдельных процессах, Docker Compose -> коммит.
- Итоги
docs/summary-review-fixes-5-10.md, статусы в отчёте ревью, README.
Не входит
- Архитектурные пункты ревью (вынос путей в
settings.py, разделениеapi_server.pyиcidr_collector.py): отдельная доработка после наблюдаемости, чтобы не смешивать с исправлениями поведения. - Ограничение частоты
POST /collect(риск 8 анализа) и повторные запросы DNS.
Риски и откат
- Этап А: изменения только на чтении, результат проверяется тестами и сравнением ответов до и после (ответы
/addressesи/addresses/diffдолжны совпасть побайтно на одной базе). - Этап Б: повторы увеличивают худшее время одного запуска сбора (описано выше); блокировка
RLockбезопасна для существующих вызовов. - Откат - возврат к предыдущему коммиту этапа; схема данных не затрагивается.