# История правок shared-sqs — 2026-10-07 (P1.2 + фикс утечки блокировки)

> Запись по требованию «документировать всё, включая собственные ошибки».
> Предыдущие записи: `doc/thinking/*-2026-10-0[56].md`, `HISTORY/2026-08-1*-session-log.md`.

## 1. Задача

Пользователь: «доделать ПОЛНОСТЬЮ и протестировать SQS». Незакрытым по плану
`plan-for-flash-2026-10-05.md` оставался единственный пункт — **P1.2
(индексация сообщений)**, отложенный 2026-10-06 с обоснованием.

## 2. Что сделано

### 2.1. Дизайн (коммит `b58e958`)

`doc/thinking/p1.2-message-index-design-2026-10-07.md`.

Ключевое решение: **tombstone + индексы + амортизированная компакция**.
Удаление не сдвигает срез (O(1)), поэтому индексы `receipt→index` и `uuid→index`
остаются валидными. Физическая чистка — `Compact`/`CompactIfNeeded` в `processQueueTick`.
Redis-схема не меняется (сообщения и так лежат в per-message HASH).

### 2.2. Код (коммит `22e9bc8`)

- `models.Queue`: `deadIdx`, `receiptIdx`, `uuidIdx`, `liveCount`, `deadCount` (все `json:"-"`)
  + примитивы: `AppendMessage`, `RemoveIndex`, `Compact`, `CompactIfNeeded`, `Reset`,
  `EnsureIndexes`/`RebuildIndexes`, `IndexOfReceipt`, `IndexOfUUID`, `MarkInFlight`,
  `ClearInFlight`, `InFlightIndexes`, `IsDead`, `LiveCount`.
- `DeleteMessage` / `ChangeMessageVisibility(±Batch)` — поиск по индексу вместо скана среза;
  удаление без сдвига остатка.
- `DeleteMessageBatch` — tombstone вместо полной пересборки среза.
- `processQueueTick` — visibility/DLQ по индексу in-flight (O(in-flight)), retention — через
  tombstone, в конце — `CompactIfNeeded`.
- send-пути — `AppendMessage`/`LiveCount`; admin/metrics/persistence — учёт живых сообщений;
  `LoadAllQueues` — `RebuildIndexes` после загрузки.

### 2.3. Тесты (коммиты `e05af04`, `5b3fb51`)

- `app/models/message_index_test.go` — 6 тестов примитивов (tombstone, компакция, индексы, Reset, порог).
- `app/gosqs/delete_middle_test.go` — удаление СЕРЕДИНЫ очереди через API: остальные не теряются,
  удалённое не возвращается.
- `app/gosqs/tenant_flow_test.go` — существующие тесты переведены на `LiveCount()`; добавлен
  `TestReceiveEmptyQueue_DoesNotLeakLock`.

## 3. НАЙДЕННЫЙ ДЕФЕКТ (моя регрессия) — утечка per-queue блокировки

### 3.1. Проявление

Стресс (2000 сообщений × циклы): `ReadTimeout`, всплеск 1.9 с на `delete_batch`,
дальше — зависание сервиса (health при этом отвечает).

### 3.2. Диагностика (факты, не догадки)

- Снят стек горутин по `SIGQUIT`: десятки горутин в `sync.RWMutex.RLock`
  (`models.go:101 Queue.RLockMu`), включая `metrics.updateGauges` (ждала 15 минут)
  и HTTP-хендлеры `GetQueueAttributesV1`.
- Аудит всех `GetLocked`/`GetRLocked`: дефект ровно один.
- `receive_message.go`:
  ```go
  queue := models.SyncQueues.GetLocked(key)     // Lock взят
  if queue == nil || queue.LiveCount() == 0 {
      return nil, false                          // ← возврат БЕЗ Unlock
  }
  defer queue.UnlockMu()
  ```

### 3.3. Причина и почему проявилось именно сейчас

Возврат до `defer` оставлял `queue.mu` захваченным навсегда (в Go мьютекс не
освобождается при выходе из функции). Прежнее условие `len(queue.Messages) == 0`
срабатывало только на полностью пустой очереди; после перехода на tombstone
появился новый случай `LiveCount()==0` при `len(Messages)>0` (все сообщения удалены,
компакция ещё не прошла) — типовой для последовательных циклов «залить → выгрести».

### 3.4. Фикс

`defer queue.UnlockMu()` — до любой проверки/возврата. Регрессионный тест
`TestReceiveEmptyQueue_DoesNotLeakLock` проверен на дефекте (падает за 2 с)
и на фиксе (зелёный).

## 4. Результаты проверок (после фикса)

| Проверка | Результат |
|---|---|
| `go build ./...`, `go vet ./...` | OK |
| `go test -race ./app/...` | зелёные |
| `tests/api_test.sh` (aws CLI, локальный сервис) | PASS=22 FAIL=0 |
| `tests/sdk_test.py` (boto3 1.43.108) | PASS=15 FAIL=0 |
| `tests/fifo_dlq_probe.py` | fifo_order / fifo_dedup / dlq = PASS |
| Стресс 3×2000 (send-batch → receive → delete-batch) | send ≈0.6 с, drain ≈1.2 с на цикл, `left=0/0`, вызовов >0.5 с нет |

## 5. Инфраструктурные заметки (для повторного прогона)

- Локальный сервис: `SHARED_SQS_SEED_DEMO=true /tmp/shared-sqs --admin-token=... --port=4100`
  (memory-only без `REDIS_ADDR`; креды демо-тенанта печатаются в лог при старте).
- `boto3` в системе НЕ установлен (PEP 668) и `python3.12-venv` отсутствует →
  wheels скачаны через `pip download` и распакованы в `/tmp/sqlibs_unpacked`,
  запуск с `PYTHONPATH=/tmp/sqlibs_unpacked`.
- Лимит очередей тенанта — 10: падавшие скрипты оставляют очереди, перед прогоном
  чистить (`aws sqs delete-queue`).
- `ENDPOINT` (api_test.sh) и `ENDPOINT_URL` (python-тесты) — разные имена переменных.

## 6. Уроки

1. Стресс-проверка обязательна: api/sdk/fifo проходили и с дефектом, он проявлялся
   только на циклах полного опустошения очереди.
2. `SIGQUIT` — быстрый и точный способ найти зависание (кто держит мьютекс), дешевле догадок.
3. При замене условия на новый критерий (`len` → `LiveCount`) надо перечитывать
   весь блок, включая порядок `defer` — старый код содержал латентную утечку лока,
   которую новый критерий сделал массовой.
4. Не запускать сервис с `| head` — SIGPIPE убивает процесс на середине теста.

## 7. Код-ревью (роль ПРО) и фиксы — коммит `48c38c4`

Проведён архитектурный разбор всех затронутых файлов. Найдено и исправлено:

1. **🔥 Батч-видимость не заводила в DLQ** (`change_message_visibility_batch.go`).
   При `VisibilityTimeout=0` батч лишь инкрементировал `Retry` и НЕ переносил
   сообщение в DLQ — логика была только в одиночной версии, счётчик и перенос
   расходились. Вынесен единый метод `models.Queue.ResetVisibilityAndTryDLQ`
   (сброс видимости + DLQ-перенос), обе версии приведены к одному пути.
   Тест `TestChangeVisibilityBatch_MovesToDLQ` (падает на старой логике, зелёный на новой).
2. **TOCTOU лимит в `SendMessageBatch`**: проверка OOM-лимита была только по
   устаревшему снимку (RLock → Lock). Добавлена повторная проверка под per-queue Lock.
3. Инварианты `InFlightIndexes` (снимок может содержать устаревшие индексы —
   потребитель обязан перепроверять) и `ensureIndexes` (проверяет согласованность
   чисел, а не валидность map-индексов → `Messages` нельзя мутировать вне методов)
   зафиксированы в док-комментариях.
4. `Compact` упрощён: один `RebuildIndexes` на выходе + `ensureIndexes` на входе
   (убран дублирующий блок), `deadCount` пересчитывается внутри.

Осталось осознанно не тронуто: `DeleteMessageBatch` итерация с `RemoveIndex` внутри
безопасна (tombstone не меняет длину) — покрыто тестами.

## 8. Полный код-ревью сервиса (2026-10-08) — коммит `5b0e89a`

Разобран весь сервис: auth (SigV4 + JWT), router, billing, tenant, admin/UI API,
создание очередей. Критичных проблем не найдено. Исправлено три замечания:

1. **TOCTOU лимита тенантов** — `TenantStore.Create`: проверка `MaxTenantsGlobal`
   была только под RLock, вставка — позже под Lock без повторной проверки. Добавлена
   проверка под Lock (как уже было в `CreateFromJWT`).
2. **Потеря `NubesSub` при рестарте** — `CreateFromJWT`: обновлённый `NubesSub`
   сохранялся в Redis только внутри ветки смены `MaxQueues`; теперь персист при
   любом реальном изменении, запись только если что-то изменилось.
3. **`http.Client` на каждый вызов** — `auth.PingNubesAPI` переведён на общий
   клиент (keep-alive).

UI/безопасность проверены отдельно: `listTenants`/`getTenant`/IDOR для UI-тенанта —
видит только себя, чужие `{id}` отсекаются, удаление через UI запрещено,
`SecretKey` отдаётся только по явному `/ui/api/credentials`.