From 19922bad42ec4f342d802b1d84808130f9e8a8fa Mon Sep 17 00:00:00 2001 From: Chris Munns Date: Mon, 14 Sep 2026 16:42:15 -0400 Subject: [PATCH 1/4] Retry table COPY after a source connection failure Treat 57P01, 57P02 and 57P03 as connection errors, and reset both the source and target connections between COPY attempts. When the source is the snapshot connection, re-open the snapshot as well. Skip the COMMIT at snapshot close when the connection is already gone. Adds the copy-source-reconnect test suite. --- src/bin/pgcopydb/pgsql.c | 27 +++++++- src/bin/pgcopydb/snapshot.c | 3 +- src/bin/pgcopydb/table-data.c | 83 ++++++++++++++++++------ tests/Makefile | 7 +- tests/copy-source-reconnect/Dockerfile | 9 +++ tests/copy-source-reconnect/Makefile | 15 +++++ tests/copy-source-reconnect/compose.yaml | 42 ++++++++++++ tests/copy-source-reconnect/copydb.sh | 61 +++++++++++++++++ 8 files changed, 222 insertions(+), 25 deletions(-) create mode 100644 tests/copy-source-reconnect/Dockerfile create mode 100644 tests/copy-source-reconnect/Makefile create mode 100644 tests/copy-source-reconnect/compose.yaml create mode 100755 tests/copy-source-reconnect/copydb.sh diff --git a/src/bin/pgcopydb/pgsql.c b/src/bin/pgcopydb/pgsql.c index 43614bccd..ebbcf49a1 100644 --- a/src/bin/pgcopydb/pgsql.c +++ b/src/bin/pgcopydb/pgsql.c @@ -2484,12 +2484,24 @@ is_response_ok(PGresult *result) #define SQLSTATE_IS_CONNECTION_EXCEPTION(pgsql) \ (pgsql->sqlstate[0] == '0' && pgsql->sqlstate[1] == '8') +/* never widen to Class 57: 57014 query_canceled must not be retried */ +#define SQLSTATE_IS_SESSION_TERMINATION(pgsql) \ + (strncmp(pgsql->sqlstate, "57P01", 5) == 0 || \ + strncmp(pgsql->sqlstate, "57P02", 5) == 0 || \ + strncmp(pgsql->sqlstate, "57P03", 5) == 0) + bool pgsql_state_is_connection_error(PGSQL *pgsql) { - return pgsql->connection != NULL && - (PQstatus(pgsql->connection) == CONNECTION_BAD || - SQLSTATE_IS_CONNECTION_EXCEPTION(pgsql)); + if (pgsql->connection != NULL && + PQstatus(pgsql->connection) == CONNECTION_BAD) + { + return true; + } + + return pgsql->status == PG_CONNECTION_BAD || + SQLSTATE_IS_CONNECTION_EXCEPTION(pgsql) || + SQLSTATE_IS_SESSION_TERMINATION(pgsql); } @@ -3288,6 +3300,10 @@ pgcopy_log_error(PGSQL *pgsql, PGresult *res, const char *context) strlcpy(pgsql->sqlstate, sqlstate, sizeof(pgsql->sqlstate)); } } + else + { + pgsql->sqlstate[0] = '\0'; + } char *endpoint = pgsql->connectionType == PGSQL_CONN_SOURCE ? "SOURCE" : "TARGET"; @@ -3325,6 +3341,11 @@ pgcopy_log_error(PGSQL *pgsql, PGresult *res, const char *context) PQclear(res); } + if (PQstatus(pgsql->connection) == CONNECTION_BAD) + { + pgsql->status = PG_CONNECTION_BAD; + } + clear_results(pgsql); pgsql_finish(pgsql); } diff --git a/src/bin/pgcopydb/snapshot.c b/src/bin/pgcopydb/snapshot.c index 2b7bf352c..8828f28f2 100644 --- a/src/bin/pgcopydb/snapshot.c +++ b/src/bin/pgcopydb/snapshot.c @@ -258,7 +258,8 @@ copydb_close_snapshot(CopyDataSpec *copySpecs) { (void) pgsql_finish(&(snapshot->stream.pgsql)); } - else if (snapshot->kind == SNAPSHOT_KIND_SQL) + else if (snapshot->kind == SNAPSHOT_KIND_SQL && + pgsql->connection != NULL) { /* only COMMIT sql snapshot kinds, no need for logical rep ones */ if (!pgsql_commit(pgsql)) diff --git a/src/bin/pgcopydb/table-data.c b/src/bin/pgcopydb/table-data.c index b94a5588e..0729eed0d 100644 --- a/src/bin/pgcopydb/table-data.c +++ b/src/bin/pgcopydb/table-data.c @@ -1383,6 +1383,42 @@ typedef struct UpdateCopyStatsContext } UpdateCopyStatsContext; +static bool +copydb_reset_copy_connections(CopyDataSpec *specs, PGSQL *src, PGSQL *dst) +{ + bool srcIsSnapshot = src == &(specs->sourceSnapshot.pgsql); + + (void) pgsql_finish(src); + (void) pgsql_finish(dst); + + if (srcIsSnapshot) + { + if (!copydb_set_snapshot(specs)) + { + log_error("Failed to re-open the source snapshot \"%s\", " + "see above for details", + specs->sourceSnapshot.snapshot); + return false; + } + + pgsql_set_copy_retry_policy(&(src->retryPolicy)); + } + else if (pgsql_open_connection(src) == NULL) + { + return false; + } + + if (!pgsql_set_gucs(dst, dstSettings)) + { + log_error("Failed to set our GUC settings on the target connection, " + "see above for details"); + return false; + } + + return true; +} + + /* * copydb_copy_table implements the sub-process activity to pg_dump | * pg_restore the table's data and then create the indexes and the constraints @@ -1434,33 +1470,40 @@ copydb_copy_table(CopyDataSpec *specs, PGSQL *src, PGSQL *dst, { ++attempts; - /* re-init stats between attempts */ - CopyStats empty = { 0 }; - stats = empty; + bool connected = + attempts == 1 || + copydb_reset_copy_connections(specs, src, dst); - UpdateCopyStatsContext context = { - .specs = specs, - .tableSpecs = tableSpecs - }; + if (connected) + { + /* re-init stats between attempts */ + CopyStats empty = { 0 }; + stats = empty; - /* ignore previous attempts, we need only one success here */ - success = pg_copy(src, dst, - &(tableSpecs->copyArgs), &stats, - &context, ©db_update_copy_stats_hook); + UpdateCopyStatsContext context = { + .specs = specs, + .tableSpecs = tableSpecs + }; - if (success) - { - /* success, get out of the retry loop */ - if (attempts > 1) + /* ignore previous attempts, we need only one success here */ + success = pg_copy(src, dst, + &(tableSpecs->copyArgs), &stats, + &context, ©db_update_copy_stats_hook); + + if (success) { - log_info("Table %s COPY succeeded after %d attempts", - tableSpecs->sourceTable->qname, - attempts); + /* success, get out of the retry loop */ + if (attempts > 1) + { + log_info("Table %s COPY succeeded after %d attempts", + tableSpecs->sourceTable->qname, + attempts); + } + break; } - break; } - /* only retry on Connection Exception errors (SQLSTATE class 08) */ + /* only retry when the source or the target connection is lost */ bool isConnectionError = pgsql_state_is_connection_error(src) || pgsql_state_is_connection_error(dst); diff --git a/tests/Makefile b/tests/Makefile index 2820bfe6c..d4b390172 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -18,7 +18,8 @@ all: pagila pagila-multi-steps blobs unit filtering filtering-standby extensions follow-pgoutput follow-wal2json follow-standby follow-9.6 follow-data-only follow-target-reconnect \ endpos-in-multi-wal-txn exclude-extension \ blob-snapshot-release follow-defer-indexes fk-not-valid defer-validate-fks \ - fk-partition-clone follow-defer-validate-fks follow-sequence-reset; + fk-partition-clone follow-defer-validate-fks follow-sequence-reset \ + copy-source-reconnect; pagila: build $(MAKE) -C $@ @@ -113,6 +114,9 @@ follow-defer-validate-fks: build follow-sequence-reset: build $(MAKE) -C $@ +copy-source-reconnect: build + $(MAKE) -C $@ + timescaledb: build $(MAKE) -C $@ @@ -128,3 +132,4 @@ build: .PHONY: endpos-in-multi-wal-txn exclude-extension .PHONY: blob-snapshot-release follow-defer-indexes fk-not-valid defer-validate-fks .PHONY: fk-partition-clone follow-defer-validate-fks follow-sequence-reset +.PHONY: copy-source-reconnect diff --git a/tests/copy-source-reconnect/Dockerfile b/tests/copy-source-reconnect/Dockerfile new file mode 100644 index 000000000..1a4cb78cf --- /dev/null +++ b/tests/copy-source-reconnect/Dockerfile @@ -0,0 +1,9 @@ +FROM pagila + +COPY --from=pgcopydb /usr/local/bin/pgcopydb /usr/local/bin + +WORKDIR /usr/src/pgcopydb +COPY copydb.sh copydb.sh + +USER docker +CMD ["/usr/src/pgcopydb/copydb.sh"] diff --git a/tests/copy-source-reconnect/Makefile b/tests/copy-source-reconnect/Makefile new file mode 100644 index 000000000..d405ef0e4 --- /dev/null +++ b/tests/copy-source-reconnect/Makefile @@ -0,0 +1,15 @@ +# Copyright (c) 2021 The PostgreSQL Global Development Group. +# Licensed under the PostgreSQL License. + +test: down run down ; + +run: build + $(DOCKER) compose run test + +down: + $(DOCKER) compose down + +build: + $(DOCKER) compose build + +.PHONY: down build test diff --git a/tests/copy-source-reconnect/compose.yaml b/tests/copy-source-reconnect/compose.yaml new file mode 100644 index 000000000..de666474f --- /dev/null +++ b/tests/copy-source-reconnect/compose.yaml @@ -0,0 +1,42 @@ +services: + source: + image: postgres:${PGVERSION:-16} + expose: + - 5432 + environment: + POSTGRES_USER: postgres + POSTGRES_PASSWORD: h4ckm3 + POSTGRES_HOST_AUTH_METHOD: trust + command: > + -c ssl=on + -c ssl_cert_file=/etc/ssl/certs/ssl-cert-snakeoil.pem + -c ssl_key_file=/etc/ssl/private/ssl-cert-snakeoil.key + target: + image: postgres:${PGVERSION:-16} + expose: + - 5432 + environment: + POSTGRES_USER: postgres + POSTGRES_PASSWORD: h4ckm3 + POSTGRES_HOST_AUTH_METHOD: trust + command: > + -c ssl=on + -c ssl_cert_file=/etc/ssl/certs/ssl-cert-snakeoil.pem + -c ssl_key_file=/etc/ssl/private/ssl-cert-snakeoil.key + test: + build: + context: . + dockerfile: Dockerfile + cap_add: + - SYS_ADMIN + - SYS_PTRACE + environment: + PGSSLMODE: "require" + PGCOPYDB_SOURCE_PGURI: postgres://postgres:h4ckm3@source/postgres + PGCOPYDB_TARGET_PGURI: postgres://postgres:h4ckm3@target/postgres + PGCOPYDB_TABLE_JOBS: 1 + PGCOPYDB_INDEX_JOBS: 1 + PGCOPYDB_FAIL_FAST: "true" + depends_on: + - source + - target diff --git a/tests/copy-source-reconnect/copydb.sh b/tests/copy-source-reconnect/copydb.sh new file mode 100755 index 000000000..971eaf364 --- /dev/null +++ b/tests/copy-source-reconnect/copydb.sh @@ -0,0 +1,61 @@ +#! /bin/bash + +set -x +set -e +set -o pipefail + +export PAGER=cat + +pgcopydb ping + +psql -d ${PGCOPYDB_SOURCE_PGURI} < pg_backend_pid() + limit 1; + + if victim is not null + then + perform pg_terminate_backend(victim); + exit; + end if; + + perform pg_sleep(0.02); + end loop; +end +\$\$; +EOF +killer=$! + +pgcopydb clone --notice 2>&1 | tee /tmp/clone.log + +wait ${killer} + +grep "COPY succeeded after" /tmp/clone.log + +count=$(psql -At -d ${PGCOPYDB_TARGET_PGURI} -c 'select count(*) from copytest') + +if [ "${count}" != "400000" ] +then + echo "target table copytest has ${count} rows, expected 400000" + exit 1 +fi + +pgcopydb compare data From 650cf084d519f6559f17f60f57372be2aa93bd9d Mon Sep 17 00:00:00 2001 From: Chris Munns Date: Mon, 14 Sep 2026 16:42:24 -0400 Subject: [PATCH 2/4] Stop CDC when the endpos is set on an idle source The feedback-timer guard tested cur_record_lsn, which is declared inside the streaming loop and reset to InvalidXLogRecPtr on every iteration, so the test never passed. Test the position already streamed instead. Without this, setting the sentinel endpos on a source with no further WAL activity never stops the stream. A replica source cannot be nudged with a write, so the follow process runs forever. --- src/bin/pgcopydb/pgsql.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bin/pgcopydb/pgsql.c b/src/bin/pgcopydb/pgsql.c index ebbcf49a1..11ca6b3b6 100644 --- a/src/bin/pgcopydb/pgsql.c +++ b/src/bin/pgcopydb/pgsql.c @@ -4289,7 +4289,7 @@ pgsql_stream_logical(LogicalStreamClient *client, LogicalStreamContext *context) * happens, stop streaming — we have all the WAL we need. */ if (context->endpos != InvalidXLogRecPtr && - context->endpos <= cur_record_lsn) + context->endpos <= client->current.written_lsn) { log_info("Streamed up to write_lsn %X/%X, " "flush_lsn %X/%X, stopping: " From a74b0d85ff8a77fe516c2de0fb7799617c805531 Mon Sep 17 00:00:00 2001 From: Chris Munns Date: Mon, 14 Sep 2026 16:42:30 -0400 Subject: [PATCH 3/4] Skip smoke tests on fork pull requests Run them on pushes and same-repo pull requests only. Also register the copy-source-reconnect suite in the tiered test matrix. --- .github/workflows/run-tests-tiered.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/run-tests-tiered.yml b/.github/workflows/run-tests-tiered.yml index 366579712..af36905ac 100644 --- a/.github/workflows/run-tests-tiered.yml +++ b/.github/workflows/run-tests-tiered.yml @@ -76,6 +76,7 @@ jobs: name: Smoke tests (PG ${{ matrix.PGVERSION }}) runs-on: ubuntu-latest needs: [style_checker, banned_api_checker, docs] + if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository strategy: fail-fast: true matrix: @@ -165,6 +166,7 @@ jobs: - fk-partition-clone - follow-defer-validate-fks - follow-sequence-reset + - copy-source-reconnect steps: - name: Checkout repository uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 From 48b084c615ed0cd9058188e688b6b21191e67da0 Mon Sep 17 00:00:00 2001 From: Chris Munns Date: Mon, 14 Sep 2026 16:42:30 -0400 Subject: [PATCH 4/4] Move the base image to Debian 12 Debian 11 is on LTS and its security archive no longer serves the package versions its index advertises, so apt-get install fails with a 404 on the pgcopydb image. This broke the eight test suites that build a Dockerfile.inject on that image. tests/Dockerfile.pagila already uses bookworm. Also pass PGCOPYDB_IMAGE to the exclude-extension test service, which hardcoded pgcopydb:pg16 and so failed the PG17 and PG18 matrix cells. --- Dockerfile | 8 ++++---- tests/exclude-extension/compose.yaml | 2 ++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Dockerfile b/Dockerfile index 4e8de2342..361dea94d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,6 +1,6 @@ # syntax=docker/dockerfile:latest # Define a base image with all our build dependencies. -FROM debian:11-slim AS build +FROM debian:12-slim AS build # multi-arch ARG TARGETPLATFORM @@ -18,7 +18,7 @@ RUN dpkg --add-architecture ${TARGETARCH:-arm64} && apt update \ gnupg RUN curl https://www.postgresql.org/media/keys/ACCC4CF8.asc | apt-key add - -RUN echo "deb http://apt.postgresql.org/pub/repos/apt bullseye-pgdg main ${PGVERSION}" > /etc/apt/sources.list.d/pgdg.list +RUN echo "deb http://apt.postgresql.org/pub/repos/apt bookworm-pgdg main ${PGVERSION}" > /etc/apt/sources.list.d/pgdg.list RUN dpkg --add-architecture ${TARGETARCH:-arm64} && apt update \ && apt install -qqy --no-install-recommends \ @@ -89,7 +89,7 @@ RUN make -s clean && make -s -j$(nproc) install COPY tests tests # Now the "run" image, as small as possible -FROM debian:11-slim AS run +FROM debian:12-slim AS run # multi-arch ARG TARGETPLATFORM @@ -110,7 +110,7 @@ RUN dpkg --add-architecture ${TARGETARCH:-arm64} && apt update \ gnupg RUN curl https://www.postgresql.org/media/keys/ACCC4CF8.asc | apt-key add - -RUN echo "deb http://apt.postgresql.org/pub/repos/apt bullseye-pgdg main ${PGVERSION}" > /etc/apt/sources.list.d/pgdg.list +RUN echo "deb http://apt.postgresql.org/pub/repos/apt bookworm-pgdg main ${PGVERSION}" > /etc/apt/sources.list.d/pgdg.list RUN dpkg --add-architecture ${TARGETARCH:-arm64} && apt update \ && apt install -qqy --no-install-suggests --no-install-recommends \ diff --git a/tests/exclude-extension/compose.yaml b/tests/exclude-extension/compose.yaml index 3f944621b..1311ee3fa 100644 --- a/tests/exclude-extension/compose.yaml +++ b/tests/exclude-extension/compose.yaml @@ -28,6 +28,8 @@ services: build: context: . dockerfile: Dockerfile + args: + PGCOPYDB_IMAGE: pgcopydb:pg${PGVERSION:-16} environment: PGCOPYDB_SOURCE_PGURI: postgres://postgres:h4ckm3@source/postgres PGCOPYDB_TARGET_PGURI: postgres://postgres:h4ckm3@target/postgres