fix(metrics): end remote catalog transactions eagerly; document state-file deploys - #139
Merged
Conversation
…EGIN/COMMIT The daemon's attached-Postgres connection sat 'idle in transaction' (REPEATABLE READ) on the shared megaduck catalog for the entire inter-query interval — minutes at a time, renewed every cycle, so effectively permanent. An idle-in-transaction snapshot pins vacuum and lengthens every writer's OCC conflict window on a catalog that is already the fleet's contention point. Root cause (verified live against megaduck, 2026-09-10): duckdb-postgres under autocommit ends the REMOTE transaction lazily — the connection stays parked in its transaction until the next statement reuses it. pg_pool_max_connections=0 does NOT fix this; the pool setting governs only the secondary connections, and the primary stays parked (also verified live). An explicit transaction commits the remote side eagerly. _run_query now brackets every query in BEGIN/COMMIT: - COMMIT fires as soon as the cursor is drained, so gauge bookkeeping cannot extend the transaction and a gauge-side exception cannot leak it - the server_sql -> local fallback reopens the bracket after the expected BinderException (rollback is version-tolerant: the failed statement either aborts the transaction or unwinds it) - the failure path rolls back best-effort; a dead connection stays the reconnect path's problem Three regression tests pin the no-open-transaction postcondition on the success, failure, and fallback paths (a leaked transaction makes the probe BEGIN raise 'cannot start a transaction within a transaction').
Millpond now deploys via the charts state-file mechanism (same as duckgres and viaduck): release dispatch -> state/millpond.yaml -> dev auto-promotes, prod behind the prod-promote-managed-warehouse approval gate. Documents the resolve/delta/fire/approve/verify procedure, that viaduck-metrics rides millpond's prod pin, and that the repo-local promote-to-prod retag now serves only mutable-tag CronJob consumers.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
viaduck-metrics(this repo'stools/ducklake_metrics.pyrunning against megaduck) holds its attached-Postgres connection idle in transaction (REPEATABLE READ) for the entire inter-query interval — minutes at a time, renewed every cycle, so effectively permanent. An idle-in-transaction snapshot on the shared catalog pins vacuum and lengthens every writer's OCC conflict window — noticed while working the 2026-09-09/10 millpond restart-storm incident, where catalog OCC contention was the stall mechanism.Root cause (verified live against megaduck)
duckdb-postgres under autocommit ends the remote transaction lazily: after a statement completes, the connection stays parked in its transaction until the next statement reuses it. Live fault matrix:
pg_pool_max_connections=0BEGIN/COMMITbracketidle,DISCARD ALLChanges
_run_querybrackets every query in explicitBEGIN/COMMIT.COMMITfires as soon as the cursor drains (gauge bookkeeping can't extend the transaction; a gauge-side exception can't leak it). The server-side→local fallback reopens the bracket after the expectedBinderException(version-tolerant rollback). The failure path rolls back best-effort.viaduck-metricsrides millpond's prod pin and the repo-local:prodretag now serves only mutable-tag CronJob consumers.How did you test this code?
Metrics suite 73/73; full suite 876 passed + 1 xfail; ruff clean on touched files. Root cause and fix behavior verified live against megaduck from the running
viaduck-metricspod (see fault matrix).