fix(postgres): send the cleanup ROLLBACK only when a transaction block is open

nicodargerequested to merge
nicodarge1/glitchtip-rust:fix-conditional-rollback-on-release into main

What

release_sync and the Drop guard send ROLLBACK unconditionally before returning a pinned connection to the pool. When no transaction block is open — the steady-state autocommit pin case, hit once per scheduler tick — PostgreSQL answers with WARNING: there is no transaction in progress and logs it server-side by default: ~86,400 lines/day on an idle GlitchTip 6.2.2 install (measured; the code comment assumed a silent NOTICE, but it is a WARNING). Closes #1 (closed).

How

  • RustTransaction gains an in_tx: Arc<AtomicBool>: begin() starts at true, pin() at false.
  • Single statements routed through the object update it via tx_transition (first-keyword inspection: BEGIN/START open, COMMIT/END/ROLLBACK close, ROLLBACK TO keeps the block open). The helper lives in gt-postgres (pyo3-free core) with unit tests, per the repo's core/bindings split.
  • Tracking is submission-time, which errs toward rolling back: a failed BEGIN costs one no-op ROLLBACK; a failed COMMIT still ends the block backend-side.
  • batch_execute_sync deliberately does not parse its script: naive splitting misreads PL/pgSQL bodies (BEGIN ... END; inside a dollar-quoted CREATE FUNCTION) as top-level transaction control, which would disarm the Drop guard mid-migration. It conservatively marks the block open — worst case one no-op ROLLBACK on release of a DDL-running session, never the hot path.
  • The ServerSideCursor micro-transaction safety net is preserved: a DECLARE failing between BEGIN and COMMIT leaves the flag set, so the release still rolls back.

Verification

  • cargo fmt / clippy clean; gt-postgres tests 90 unit + 17 wire, all green (3 new unit tests for tx_transition).
  • pytest suite unchanged (221 passed; the 5 test_pg_asyncio copy_in failures are pre-existing on main in the same environment).
  • Wire-level check against postgres 17 with log_statement=all: releasing an idle pin sends no statement at all; releasing with an open BEGIN block sends exactly one ROLLBACK. Same check on v0.6.1 shows the WARNING once per release.

Note

The structurally-complete alternative would be surfacing the backend's authoritative transaction status from the ReadyForQuery wire message, which the vendored tokio-postgres already receives and discards (vendor/tokio-postgres/src/simple_query.rs, src/query.rs) — happy to rework in that direction if you prefer patching the vendored crate (as done for the buffer cap in GT_PATCH.md); the text-level tracking here has the advantage of not touching the vendor tree.

Merge request reports