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>
This commit is contained in:
1 parent
0155fd3f38
commit
d6b69d0842
6 files changed
+103
-33
No files matched your search
@@ -0,0 +1,40 @@
|
||||
# План: исправления 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` безопасна для существующих вызовов.
|
||||
- Откат - возврат к предыдущему коммиту этапа; схема данных не затрагивается.
|
||||
Reference in new issue
Block a user