fix(metrics): end remote catalog transactions eagerly; document state-file deploys

AGENT.md

@@ -446,6 +446,63 @@ Every merge to `main` triggers `.github/workflows/release.yaml`:
 
 The tarball is the primary artifact for external Docker builds (e.g. `posthog-cloud-infra`). It includes the lockfile so `uv sync --frozen` produces reproducible installs with pinned binary wheels. Do not distribute standalone wheels — they lack the lockfile and resolve unpinned deps from PyPI.
 
+## Promoting to Prod
+
+Dev tracks head automatically: the `commit_state_update` dispatch above
+writes `state.millpond.image.sha` in PostHog/charts and the deploy bot
+promotes `image.dev` (mw-dev rolls immediately). Prod is manual by
+policy — `state/millpond.yaml` sets `require_prod_approval: true`, so
+the ONLY path to prod is the charts repo's `promote-to-prod.yml`
+workflow, gated by required reviewers on the
+`prod-promote-managed-warehouse` GitHub environment. Results post to
+#alerts-managed-warehouse. (Same mechanism as duckgres and viaduck.)
+
+1. **Resolve the ref** — the promotable ref is the multi-arch manifest
+   digest recorded in the state file, never a per-arch digest:
+
+   ```bash
+   STATE=$(gh api -H "Accept: application/vnd.github.raw" \
+     /repos/PostHog/charts/contents/state/millpond.yaml)
+   DEV=$(echo "$STATE" | yq '.state.millpond.image.dev')
+   PROD=$(echo "$STATE" | yq '.state.millpond.image.prod')
+   echo "dev:  $DEV"; echo "prod: $PROD"
+   # The <git-sha> prefix of $DEV must be the millpond main commit you
+   # intend to ship. If dev lags sha, the CD/deploy-bot hop hasn't
+   # landed yet — wait.
+   ```
+
+2. **Show the delta going out**: `git log --oneline "${PROD%%@*}..${DEV%%@*}"`
+
+3. **Fire the promotion** (parks at the approval gate; nothing deploys yet):
+
+   ```bash
+   gh workflow run promote-to-prod.yml -R PostHog/charts \
+     -f app=millpond -f image="$DEV"
+   ```
+
+   Open the run page (`gh run list -R PostHog/charts
+   --workflow=promote-to-prod.yml --limit 1 --json url --jq '.[0].url'`)
+   and have a required reviewer approve the pending deployment. Never
+   self-approve programmatically on the operator's behalf.
+
+4. **Watch and verify**: `gh run watch <run-id> -R PostHog/charts
+   --exit-status`, then confirm `image.prod` in the state file moved.
+   ArgoCD rolls the prod StatefulSets; `kubectl -n millpond get pods -w`
+   for verification beyond ArgoCD. Rollback = promote the previous
+   known-good ref (visible in `git log -- state/millpond.yaml` in the
+   charts repo) through the same workflow.
+
+Two millpond-specific notes:
+
+- The `viaduck-metrics` Deployment (viaduck namespace) runs THIS image
+  at millpond's prod pin — it hosts `tools/ducklake_metrics.py`. A
+  millpond prod promotion rolls that daemon too.
+- The repo-local `.github/workflows/promote-to-prod.yaml` (registry
+  retag of `:prod`) is NOT the fleet deploy path anymore; the mutable
+  `:prod` tag remains only for consumers that deliberately pull it
+  fresh per run (maintenance CronJobs). The ingest StatefulSets follow
+  the digest-pinned state file only.
+
 ## Deployment Strategy
 
 Rolling updates are a poor fit for static partition assignment — during the roll, pods run with different `REPLICA_COUNT` values, causing temporary double-assignment (duplicate writes) or gaps. Since Kafka is the durable buffer, a simpler strategy works:

tests/unit/test_ducklake_metrics.py

@@ -269,6 +269,58 @@ def test_column_name_mismatch_raises_through_error_path(self, conn, registry):
 
         assert _gauge_value(registry, "ducklake_metrics_query_errors_total", {"query": "t_mismatch"}) == 1
 
+    def test_success_leaves_no_open_transaction(self, conn, registry):
+        # Regression (2026-09-10): under autocommit, duckdb-postgres ended
+        # the remote transaction lazily, so the daemon's attached-Postgres
+        # connection sat "idle in transaction" on the shared catalog for
+        # the whole inter-query interval. _run_query now brackets every
+        # query in an explicit BEGIN/COMMIT. A leftover open transaction
+        # makes the BEGIN below raise ("cannot start a transaction within
+        # a transaction").
+        q = dm.Query(name="t_txn", help="t", sql="SELECT 1 AS n", interval_seconds=60, labels=[], values=["n"])
+        gauges = dm._build_query_gauges([q], registry=registry)
+        sm = dm._build_self_metrics(registry=registry)
+
+        assert _run(conn, q, gauges[q.name], sm) is True
+        conn.execute("BEGIN")
+        conn.execute("COMMIT")
+
+    def test_failure_leaves_no_open_transaction(self, conn, registry):
+        # The failure path must roll the bracket back, not leak it.
+        q = dm.Query(
+            name="t_txn_fail", help="t", sql="SELECT * FROM no_such_table", interval_seconds=60, labels=[], values=["n"]
+        )
+        gauges = dm._build_query_gauges([q], registry=registry)
+        sm = dm._build_self_metrics(registry=registry)
+
+        assert _run(conn, q, gauges[q.name], sm) is False
+        conn.execute("BEGIN")
+        conn.execute("COMMIT")
+
+    def test_server_sql_fallback_still_works_inside_bracket(self, conn, registry):
+        # The server-side form fails on a lake without the pg attach
+        # (BinderException / CatalogException). The bracket must survive
+        # that failed statement and run the local fallback in a fresh
+        # transaction.
+        conn.execute("CREATE TABLE fb (n INTEGER)")
+        conn.execute("INSERT INTO fb VALUES (7)")
+        q = dm.Query(
+            name="t_fallback",
+            help="t",
+            sql="SELECT n FROM fb",
+            server_sql="SELECT n FROM fb",
+            interval_seconds=60,
+            labels=[],
+            values=["n"],
+        )
+        gauges = dm._build_query_gauges([q], registry=registry)
+        sm = dm._build_self_metrics(registry=registry)
+
+        assert _run(conn, q, gauges[q.name], sm) is True
+        assert _gauge_value(registry, "t_fallback_n") == 7.0
+        conn.execute("BEGIN")
+        conn.execute("COMMIT")
+
 
 # ---------------------------------------------------------------------------
 # Built-in: ducklake_pending_deletes — validate the SQL shape against an

tools/ducklake_metrics.py

@@ -749,6 +749,23 @@ def _run_query(
     if liveness is not None:
         liveness.current_query_start = t0
     try:
+        # Explicit transaction bracket, NOT for atomicity (these are
+        # single read-only statements) but for remote-connection hygiene.
+        # Under autocommit, duckdb-postgres ends the REMOTE transaction
+        # lazily: after a statement completes, the attached-Postgres
+        # connection sits in its REPEATABLE READ transaction ("idle in
+        # transaction" in pg_stat_activity) until the next statement
+        # reuses it — for this daemon, the entire inter-query interval
+        # (minutes). An idle-in-transaction snapshot on the shared
+        # catalog pins vacuum and lengthens every writer's OCC conflict
+        # window. An explicit COMMIT ends the remote transaction eagerly
+        # (verified against megaduck 2026-09-10: bare scan left the
+        # connection in-transaction for the full idle window;
+        # pg_pool_max_connections=0 did NOT help — the primary
+        # connection is exempt from the pool; a BEGIN/COMMIT bracket
+        # returned it clean). Safe here: the scheduler is strictly
+        # serial on this connection.
+        conn.execute("BEGIN")
         cur = None
         if q.server_sql is not None:
             # Server-side first (see Query.server_sql). Bind/catalog errors
@@ -761,13 +778,28 @@ def _run_query(
                     f"{ducklake_maintenance._sql_string_literal(q.server_sql)})"
                 )
             except (duckdb.BinderException, duckdb.CatalogException) as e:
+                # The failed statement may have aborted the explicit
+                # transaction; reopen it so the fallback runs cleanly.
+                # ROLLBACK is best-effort: depending on the duckdb
+                # version the failed statement either aborts the
+                # transaction (ROLLBACK required) or unwinds it
+                # (ROLLBACK raises "no transaction is active").
+                try:
+                    conn.execute("ROLLBACK")
+                except duckdb.Error:
+                    pass
+                conn.execute("BEGIN")
                 if q.name not in _SERVER_SQL_FALLBACK_LOGGED:
                     _SERVER_SQL_FALLBACK_LOGGED.add(q.name)
                     log.info("query %s: server-side form unavailable (%s); using local metadata attach", q.name, e)
         if cur is None:
             cur = conn.execute(q.sql)
         cols = [d[0] for d in cur.description]
         rows = cur.fetchall()
+        # Commit as soon as the cursor is drained: gauge bookkeeping
+        # below must not extend the remote transaction's lifetime, and a
+        # gauge-side exception must not leave the transaction open.
+        conn.execute("COMMIT")
         try:
             label_idx = [cols.index(name) for name in q.labels]
             value_idx = [cols.index(name) for name in q.values]
@@ -795,6 +827,13 @@ def _run_query(
         log.debug("query %s: %d rows in %.3fs", q.name, len(rows), elapsed)
         return True
     except Exception:
+        # Close any transaction the failure left open (including the
+        # explicit bracket above). Best-effort: on a dead connection the
+        # ROLLBACK fails too, and the reconnect path owns recovery.
+        try:
+            conn.execute("ROLLBACK")
+        except Exception:  # noqa: BLE001
+            pass
         log.exception("query %s failed", q.name)
         self_metrics.errors.labels(tenant, q.name).inc()
         return False