Skip to content

follow: release the source snapshot connection after the catalog read - #53

Open
teknogeek0 wants to merge 1 commit into
mainfrom
fix/follow-release-snapshot-connection
Open

follow: release the source snapshot connection after the catalog read#53
teknogeek0 wants to merge 1 commit into
mainfrom
fix/follow-release-snapshot-connection

Conversation

@teknogeek0

Copy link
Copy Markdown
Collaborator

Problem

Standalone pgcopydb follow (notably follow --resume) opens a source connection to hold a snapshot for the one-time initial catalog read, and then never closes it. It sits idle in transaction on the source for the entire follow run — which for an online migration can be days.

On a --resume run it's worse: the catalog read is a cache hit (the source catalog was already fetched into the local SQLite state in a prior run), so copydb_fetch_schema_and_prepare_specs returns early via "Re-using catalog caches" and this connection is opened and never even used, yet still held open for the whole run.

It holds no snapshot in that case (backend_xmin is NULL), so this is a connection-hygiene issue, not a vacuum-horizon one — but a long-lived idle-in-transaction connection is exactly the kind of thing that alarms source DBAs and would become a real problem if that transaction ever acquired a snapshot.

Fix

Close the source snapshot connection in cli_follow right after the catalog read. The follow phase streams from the replication slot, and the database setup and sequence-reset steps each open their own connection (via copydb_copy_snapshot, which copies the snapshot metadata into a fresh connection), so nothing uses this connection again. This mirrors clone --follow, which already closes its snapshot once the base copy is done (cli_clone_follow.c clone_and_follow).

The close is gated on the live connection (sourceSnapshot.pgsql.connection != NULL), not sourceSnapshot.state: when the catalog read actually runs, copydb_fetch_schema_and_prepare_specs commits and finishes this connection itself but leaves state as SET, so state is unreliable here. A non-NULL connection means the fetch left it open (cache-hit / not-consistent cases) and it's ours to close; a NULL connection means it was already torn down. (An earlier version gated on state and hit BUG: pgsql_commit() without holding an open multi statement connection on the resume/re-use path — the connection != NULL guard is what the resume paths actually need.)

Testing (PostgreSQL 18)

Check Result
make build / citus_indent --check clean
follow-sequence-reset pass (verifies sequence reset, which runs after the close, still works)
follow-data-only, cdc-endpos-between-transaction, endpos-in-multi-wal-txn, cdc-prune pass (all exercise the standalone follow / cli_follow path)
full make tests battery pass

The five standalone-follow suites are the ones that exercise cli_follow; the endpos/resume ones specifically cover the re-use path where state is stale, confirming the connection != NULL guard.

`pgcopydb follow` (standalone, e.g. `follow --resume`) opened a source
connection to hold a snapshot for the initial catalog read, then never
closed it — it stayed idle in transaction on the source for the entire
follow run (which can be days). On a --resume run the catalog read is a
cache hit, so the connection is opened and never even used, yet still held.

The follow phase streams from the replication slot, and the database setup
and sequence-reset steps each open their own connection (via
copydb_copy_snapshot), so nothing needs this connection after the catalog
read. Close it right after, mirroring clone --follow, which already closes
its snapshot once the base copy is done.

Gate the close on the live connection rather than sourceSnapshot.state:
when the catalog read runs, copydb_fetch_schema_and_prepare_specs commits
and finishes this connection itself but leaves state as SET, so state is
unreliable. A non-NULL connection means the fetch left it open and it is
ours to close; a NULL connection means it was already torn down.

It holds no snapshot in the --resume case (backend_xmin is NULL), so this
is a connection-hygiene fix, not a vacuum-horizon fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant