fix(pzz_loader): SAVEPOINT pattern + outer error handler (transaction correctness) #124

Merged

lekss361 merged 1 commit from fix/pzz-loader-savepoint into main

2026-05-14 20:29:23 +00:00

lekss361 commented

2026-05-14 20:22:59 +00:00

(Migrated from github.com)

Summary

Per code review audit (May 14): inner db.rollback() в loop ломал outer transaction → inconsistent counts vs DB state.

Change

backend/app/services/site_finder/pzz_loader.py sync_pzz_zones_to_db() (lines 79-162):

Before — failure одной row rolls back whole outer tx:

for f in features:
    try:
        db.execute(upsert_stmt, params)
        inserted += 1
    except Exception as e:
        db.rollback()  # ❌ rolls back ALL previous successful rows
        skipped += 1
        continue
db.commit()

After — SAVEPOINT per-row + outer error handler:

try:
    for f in features:
        try:
            with db.begin_nested():  # SAVEPOINT
                db.execute(upsert_stmt, params)
                inserted += 1
        except Exception as e:
            logger.warning("failed row: %s", e)
            skipped += 1
            continue
    db.commit()
except Exception as e:
    db.rollback()
    logger.exception("pzz_loader: unexpected outer error: %s", e)
    raise
finally:
    db.close()

Pattern matches domrf_kn.py SAVEPOINT usage (reference).

Preserved

Function signature + return {fetched, inserted, updated, skipped} shape — unchanged
skipped count теперь корректно включает DB-failure rows
All logging contracts

Tests

No test_pzz_loader.py exists — skipped
ruff + AST passed clean

Vault

fixes/Bug_Pzz_Loader_Missing_Savepoint_May14.md — created.

Closes audit critical #6.

## Summary Per code review audit (May 14): inner `db.rollback()` в loop ломал outer transaction → inconsistent counts vs DB state. ## Change `backend/app/services/site_finder/pzz_loader.py` `sync_pzz_zones_to_db()` (lines 79-162): **Before** — failure одной row rolls back **whole** outer tx: ```python for f in features: try: db.execute(upsert_stmt, params) inserted += 1 except Exception as e: db.rollback() # ❌ rolls back ALL previous successful rows skipped += 1 continue db.commit() ``` **After** — SAVEPOINT per-row + outer error handler: ```python try: for f in features: try: with db.begin_nested(): # SAVEPOINT db.execute(upsert_stmt, params) inserted += 1 except Exception as e: logger.warning("failed row: %s", e) skipped += 1 continue db.commit() except Exception as e: db.rollback() logger.exception("pzz_loader: unexpected outer error: %s", e) raise finally: db.close() ``` Pattern matches `domrf_kn.py` SAVEPOINT usage (reference). ## Preserved - Function signature + return `{fetched, inserted, updated, skipped}` shape — unchanged - `skipped` count теперь корректно включает DB-failure rows - All logging contracts ## Tests - No `test_pzz_loader.py` exists — skipped - ruff + AST passed clean ## Vault `fixes/Bug_Pzz_Loader_Missing_Savepoint_May14.md` — created. Closes audit critical #6.

lekss361 commented

2026-05-14 20:25:11 +00:00

(Migrated from github.com)

Bot review

Правильный fix для классического анти-паттерна: db.rollback() внутри цикла откатывает внешнюю транзакцию, а не одну запись.

✅ db.begin_nested() = SAVEPOINT — изолирует rollback одной записи, остальные inserted/updated сохраняются
✅ Outer try/except/raise — программные ошибки (не data errors) корректно пропагируются после db.rollback()
✅ finally: db.close() — leak-safe
✅ skipped counter теперь правдив — включает DB-failure rows (раньше счётчик расходился с реальностью, т.к. rollback стирал inserted в той же tx)
✅ Function signature + return shape {fetched, inserted, updated, skipped} сохранены

Технические заметки

db.begin_nested() требует активной outer-транзакции. В SQLAlchemy 2.0 это автоматически — первый db.execute() в session запускает autobegin. Дополнительный db.begin() не нужен. db.commit() в конце завершает autobegin'нутую outer tx.

PR утверждает что pattern matches domrf_kn.py (reference) — доверяю, не проверял grep'ом.

CI

backend: 🔄
frontend: 🔄

Мерджу когда оба зелёные.

## Bot review Правильный fix для классического анти-паттерна: `db.rollback()` внутри цикла откатывает **внешнюю** транзакцию, а не одну запись. ✅ **`db.begin_nested()` = SAVEPOINT** — изолирует rollback одной записи, остальные `inserted/updated` сохраняются ✅ **Outer try/except/raise** — программные ошибки (не data errors) корректно пропагируются после `db.rollback()` ✅ **finally: db.close()** — leak-safe ✅ **`skipped` counter теперь правдив** — включает DB-failure rows (раньше счётчик расходился с реальностью, т.к. rollback стирал inserted в той же tx) ✅ Function signature + return shape `{fetched, inserted, updated, skipped}` сохранены ### Технические заметки `db.begin_nested()` требует активной outer-транзакции. В SQLAlchemy 2.0 это автоматически — первый `db.execute()` в session запускает autobegin. Дополнительный `db.begin()` не нужен. `db.commit()` в конце завершает autobegin'нутую outer tx. PR утверждает что pattern matches `domrf_kn.py` (reference) — доверяю, не проверял grep'ом. ### CI - backend: 🔄 - frontend: 🔄 Мерджу когда оба зелёные.

lekss361 commented

2026-05-14 20:27:52 +00:00

(Migrated from github.com)

Tracking: part of audit batch #127. Vault: fixes/Bug_Pzz_Loader_Missing_Savepoint_May14.md.

Tracking: part of audit batch #127. Vault: `fixes/Bug_Pzz_Loader_Missing_Savepoint_May14.md`.

lekss361 commented

2026-05-14 20:28:51 +00:00

(Migrated from github.com)

Final review (SHA `a7617a2`, CI ✅)

backend: SUCCESS
frontend: SUCCESS
Transaction correctness fix — корректное лечение rollback-in-loop

approve merge

## Final review (SHA `a7617a2`, CI ✅) - backend: SUCCESS - frontend: SUCCESS - Transaction correctness fix — корректное лечение rollback-in-loop approve merge