fix(cadastre): remove shared phase_state early-exit (race condition root-cause) #183

Merged

lekss361 merged 1 commit from fix/cadastre-remove-shared-phase-state into main

2026-05-15 14:41:27 +00:00

lekss361 commented

2026-05-15 14:33:31 +00:00

(Migrated from github.com)

Summary

ROOT CAUSE FIX for low cadastre coverage. Revealed by Fix C (#182) metrics — pilot v7 made only 5 NSPD requests for 50 quarters because 95% of parallel Celery workers early-exited without doing any work.

Root cause

`cadastre_jobs.phase_state` is shared between ALL parallel workers of the same job (one row per job_id). The early-exit in `harvest_quarter`:

```python
if phase_state.get("phase") == "done":
return result
```

triggered as soon as the FIRST worker completed its quarter (Phase 4 → progress_cb writes `{"phase": "done"}`). All other workers (concurrently running and re-reading same shared row) saw `phase=done` and bailed before Phase 1.

Effect:

Pilot v7 (post #180/#181/#182): 5 NSPD req / 50 quarters = 0.1 per quarter
Full ekb_full job #8: 4950 req / 2408 quarters = 2 per quarter — 95%+ workers skipped
`cad_parcels` coverage 1.6%, `cad_zouit/enk` = 0 — not because Fix A/B/C broken, but because workers never reached Phase 1/1.5

Fix

Delete the early-exit entirely. Idempotency is already guaranteed by `ON CONFLICT DO UPDATE` in every `upsert_*` function. If a Celery task is re-enqueued (acks_late + retry), it re-runs harvest_quarter and writes the same rows — no duplicates.

Cost: slight extra NSPD calls on re-enqueue (rare). Benefit: correct execution for ALL quarters in concurrent jobs.

Tests

`test_harvest_quarter_phase_done_early_return` → renamed to `test_harvest_quarter_does_not_early_exit_on_shared_phase_done` with inverted assertion: search_by_quarter MUST be called even when shared phase_state.phase = "done" (from another quarter's worker).
36/36 passing.

Test plan

Local pytest 36 passed
After merge + deploy → pilot v8 → expect:
- `requests_count` >= 50 (at least 1 NSPD call per quarter)
- cad_zouit/cad_enk > 0 (Fix A from #180 now actually triggers)
- More broken quarters auto-healed (Fix B from #181 now triggers for all)

Closes part of #168 (root-cause).

## Summary **ROOT CAUSE FIX** for low cadastre coverage. Revealed by Fix C (#182) metrics — pilot v7 made only **5 NSPD requests for 50 quarters** because 95% of parallel Celery workers early-exited without doing any work. ## Root cause \`cadastre_jobs.phase_state\` is **shared between ALL parallel workers** of the same job (one row per job_id). The early-exit in \`harvest_quarter\`: \`\`\`python if phase_state.get(\"phase\") == \"done\": return result \`\`\` triggered as soon as the FIRST worker completed its quarter (Phase 4 → progress_cb writes \`{\"phase\": \"done\"}\`). All other workers (concurrently running and re-reading same shared row) saw \`phase=done\` and bailed before Phase 1. Effect: - Pilot v7 (post #180/#181/#182): **5 NSPD req / 50 quarters = 0.1 per quarter** - Full ekb_full job #8: **4950 req / 2408 quarters = 2 per quarter** — 95%+ workers skipped - \`cad_parcels\` coverage 1.6%, \`cad_zouit/enk\` = 0 — not because Fix A/B/C broken, but because workers never reached Phase 1/1.5 ## Fix Delete the early-exit entirely. Idempotency is already guaranteed by \`ON CONFLICT DO UPDATE\` in every \`upsert_*\` function. If a Celery task is re-enqueued (acks_late + retry), it re-runs harvest_quarter and writes the same rows — no duplicates. Cost: slight extra NSPD calls on re-enqueue (rare). Benefit: **correct execution** for ALL quarters in concurrent jobs. ## Tests - \`test_harvest_quarter_phase_done_early_return\` → renamed to \`test_harvest_quarter_does_not_early_exit_on_shared_phase_done\` with **inverted assertion**: search_by_quarter MUST be called even when shared phase_state.phase = \"done\" (from another quarter's worker). - 36/36 passing. ## Test plan - [x] Local pytest 36 passed - [ ] After merge + deploy → pilot v8 → expect: - \`requests_count\` >= 50 (at least 1 NSPD call per quarter) - cad_zouit/cad_enk > 0 (Fix A from #180 now actually triggers) - More broken quarters auto-healed (Fix B from #181 now triggers for all) Closes part of #168 (root-cause).

lekss361 commented

2026-05-15 14:41:19 +00:00

(Migrated from github.com)

Review

CI зелёный (backend 1m9s + frontend 1m41s) + cross-check (agent):

✅ Race diagnosis правильный

cadastre_jobs.phase_state per-job_id (single row keyed by job_id):

scrape_cadastre.py:84 — phase_state = COALESCE(phase_state, '{}') || :update (JSONB merge на ту же row)
scrape_cadastre.py:109 — UPDATE cadastre_jobs SET ... WHERE job_id = :id
Старый bulk_harvest.py:118 — WHERE job_id = :id без quarter filter

Каждый parallel worker для того же job читает/пишет ту же row → race реальна. Объясняет pilot v7 (5 req / 50 quarters = 0.1/q) и job #8 (4950 / 2408 ≈ 2/q): первый worker заканчивал quarter → progress_cb писал phase=done в shared row → все остальные workers читали и бейлились до Phase 1.

✅ Idempotency подтверждена — все 8 upsert_* имеют ON CONFLICT

Функция	Line	ON CONFLICT
`upsert_parcel`	442	`(cad_num) DO UPDATE`
`upsert_building`	555	`(cad_num) DO UPDATE`
`upsert_construction`	674	`(cad_num) DO UPDATE`
`upsert_onc`	770	`(cad_num) DO UPDATE`
`upsert_enk`	831	`(cad_num) DO UPDATE`
`upsert_zouit`	914	`(reg_numb_border, category_id) DO UPDATE`
`upsert_quarter_stats`	1083	`(cad_number) DO UPDATE`
`upsert_quarter_geom_from_feature`	1011	pure UPDATE (naturally idempotent)

Zero plain INSERT без conflict clause. Re-enqueued task safe.

✅ Resumability tradeoff приемлем

Удалённый early-exit давал ложную resumability (он не работал из-за race). Реальный recovery layer — Celery acks_late+retry. Worst case re-enqueue → лишние NSPD calls, bounded _SEMAPHORE(3) + _MAX_RETRIES=3.

✅ Test correctly inverted

test_harvest_quarter_does_not_early_exit_on_shared_phase_done мокает shared phase_state = {"phase": "done", "quarter": "66:41:9999999"} (от другого quarter), assert'ит:

search_by_quarter.assert_awaited_once_with(...) — work выполняется
result.phase_state == {"phase":"done","quarter":"66:41:0303161"} — этот worker пишет СВОЙ quarter, не сохраняет shared

Сильный regression shield.

✅ Drift отсутствует

HarvestResult.phase_state unchanged. update_progress callback unchanged. Только internal behavior harvest_quarter меняется. No TS/pydantic.

Контекст

Этот PR — root cause fix для всей серии #175–#182. После deploy pilot v8 должен дать:

≥50 NSPD req для 50 quarters (т.е. ≥1 на quarter)
ZOUIT/ENK >0 (Fix #180 теперь реально триггерится)
Больше healed quarters (Fix #181 теперь реально триггерится для всех)
requests_count теперь точно отражает реальную нагрузку (Fix #182)

approve merge

## Review CI зелёный (backend 1m9s + frontend 1m41s) + cross-check (agent): ### ✅ Race diagnosis правильный `cadastre_jobs.phase_state` per-job_id (single row keyed by `job_id`): - `scrape_cadastre.py:84` — `phase_state = COALESCE(phase_state, '{}') || :update` (JSONB merge на ту же row) - `scrape_cadastre.py:109` — `UPDATE cadastre_jobs SET ... WHERE job_id = :id` - Старый `bulk_harvest.py:118` — `WHERE job_id = :id` без quarter filter Каждый parallel worker для того же job читает/пишет ту же row → race реальна. Объясняет pilot v7 (5 req / 50 quarters = 0.1/q) и job #8 (4950 / 2408 ≈ 2/q): первый worker заканчивал quarter → progress_cb писал `phase=done` в shared row → все остальные workers читали и бейлились до Phase 1. ### ✅ Idempotency подтверждена — все 8 upsert_* имеют ON CONFLICT | Функция | Line | ON CONFLICT | |---|---|---| | `upsert_parcel` | 442 | `(cad_num) DO UPDATE` | | `upsert_building` | 555 | `(cad_num) DO UPDATE` | | `upsert_construction` | 674 | `(cad_num) DO UPDATE` | | `upsert_onc` | 770 | `(cad_num) DO UPDATE` | | `upsert_enk` | 831 | `(cad_num) DO UPDATE` | | `upsert_zouit` | 914 | `(reg_numb_border, category_id) DO UPDATE` | | `upsert_quarter_stats` | 1083 | `(cad_number) DO UPDATE` | | `upsert_quarter_geom_from_feature` | 1011 | pure UPDATE (naturally idempotent) | Zero plain INSERT без conflict clause. Re-enqueued task safe. ### ✅ Resumability tradeoff приемлем Удалённый early-exit давал ложную resumability (он не работал из-за race). Реальный recovery layer — Celery acks_late+retry. Worst case re-enqueue → лишние NSPD calls, bounded `_SEMAPHORE(3)` + `_MAX_RETRIES=3`. ### ✅ Test correctly inverted `test_harvest_quarter_does_not_early_exit_on_shared_phase_done` мокает shared `phase_state = {"phase": "done", "quarter": "66:41:9999999"}` (от другого quarter), assert'ит: - `search_by_quarter.assert_awaited_once_with(...)` — work выполняется - `result.phase_state == {"phase":"done","quarter":"66:41:0303161"}` — этот worker пишет СВОЙ quarter, не сохраняет shared Сильный regression shield. ### ✅ Drift отсутствует `HarvestResult.phase_state` unchanged. `update_progress` callback unchanged. Только internal behavior `harvest_quarter` меняется. No TS/pydantic. ### Контекст Этот PR — **root cause fix** для всей серии #175–#182. После deploy pilot v8 должен дать: - ≥50 NSPD req для 50 quarters (т.е. ≥1 на quarter) - ZOUIT/ENK >0 (Fix #180 теперь реально триггерится) - Больше healed quarters (Fix #181 теперь реально триггерится для всех) - `requests_count` теперь точно отражает реальную нагрузку (Fix #182) approve merge