agora inbox for [email protected]help / color / mirror / Atom feed
Re: Preferred use of macro GetPGProcByNumber 7+ messages / 5 participants [nested] [flat]
* Re: Preferred use of macro GetPGProcByNumber @ 2026-01-05 08:20 David Rowley <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: David Rowley @ 2026-01-05 08:20 UTC (permalink / raw) To: Maksim.Melnikov <[email protected]>; +Cc: Chao Li <[email protected]>; [email protected] On Tue, 16 Sept 2025 at 20:06, Maksim.Melnikov <[email protected]> wrote: > Thanks for your comment, I've attached patch version without > procarray.c changes. Seems simple enough. Pushed. David ^ permalink raw reply [nested|flat] 7+ messages in thread
* [PATCH v44b 4/4] hack deadlock detector @ 2026-03-25 19:35 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Álvaro Herrera @ 2026-03-25 19:35 UTC (permalink / raw) --- src/backend/commands/cluster.c | 10 +++ src/backend/storage/lmgr/deadlock.c | 12 +++ src/include/storage/proc.h | 1 + src/test/modules/injection_points/Makefile | 1 + .../expected/repack_deadlock.out | 63 ++++++++++++++ src/test/modules/injection_points/meson.build | 1 + .../specs/repack_deadlock.spec | 83 +++++++++++++++++++ 7 files changed, 171 insertions(+) create mode 100644 src/test/modules/injection_points/expected/repack_deadlock.out create mode 100644 src/test/modules/injection_points/specs/repack_deadlock.spec diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 2c3058ba10d..d5b1dfbff69 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -372,6 +372,16 @@ ExecRepack(ParseState *pstate, RepackStmt *stmt, bool isTopLevel) /* Determine the lock mode to use. */ lockmode = RepackLockLevel((params.options & CLUOPT_CONCURRENT) != 0); + /* + * If in concurrent mode, set the PROC_IN_CONCURRENT_REPACK flag. This + * makes the deadlock checker cause anyone that would conflict with us + * to error out. It's important to set this flag ahead of actually locking + * the relation; it won't of course affect anyone until we do have a lock + * that others can conflict with. + */ + if ((params.options & CLUOPT_CONCURRENT) != 0) + MyProc->statusFlags |= PROC_IN_CONCURRENT_REPACK; + /* * If a single relation is specified, process it and we're done ... unless * the relation is a partitioned table, in which case we fall through. diff --git a/src/backend/storage/lmgr/deadlock.c b/src/backend/storage/lmgr/deadlock.c index b8962d875b6..10266e384aa 100644 --- a/src/backend/storage/lmgr/deadlock.c +++ b/src/backend/storage/lmgr/deadlock.c @@ -620,6 +620,18 @@ FindLockCycleRecurseMember(PGPROC *checkProc, proc->statusFlags & PROC_IS_AUTOVACUUM) blocking_autovacuum_proc = proc; + /* + * If we note that we're blocked by some process running + * REPACK (CONCURRENTLY), just fail. That process is + * going to upgrade its lock at some point, and it would + * be inappropriate for any other process to cause that + * to fail. + */ + if (checkProc == MyProc && + proc->statusFlags & PROC_IN_CONCURRENT_REPACK) + /* not a serious error message proposal */ + elog(ERROR, "oops, we're blocked by REPACK, time to die"); + /* We're done looking at this proclock */ break; } diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index 1dad125706e..4e340ead455 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -69,6 +69,7 @@ struct XidCache #define PROC_AFFECTS_ALL_HORIZONS 0x20 /* this proc's xmin must be * included in vacuum horizons * in all databases */ +#define PROC_IN_CONCURRENT_REPACK 0x40 /* REPACK (CONCURRENTLY) */ /* flags reset at EOXact */ #define PROC_VACUUM_STATE_MASK \ diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile index 2cd7d87c533..f7663859fe2 100644 --- a/src/test/modules/injection_points/Makefile +++ b/src/test/modules/injection_points/Makefile @@ -15,6 +15,7 @@ REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress ISOLATION = basic \ inplace \ repack \ + repack_deadlock \ repack_toast \ syscache-update-pruned \ heap_lock_update diff --git a/src/test/modules/injection_points/expected/repack_deadlock.out b/src/test/modules/injection_points/expected/repack_deadlock.out new file mode 100644 index 00000000000..42505d10933 --- /dev/null +++ b/src/test/modules/injection_points/expected/repack_deadlock.out @@ -0,0 +1,63 @@ +Parsed test spec with 2 sessions + +starting permutation: wait_before_lock add_column wakeup_before_lock check1 +injection_points_attach +----------------------- + +(1 row) + +step wait_before_lock: + REPACK (CONCURRENTLY) repack_deadlock USING INDEX repack_deadlock_pkey; + <waiting ...> +step add_column: + alter table repack_deadlock add column noise text; + <waiting ...> +step add_column: <... completed> +ERROR: oops, we're blocked by REPACK, time to die +step wakeup_before_lock: + SELECT injection_points_wakeup('repack-concurrently-before-lock'); + +injection_points_wakeup +----------------------- + +(1 row) + +step wait_before_lock: <... completed> +step check1: + INSERT INTO relfilenodes(node) + SELECT relfilenode FROM pg_class WHERE relname='repack_deadlock'; + + SELECT count(DISTINCT node) FROM relfilenodes; + + SELECT i, j FROM repack_deadlock ORDER BY i, j; + + INSERT INTO data_s1(i, j) + SELECT i, j FROM repack_deadlock; + + SELECT count(*) + FROM data_s1 d1 FULL JOIN data_s2 d2 USING (i, j) + WHERE d1.i ISNULL OR d2.i ISNULL; + +count +----- + 1 +(1 row) + +i|j +-+- +1|1 +2|2 +3|3 +4|4 +(4 rows) + +count +----- + 4 +(1 row) + +injection_points_detach +----------------------- + +(1 row) + diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build index a414abb924b..1cd88d6db65 100644 --- a/src/test/modules/injection_points/meson.build +++ b/src/test/modules/injection_points/meson.build @@ -46,6 +46,7 @@ tests += { 'basic', 'inplace', 'repack', + 'repack_deadlock', 'repack_toast', 'syscache-update-pruned', 'heap_lock_update', diff --git a/src/test/modules/injection_points/specs/repack_deadlock.spec b/src/test/modules/injection_points/specs/repack_deadlock.spec new file mode 100644 index 00000000000..9d23a6588c2 --- /dev/null +++ b/src/test/modules/injection_points/specs/repack_deadlock.spec @@ -0,0 +1,83 @@ +# Test REPACK with a concurrent transaction that would cause a deadlock +setup +{ + CREATE EXTENSION injection_points; + + CREATE TABLE repack_deadlock(i int PRIMARY KEY, j int); + INSERT INTO repack_deadlock(i, j) VALUES (1, 1), (2, 2), (3, 3), (4, 4); + + CREATE TABLE relfilenodes(node oid); + + CREATE TABLE data_s1(i int, j int); + CREATE TABLE data_s2(i int, j int); +} + +teardown +{ + DROP TABLE repack_deadlock; + DROP EXTENSION injection_points; + + DROP TABLE relfilenodes; + DROP TABLE data_s1; + DROP TABLE data_s2; +} + +session s1 +setup +{ + SELECT injection_points_set_local(); + SELECT injection_points_attach('repack-concurrently-before-lock', 'wait'); +} +# Perform the initial load and wait for s2 to do some data changes. +step wait_before_lock +{ + REPACK (CONCURRENTLY) repack_deadlock USING INDEX repack_deadlock_pkey; +} +# Check the table from the perspective of s1. +# +# Besides the contents, we also check that relfilenode has changed. + +# Have each session write the contents into a table and use FULL JOIN to check +# if the outputs are identical. +step check1 +{ + INSERT INTO relfilenodes(node) + SELECT relfilenode FROM pg_class WHERE relname='repack_deadlock'; + + SELECT count(DISTINCT node) FROM relfilenodes; + + SELECT i, j FROM repack_deadlock ORDER BY i, j; + + INSERT INTO data_s1(i, j) + SELECT i, j FROM repack_deadlock; + + SELECT count(*) + FROM data_s1 d1 FULL JOIN data_s2 d2 USING (i, j) + WHERE d1.i ISNULL OR d2.i ISNULL; +} +teardown +{ + SELECT injection_points_detach('repack-concurrently-before-lock'); +} + +session s2 +# Change the existing data. UPDATE changes both key and non-key columns. Also +# update one row twice to test whether tuple version generated by this session +# can be found. +step add_column +{ + alter table repack_deadlock add column noise text; +} + +step wakeup_before_lock +{ + SELECT injection_points_wakeup('repack-concurrently-before-lock'); +} + +# Test if data changes introduced while one session is performing REPACK +# CONCURRENTLY find their way into the table. +permutation + wait_before_lock + add_column + wakeup_before_lock + check1 -- 2.47.3 --fot5plqmb33ey33c-- ^ permalink raw reply [nested|flat] 7+ messages in thread
* [PATCH v44b 4/4] hack deadlock detector @ 2026-03-25 19:35 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Álvaro Herrera @ 2026-03-25 19:35 UTC (permalink / raw) --- src/backend/commands/cluster.c | 10 +++ src/backend/storage/lmgr/deadlock.c | 12 +++ src/include/storage/proc.h | 1 + src/test/modules/injection_points/Makefile | 1 + .../expected/repack_deadlock.out | 63 ++++++++++++++ src/test/modules/injection_points/meson.build | 1 + .../specs/repack_deadlock.spec | 83 +++++++++++++++++++ 7 files changed, 171 insertions(+) create mode 100644 src/test/modules/injection_points/expected/repack_deadlock.out create mode 100644 src/test/modules/injection_points/specs/repack_deadlock.spec diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 2c3058ba10d..d5b1dfbff69 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -372,6 +372,16 @@ ExecRepack(ParseState *pstate, RepackStmt *stmt, bool isTopLevel) /* Determine the lock mode to use. */ lockmode = RepackLockLevel((params.options & CLUOPT_CONCURRENT) != 0); + /* + * If in concurrent mode, set the PROC_IN_CONCURRENT_REPACK flag. This + * makes the deadlock checker cause anyone that would conflict with us + * to error out. It's important to set this flag ahead of actually locking + * the relation; it won't of course affect anyone until we do have a lock + * that others can conflict with. + */ + if ((params.options & CLUOPT_CONCURRENT) != 0) + MyProc->statusFlags |= PROC_IN_CONCURRENT_REPACK; + /* * If a single relation is specified, process it and we're done ... unless * the relation is a partitioned table, in which case we fall through. diff --git a/src/backend/storage/lmgr/deadlock.c b/src/backend/storage/lmgr/deadlock.c index b8962d875b6..10266e384aa 100644 --- a/src/backend/storage/lmgr/deadlock.c +++ b/src/backend/storage/lmgr/deadlock.c @@ -620,6 +620,18 @@ FindLockCycleRecurseMember(PGPROC *checkProc, proc->statusFlags & PROC_IS_AUTOVACUUM) blocking_autovacuum_proc = proc; + /* + * If we note that we're blocked by some process running + * REPACK (CONCURRENTLY), just fail. That process is + * going to upgrade its lock at some point, and it would + * be inappropriate for any other process to cause that + * to fail. + */ + if (checkProc == MyProc && + proc->statusFlags & PROC_IN_CONCURRENT_REPACK) + /* not a serious error message proposal */ + elog(ERROR, "oops, we're blocked by REPACK, time to die"); + /* We're done looking at this proclock */ break; } diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index 1dad125706e..4e340ead455 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -69,6 +69,7 @@ struct XidCache #define PROC_AFFECTS_ALL_HORIZONS 0x20 /* this proc's xmin must be * included in vacuum horizons * in all databases */ +#define PROC_IN_CONCURRENT_REPACK 0x40 /* REPACK (CONCURRENTLY) */ /* flags reset at EOXact */ #define PROC_VACUUM_STATE_MASK \ diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile index 2cd7d87c533..f7663859fe2 100644 --- a/src/test/modules/injection_points/Makefile +++ b/src/test/modules/injection_points/Makefile @@ -15,6 +15,7 @@ REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress ISOLATION = basic \ inplace \ repack \ + repack_deadlock \ repack_toast \ syscache-update-pruned \ heap_lock_update diff --git a/src/test/modules/injection_points/expected/repack_deadlock.out b/src/test/modules/injection_points/expected/repack_deadlock.out new file mode 100644 index 00000000000..42505d10933 --- /dev/null +++ b/src/test/modules/injection_points/expected/repack_deadlock.out @@ -0,0 +1,63 @@ +Parsed test spec with 2 sessions + +starting permutation: wait_before_lock add_column wakeup_before_lock check1 +injection_points_attach +----------------------- + +(1 row) + +step wait_before_lock: + REPACK (CONCURRENTLY) repack_deadlock USING INDEX repack_deadlock_pkey; + <waiting ...> +step add_column: + alter table repack_deadlock add column noise text; + <waiting ...> +step add_column: <... completed> +ERROR: oops, we're blocked by REPACK, time to die +step wakeup_before_lock: + SELECT injection_points_wakeup('repack-concurrently-before-lock'); + +injection_points_wakeup +----------------------- + +(1 row) + +step wait_before_lock: <... completed> +step check1: + INSERT INTO relfilenodes(node) + SELECT relfilenode FROM pg_class WHERE relname='repack_deadlock'; + + SELECT count(DISTINCT node) FROM relfilenodes; + + SELECT i, j FROM repack_deadlock ORDER BY i, j; + + INSERT INTO data_s1(i, j) + SELECT i, j FROM repack_deadlock; + + SELECT count(*) + FROM data_s1 d1 FULL JOIN data_s2 d2 USING (i, j) + WHERE d1.i ISNULL OR d2.i ISNULL; + +count +----- + 1 +(1 row) + +i|j +-+- +1|1 +2|2 +3|3 +4|4 +(4 rows) + +count +----- + 4 +(1 row) + +injection_points_detach +----------------------- + +(1 row) + diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build index a414abb924b..1cd88d6db65 100644 --- a/src/test/modules/injection_points/meson.build +++ b/src/test/modules/injection_points/meson.build @@ -46,6 +46,7 @@ tests += { 'basic', 'inplace', 'repack', + 'repack_deadlock', 'repack_toast', 'syscache-update-pruned', 'heap_lock_update', diff --git a/src/test/modules/injection_points/specs/repack_deadlock.spec b/src/test/modules/injection_points/specs/repack_deadlock.spec new file mode 100644 index 00000000000..9d23a6588c2 --- /dev/null +++ b/src/test/modules/injection_points/specs/repack_deadlock.spec @@ -0,0 +1,83 @@ +# Test REPACK with a concurrent transaction that would cause a deadlock +setup +{ + CREATE EXTENSION injection_points; + + CREATE TABLE repack_deadlock(i int PRIMARY KEY, j int); + INSERT INTO repack_deadlock(i, j) VALUES (1, 1), (2, 2), (3, 3), (4, 4); + + CREATE TABLE relfilenodes(node oid); + + CREATE TABLE data_s1(i int, j int); + CREATE TABLE data_s2(i int, j int); +} + +teardown +{ + DROP TABLE repack_deadlock; + DROP EXTENSION injection_points; + + DROP TABLE relfilenodes; + DROP TABLE data_s1; + DROP TABLE data_s2; +} + +session s1 +setup +{ + SELECT injection_points_set_local(); + SELECT injection_points_attach('repack-concurrently-before-lock', 'wait'); +} +# Perform the initial load and wait for s2 to do some data changes. +step wait_before_lock +{ + REPACK (CONCURRENTLY) repack_deadlock USING INDEX repack_deadlock_pkey; +} +# Check the table from the perspective of s1. +# +# Besides the contents, we also check that relfilenode has changed. + +# Have each session write the contents into a table and use FULL JOIN to check +# if the outputs are identical. +step check1 +{ + INSERT INTO relfilenodes(node) + SELECT relfilenode FROM pg_class WHERE relname='repack_deadlock'; + + SELECT count(DISTINCT node) FROM relfilenodes; + + SELECT i, j FROM repack_deadlock ORDER BY i, j; + + INSERT INTO data_s1(i, j) + SELECT i, j FROM repack_deadlock; + + SELECT count(*) + FROM data_s1 d1 FULL JOIN data_s2 d2 USING (i, j) + WHERE d1.i ISNULL OR d2.i ISNULL; +} +teardown +{ + SELECT injection_points_detach('repack-concurrently-before-lock'); +} + +session s2 +# Change the existing data. UPDATE changes both key and non-key columns. Also +# update one row twice to test whether tuple version generated by this session +# can be found. +step add_column +{ + alter table repack_deadlock add column noise text; +} + +step wakeup_before_lock +{ + SELECT injection_points_wakeup('repack-concurrently-before-lock'); +} + +# Test if data changes introduced while one session is performing REPACK +# CONCURRENTLY find their way into the table. +permutation + wait_before_lock + add_column + wakeup_before_lock + check1 -- 2.47.3 --fot5plqmb33ey33c-- ^ permalink raw reply [nested|flat] 7+ messages in thread
* xact_rollback spikes when logical walsender exits @ 2026-04-17 15:15 Nikolay Samokhvalov <[email protected]> 2026-04-20 16:35 ` Re: xact_rollback spikes when logical walsender exits Fujii Masao <[email protected]> 2026-04-21 06:38 ` Re: xact_rollback spikes when logical walsender exits vignesh C <[email protected]> 0 siblings, 2 replies; 7+ messages in thread From: Nikolay Samokhvalov @ 2026-04-17 15:15 UTC (permalink / raw) To: pgsql-hackers <[email protected]>; +Cc: Rafael Thofehrn Castro <[email protected]> Hi hackers, There is a bug on logical-replication publishers where every decoded committed transaction bumps pg_stat_database.xact_rollback. ReorderBufferProcessTXN() ends each decoded transaction with AbortCurrentTransaction() for catalog cleanup; in the walsender that is a top-level abort, so AtEOXact_PgStat_Database(isCommit=false) increments the backend-local pgStatXactRollback. The counts are flushed to shared stats on walsender exit, producing an acute spike. Result: for production systems with SREs on call and tight alerting on xact_rollback, this turns routine logical-replication operations (disabling a subscription, dropping a slot, walsender restart) into false-positive pages. Reported in [1]; also experienced at GitLab [2][3][4]. Attaching a simple patch that adds a backend-local flag pgStatXactSkipCounters in pgstat_database.c that AtEOXact_PgStat_Database() honors to skip the counter bump. Added TAP test that fails on master with 5/0 and passes with the patch. If there is agreement on this shape, happy to send patches for all supported branches. Let me know what you think. [1] https://postgr.es/m/CAG0ozMo_xWQn%2BAvv8jzbbhePGp5OnhdO%2BYWTkdg4faWSXz0Jzg%40mail.gmail.com [2] https://gitlab.com/gitlab-com/gl-infra/production/-/work_items/8290 [3] https://gitlab.com/postgres-ai/postgresql-consulting/tests-and-benchmarks/-/work_items/39 [4] https://gitlab.com/gitlab-org/orbit/knowledge-graph/-/work_items/406 Nik Attachments: [application/octet-stream] v1-logical-rollback-spike.patch (9.7K, ../../CAM527d_EbU5Li4a5FdKQjYsdF-4Lqr_i3jXmZOm7Wbb=Q2KzTw@mail.gmail.com/2-v1-logical-rollback-spike.patch) download | inline diff: From 11b70a070c40433389fec476464078a7038384e3 Mon Sep 17 00:00:00 2001 From: Nik Samokhvalov <[email protected]> Date: Thu, 16 Apr 2026 19:25:55 -0700 Subject: [PATCH] Do not attribute logical decoding aborts to xact_rollback On a logical-replication publisher, every decoded committed transaction bumps pg_stat_database.xact_rollback because the walsender ends each decoded transaction with a top-level AbortCurrentTransaction() for catalog cleanup. The counts surface in shared stats on walsender exit, producing a visible spike that has nothing to do with user rollbacks. Add pgStatXactSkipCounters, a backend-local flag consulted by AtEOXact_PgStat_Database() alongside the existing parallel-worker check. ReorderBufferProcessTXN() (both abort sites, gated on !using_subtxn) and SnapBuildClearExportedSnapshot() bracket their AbortCurrentTransaction() with it. StartTransaction() asserts the flag is clear to catch leaks. TAP test src/test/subscription/t/039_publisher_xact_rollback.pl: 5 autocommit INSERTs, DISABLE subscription, assert the xact_rollback delta is 0. Reported-by: Rafael Thofehrn Castro Discussion: https://postgr.es/m/CAG0ozMo_xWQn%2BAvv8jzbbhePGp5OnhdO%2BYWTkdg4faWSXz0Jzg%40mail.gmail.com --- src/backend/access/transam/xact.c | 1 + .../replication/logical/reorderbuffer.c | 14 +++- src/backend/replication/logical/snapbuild.c | 9 ++- src/backend/utils/activity/pgstat_database.c | 31 ++++---- src/include/pgstat.h | 3 + src/test/subscription/meson.build | 1 + .../t/039_publisher_xact_rollback.pl | 71 +++++++++++++++++++ 7 files changed, 115 insertions(+), 15 deletions(-) create mode 100644 src/test/subscription/t/039_publisher_xact_rollback.pl diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 48bc90c967353..18dff512000c7 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -2118,6 +2118,7 @@ StartTransaction(void) /* check the current transaction state */ Assert(s->state == TRANS_DEFAULT); + Assert(!pgStatXactSkipCounters); /* * Set the current transaction state information appropriately during diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c index 682d13c9f22f0..c100506e259bd 100644 --- a/src/backend/replication/logical/reorderbuffer.c +++ b/src/backend/replication/logical/reorderbuffer.c @@ -2674,9 +2674,14 @@ ReorderBufferProcessTXN(ReorderBuffer *rb, ReorderBufferTXN *txn, * Aborting the current (sub-)transaction as a whole has the right * semantics. We want all locks acquired in here to be released, not * reassigned to the parent and we do not want any database access - * have persistent effects. + * have persistent effects. In the !using_subtxn case this is a + * top-level abort; keep it out of pg_stat_database.xact_rollback. */ + if (!using_subtxn) + pgStatXactSkipCounters = true; AbortCurrentTransaction(); + /* unconditional clear is fine; it was false coming in */ + pgStatXactSkipCounters = false; /* make sure there's no cache pollution */ if (rbtxn_distr_inval_overflowed(txn)) @@ -2737,9 +2742,14 @@ ReorderBufferProcessTXN(ReorderBuffer *rb, ReorderBufferTXN *txn, /* * Force cache invalidation to happen outside of a valid transaction - * to prevent catalog access as we just caught an error. + * to prevent catalog access as we just caught an error. As above, + * keep the top-level abort out of pg_stat_database.xact_rollback. */ + if (!using_subtxn) + pgStatXactSkipCounters = true; AbortCurrentTransaction(); + /* unconditional clear is fine; it was false coming in */ + pgStatXactSkipCounters = false; /* make sure there's no cache pollution */ if (rbtxn_distr_inval_overflowed(txn)) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index c8309b96ed45c..d15db19abf3b6 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -631,8 +631,15 @@ SnapBuildClearExportedSnapshot(void) */ tmpResOwner = SavedResourceOwnerDuringExport; - /* make sure nothing could have ever happened */ + /* + * Make sure nothing could have ever happened. Keep this cleanup abort + * out of pg_stat_database.xact_rollback; the invariant that we are at + * top level is required so the abort reaches AtEOXact_PgStat_Database. + */ + Assert(!IsSubTransaction()); + pgStatXactSkipCounters = true; AbortCurrentTransaction(); + pgStatXactSkipCounters = false; CurrentResourceOwner = tmpResOwner; } diff --git a/src/backend/utils/activity/pgstat_database.c b/src/backend/utils/activity/pgstat_database.c index 7f3bc0165931c..00c349631ca7b 100644 --- a/src/backend/utils/activity/pgstat_database.c +++ b/src/backend/utils/activity/pgstat_database.c @@ -291,21 +291,28 @@ pgstat_fetch_stat_dbentry(Oid dboid) pgstat_fetch_entry(PGSTAT_KIND_DATABASE, dboid, InvalidOid, NULL); } +/* + * When true, AtEOXact_PgStat_Database() skips the xact_commit / + * xact_rollback bump for the current (sub-)transaction end. Logical + * decoding sets this around its catalog-cleanup AbortCurrentTransaction() + * so that abort is not attributed to pg_stat_database. + */ +bool pgStatXactSkipCounters = false; + void AtEOXact_PgStat_Database(bool isCommit, bool parallel) { - /* Don't count parallel worker transaction stats */ - if (!parallel) - { - /* - * Count transaction commit or abort. (We use counters, not just - * bools, in case the reporting message isn't sent right away.) - */ - if (isCommit) - pgStatXactCommit++; - else - pgStatXactRollback++; - } + if (parallel || pgStatXactSkipCounters) + return; + + /* + * Count transaction commit or abort. (We use counters, not just bools, + * in case the reporting message isn't sent right away.) + */ + if (isCommit) + pgStatXactCommit++; + else + pgStatXactRollback++; } /* diff --git a/src/include/pgstat.h b/src/include/pgstat.h index dfa2e8376382a..8e90903b56b7f 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -875,6 +875,9 @@ extern PGDLLIMPORT PgStat_Counter pgStatBlockWriteTime; extern PGDLLIMPORT PgStat_Counter pgStatActiveTime; extern PGDLLIMPORT PgStat_Counter pgStatTransactionIdleTime; +/* see AtEOXact_PgStat_Database() */ +extern PGDLLIMPORT bool pgStatXactSkipCounters; + /* updated by the traffic cop and in errfinish() */ extern PGDLLIMPORT SessionEndType pgStatSessionEndCause; diff --git a/src/test/subscription/meson.build b/src/test/subscription/meson.build index e71e95c6297eb..268fa8c3e9c13 100644 --- a/src/test/subscription/meson.build +++ b/src/test/subscription/meson.build @@ -48,6 +48,7 @@ tests += { 't/036_sequences.pl', 't/037_except.pl', 't/038_walsnd_shutdown_timeout.pl', + 't/039_publisher_xact_rollback.pl', 't/100_bugs.pl', ], }, diff --git a/src/test/subscription/t/039_publisher_xact_rollback.pl b/src/test/subscription/t/039_publisher_xact_rollback.pl new file mode 100644 index 0000000000000..b7dc5b20d6176 --- /dev/null +++ b/src/test/subscription/t/039_publisher_xact_rollback.pl @@ -0,0 +1,71 @@ + +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Check that pg_stat_database.xact_rollback on a logical-replication +# publisher is not inflated by the walsender's internal catalog-cleanup +# aborts. ReorderBufferProcessTXN() ends each decoded transaction with +# AbortCurrentTransaction(); in the walsender that is a top-level abort +# whose counter increment flushes to shared stats on walsender exit. +use strict; +use warnings FATAL => 'all'; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +my $node_publisher = PostgreSQL::Test::Cluster->new('publisher'); +$node_publisher->init(allows_streaming => 'logical'); +$node_publisher->start; + +my $node_subscriber = PostgreSQL::Test::Cluster->new('subscriber'); +$node_subscriber->init; +$node_subscriber->start; + +$node_publisher->safe_psql('postgres', + 'CREATE TABLE t (id int PRIMARY KEY)'); +$node_subscriber->safe_psql('postgres', + 'CREATE TABLE t (id int PRIMARY KEY)'); + +$node_publisher->safe_psql('postgres', 'CREATE PUBLICATION p FOR TABLE t'); + +my $publisher_connstr = $node_publisher->connstr . ' dbname=postgres'; +$node_subscriber->safe_psql('postgres', + "CREATE SUBSCRIPTION s CONNECTION '$publisher_connstr' PUBLICATION p"); + +$node_subscriber->wait_for_subscription_sync($node_publisher, 's'); + +# Use a baseline-delta rather than pg_stat_reset() to tolerate ambient +# rollback activity. +my $base = $node_publisher->safe_psql('postgres', + "SELECT xact_rollback FROM pg_stat_database WHERE datname = 'postgres'"); +chomp $base; + +# Five autocommit INSERTs: each becomes one decoded committed txn on the +# walsender. Without the fix, that's five spurious rollbacks after DISABLE. +my $n = 5; +$node_publisher->safe_psql('postgres', + join('', map { "INSERT INTO t VALUES ($_);\n" } 1 .. $n)); + +$node_publisher->wait_for_catchup('s'); + +# Disabling the subscription terminates the walsender; its shutdown hook +# flushes pgstat counters to shared stats. +$node_subscriber->safe_psql('postgres', 'ALTER SUBSCRIPTION s DISABLE'); + +# Wait for this subscription's walsender (filter by application_name). +$node_publisher->poll_query_until( + 'postgres', q{ + SELECT count(*) = 0 FROM pg_stat_activity + WHERE backend_type = 'walsender' AND application_name = 's' +}) + or die 's walsender did not exit'; + +my $final = $node_publisher->safe_psql('postgres', + "SELECT xact_rollback FROM pg_stat_database WHERE datname = 'postgres'"); +chomp $final; + +cmp_ok( + $final - $base, '==', 0, + 'walsender does not inflate publisher xact_rollback for decoded transactions' +); + +done_testing(); ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: xact_rollback spikes when logical walsender exits 2026-04-17 15:15 xact_rollback spikes when logical walsender exits Nikolay Samokhvalov <[email protected]> @ 2026-04-20 16:35 ` Fujii Masao <[email protected]> 1 sibling, 0 replies; 7+ messages in thread From: Fujii Masao @ 2026-04-20 16:35 UTC (permalink / raw) To: Nikolay Samokhvalov <[email protected]>; +Cc: pgsql-hackers <[email protected]>; Rafael Thofehrn Castro <[email protected]> On Sat, Apr 18, 2026 at 12:15 AM Nikolay Samokhvalov <[email protected]> wrote: > > Hi hackers, > > There is a bug on logical-replication publishers where every decoded > committed transaction bumps pg_stat_database.xact_rollback. > ReorderBufferProcessTXN() ends each decoded transaction with > AbortCurrentTransaction() for catalog cleanup; in the walsender that > is a top-level abort, so AtEOXact_PgStat_Database(isCommit=false) > increments the backend-local pgStatXactRollback. > > The counts are flushed to shared stats on walsender exit, producing > an acute spike. Result: for production systems with SREs on call and tight > alerting on xact_rollback, this turns routine logical-replication operations > (disabling a subscription, dropping a slot, walsender restart) into > false-positive pages. > > Reported in [1]; also experienced at GitLab [2][3][4]. > > Attaching a simple patch that adds a backend-local flag pgStatXactSkipCounters > in pgstat_database.c that AtEOXact_PgStat_Database() honors to skip > the counter bump. > > Added TAP test that fails on master with 5/0 and passes with the patch. > > If there is agreement on this shape, happy to send patches for all > supported branches. Let me know what you think. Thanks for the report and patch! How to implement a solution depends on what xact_rollback in pg_stat_database is intended to mean. So at first we should consider which rollbacks should it count? The documentation does not currently give an explicit definition. At present, xact_rollback appears to count all rollbacks, explicit or implicit, by any process connected to the database, including regular backends, autovacuum workers, and logical walsenders. If that is the intended definition, then rollbacks implicitly performed by logical walsenders during logical replication should also be counted. Of course, even if we keep that definition, the sudden increase in xact_rollback might still be a problem, so we might need to call pgstat_report_stat() immediately after pgstat_flush_io() in walsender, so the counters continue to be updated periodically during logical replication. On the other hand, your patch seems to assume a different definition: that xact_rollback should count all explicit and implicit rollbacks, except those performed by logical walsenders during logical replication. That would be one possible approach, although it seems a bit odd to exclude only one subset of rollbacks. A third option would be to define xact_rollback more narrowly, counting only rollbacks by regular backends, and excluding rollbacks by processes such as autovacuum or walsender. At least in my view, xact_commit and xact_rollback in pg_stat_database are typically used by DBAs to check whether client transactions are committing or rolling back as expected. From that perspective, it seems intuitive for xact_rollback to count only rollbacks by regular backends. But others may reasonably see it differently. Regards, -- Fujii Masao ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: xact_rollback spikes when logical walsender exits 2026-04-17 15:15 xact_rollback spikes when logical walsender exits Nikolay Samokhvalov <[email protected]> @ 2026-04-21 06:38 ` vignesh C <[email protected]> 2026-05-13 00:06 ` Re: xact_rollback spikes when logical walsender exits Nikolay Samokhvalov <[email protected]> 1 sibling, 1 reply; 7+ messages in thread From: vignesh C @ 2026-04-21 06:38 UTC (permalink / raw) To: Nikolay Samokhvalov <[email protected]>; +Cc: pgsql-hackers <[email protected]>; Rafael Thofehrn Castro <[email protected]> On Fri, 17 Apr 2026 at 20:45, Nikolay Samokhvalov <[email protected]> wrote: > > Hi hackers, > > There is a bug on logical-replication publishers where every decoded > committed transaction bumps pg_stat_database.xact_rollback. > ReorderBufferProcessTXN() ends each decoded transaction with > AbortCurrentTransaction() for catalog cleanup; in the walsender that > is a top-level abort, so AtEOXact_PgStat_Database(isCommit=false) > increments the backend-local pgStatXactRollback. > > The counts are flushed to shared stats on walsender exit, producing > an acute spike. Result: for production systems with SREs on call and tight > alerting on xact_rollback, this turns routine logical-replication operations > (disabling a subscription, dropping a slot, walsender restart) into > false-positive pages. > > Reported in [1]; also experienced at GitLab [2][3][4]. > > Attaching a simple patch that adds a backend-local flag pgStatXactSkipCounters > in pgstat_database.c that AtEOXact_PgStat_Database() honors to skip > the counter bump. > > Added TAP test that fails on master with 5/0 and passes with the patch. > > If there is agreement on this shape, happy to send patches for all > supported branches. Let me know what you think. Thanks for reporting this and for the patch the problem description matches what I've observed as well. The current behavior could be misleading, since these rollbacks correspond to internal decoding cleanup rather than actual user visible transaction aborts. Another approach could be to introduce a wrapper around AbortCurrentTransaction(), for example AbortCurrentTransactionWithoutUpdateStats(), that skips the AtEOXact_PgStat() call in this case. Thoughts? Regards, Vignesh ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: xact_rollback spikes when logical walsender exits 2026-04-17 15:15 xact_rollback spikes when logical walsender exits Nikolay Samokhvalov <[email protected]> 2026-04-21 06:38 ` Re: xact_rollback spikes when logical walsender exits vignesh C <[email protected]> @ 2026-05-13 00:06 ` Nikolay Samokhvalov <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Nikolay Samokhvalov @ 2026-05-13 00:06 UTC (permalink / raw) To: vignesh C <[email protected]>; +Cc: pgsql-hackers <[email protected]>; Rafael Thofehrn Castro <[email protected]> On Mon, Apr 20, 2026 at 11:38 PM vignesh C <[email protected]> wrote: > Another approach could be to introduce a wrapper around > AbortCurrentTransaction(), for example > AbortCurrentTransactionWithoutUpdateStats(), that skips the > AtEOXact_PgStat() call in this case. > Thoughts? Thanks -- v2 attached, adopting your wrapper. One scope choice worth flagging: the wrapper suppresses only AtEOXact_PgStat_Database() (the DB-level xact_commit/xact_rollback counter), not all of AtEOXact_PgStat(). Per-relation and subxact stat handling still run, so nothing accumulated during the cleanup is lost. Renamed to AbortCurrentTransactionWithoutXactStats() to match. Nik Attachments: [application/octet-stream] v2-logical-rollback-spike.patch (12.3K, ../../CAM527d87FpMWfDbCfJF9EF9uOXMcX87+bafxjHA_dGydH_RB9A@mail.gmail.com/3-v2-logical-rollback-spike.patch) download | inline diff: From 62b52d0011f08d472f3d60b67110453b07f1017d Mon Sep 17 00:00:00 2001 From: Nikolay Samokhvalov <[email protected]> Date: Thu, 16 Apr 2026 19:25:55 -0700 Subject: [PATCH] Do not count logical decoding cleanup aborts in xact_rollback ReorderBufferProcessTXN() aborts the current transaction after each decoded commit to release locks and clean up catalog access. In a logical walsender that is a top-level abort, so every decoded commit bumps pg_stat_database.xact_rollback; the counts surface as a spike on walsender exit (e.g. when a subscription is disabled). Add AbortCurrentTransactionWithoutXactStats(), a wrapper that suppresses only the DB-level xact_commit/xact_rollback counter and leaves per-relation and subxact stat handling intact. Use it from the top-level cleanup paths in ReorderBufferProcessTXN() (gated on !using_subtxn) and SnapBuildClearExportedSnapshot(). A TAP test asserts a publisher xact_rollback delta of 0 across five decoded transactions (delta is 5 without the fix). Reported-by: Rafael Thofehrn Castro Discussion: https://postgr.es/m/CAM527d_EbU5Li4a5FdKQjYsdF-4Lqr_i3jXmZOm7Wbb%3DQ2KzTw%40mail.gmail.com --- src/backend/access/transam/twophase.c | 2 +- src/backend/access/transam/xact.c | 39 ++++++++++- .../replication/logical/reorderbuffer.c | 16 +++-- src/backend/replication/logical/snapbuild.c | 9 ++- src/backend/utils/activity/pgstat_xact.c | 11 ++- src/include/access/xact.h | 1 + src/include/pgstat.h | 2 +- src/test/subscription/meson.build | 1 + .../t/039_publisher_xact_rollback.pl | 70 +++++++++++++++++++ 9 files changed, 139 insertions(+), 12 deletions(-) create mode 100644 src/test/subscription/t/039_publisher_xact_rollback.pl diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index 1035e8b3fc7..3aa31e7f805 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -1676,7 +1676,7 @@ FinishPreparedTransaction(const char *gid, bool isCommit) LWLockRelease(TwoPhaseStateLock); /* Count the prepared xact as committed or aborted */ - AtEOXact_PgStat(isCommit, false); + AtEOXact_PgStat(isCommit, false, true); /* * And now we can clean up any files we may have left. diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 48bc90c9673..35d5e174011 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -316,6 +316,15 @@ typedef struct XactCallbackItem static XactCallbackItem *Xact_callbacks = NULL; +/* + * When true, suppress the pg_stat_database xact_commit/xact_rollback bump + * for the current transaction end. Must only be set via + * AbortCurrentTransactionWithoutXactStats(); assertions in + * StartTransaction() and in the wrapper itself guard against the flag + * leaking across transactions. + */ +static bool xactSkipXactStats = false; + /* * List of add-on start- and end-of-subxact callbacks */ @@ -2118,6 +2127,7 @@ StartTransaction(void) /* check the current transaction state */ Assert(s->state == TRANS_DEFAULT); + Assert(!xactSkipXactStats); /* * Set the current transaction state information appropriately during @@ -2514,7 +2524,7 @@ CommitTransaction(void) AtEOXact_Files(true); AtEOXact_ComboCid(); AtEOXact_HashTables(true); - AtEOXact_PgStat(true, is_parallel_worker); + AtEOXact_PgStat(true, is_parallel_worker, true); AtEOXact_Snapshot(true, false); AtEOXact_ApplyLauncher(true); AtEOXact_LogicalRepWorkers(true); @@ -3039,7 +3049,7 @@ AbortTransaction(void) AtEOXact_Files(false); AtEOXact_ComboCid(); AtEOXact_HashTables(false); - AtEOXact_PgStat(false, is_parallel_worker); + AtEOXact_PgStat(false, is_parallel_worker, !xactSkipXactStats); AtEOXact_ApplyLauncher(false); AtEOXact_LogicalRepWorkers(false); AtEOXact_LogicalCtl(); @@ -3509,6 +3519,31 @@ AbortCurrentTransaction(void) } } +/* + * AbortCurrentTransactionWithoutXactStats + * + * Like AbortCurrentTransaction(), but do not count the transaction abort in + * pg_stat_database.xact_rollback. This is for internal cleanup aborts that + * release transaction-local resources but do not represent a user-visible + * transaction rollback. + */ +void +AbortCurrentTransactionWithoutXactStats(void) +{ + Assert(!xactSkipXactStats); + + xactSkipXactStats = true; + PG_TRY(); + { + AbortCurrentTransaction(); + } + PG_FINALLY(); + { + xactSkipXactStats = false; + } + PG_END_TRY(); +} + /* * AbortCurrentTransactionInternal - a function doing an iteration of work * regarding handling the current transaction abort. In the case of diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c index 682d13c9f22..f953676bfe1 100644 --- a/src/backend/replication/logical/reorderbuffer.c +++ b/src/backend/replication/logical/reorderbuffer.c @@ -2674,9 +2674,13 @@ ReorderBufferProcessTXN(ReorderBuffer *rb, ReorderBufferTXN *txn, * Aborting the current (sub-)transaction as a whole has the right * semantics. We want all locks acquired in here to be released, not * reassigned to the parent and we do not want any database access - * have persistent effects. + * have persistent effects. In the !using_subtxn case this is a + * top-level abort; keep it out of pg_stat_database.xact_rollback. */ - AbortCurrentTransaction(); + if (using_subtxn) + AbortCurrentTransaction(); + else + AbortCurrentTransactionWithoutXactStats(); /* make sure there's no cache pollution */ if (rbtxn_distr_inval_overflowed(txn)) @@ -2737,9 +2741,13 @@ ReorderBufferProcessTXN(ReorderBuffer *rb, ReorderBufferTXN *txn, /* * Force cache invalidation to happen outside of a valid transaction - * to prevent catalog access as we just caught an error. + * to prevent catalog access as we just caught an error. As above, + * keep the top-level abort out of pg_stat_database.xact_rollback. */ - AbortCurrentTransaction(); + if (using_subtxn) + AbortCurrentTransaction(); + else + AbortCurrentTransactionWithoutXactStats(); /* make sure there's no cache pollution */ if (rbtxn_distr_inval_overflowed(txn)) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index c8309b96ed4..ba9af88c505 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -631,8 +631,13 @@ SnapBuildClearExportedSnapshot(void) */ tmpResOwner = SavedResourceOwnerDuringExport; - /* make sure nothing could have ever happened */ - AbortCurrentTransaction(); + /* + * Make sure nothing could have ever happened. Keep this cleanup abort + * out of pg_stat_database.xact_rollback; we must be at top level so + * the abort reaches AtEOXact_PgStat_Database. + */ + Assert(!IsSubTransaction()); + AbortCurrentTransactionWithoutXactStats(); CurrentResourceOwner = tmpResOwner; } diff --git a/src/backend/utils/activity/pgstat_xact.c b/src/backend/utils/activity/pgstat_xact.c index 5e2d69e6297..ea9f703c088 100644 --- a/src/backend/utils/activity/pgstat_xact.c +++ b/src/backend/utils/activity/pgstat_xact.c @@ -37,11 +37,18 @@ static PgStat_SubXactStatus *pgStatXactStack = NULL; * Called from access/transam/xact.c at top-level transaction commit/abort. */ void -AtEOXact_PgStat(bool isCommit, bool parallel) +AtEOXact_PgStat(bool isCommit, bool parallel, bool count_xact_stats) { PgStat_SubXactStatus *xact_state; - AtEOXact_PgStat_Database(isCommit, parallel); + /* + * Only the database-level xact_commit/xact_rollback counter is gated + * here. Per-relation and subxact stat handling below must still run + * unconditionally so any stats accumulated during the transaction are + * not lost. + */ + if (count_xact_stats) + AtEOXact_PgStat_Database(isCommit, parallel); /* handle transactional stats information */ xact_state = pgStatXactStack; diff --git a/src/include/access/xact.h b/src/include/access/xact.h index a8cbdf247c8..a5ec9a027d4 100644 --- a/src/include/access/xact.h +++ b/src/include/access/xact.h @@ -467,6 +467,7 @@ extern void SaveTransactionCharacteristics(SavedTransactionCharacteristics *s); extern void RestoreTransactionCharacteristics(const SavedTransactionCharacteristics *s); extern void CommitTransactionCommand(void); extern void AbortCurrentTransaction(void); +extern void AbortCurrentTransactionWithoutXactStats(void); extern void BeginTransactionBlock(void); extern bool EndTransactionBlock(bool chain); extern bool PrepareTransactionBlock(const char *gid); diff --git a/src/include/pgstat.h b/src/include/pgstat.h index dfa2e837638..8509a590bbf 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -814,7 +814,7 @@ extern PgStat_StatSubEntry *pgstat_fetch_stat_subscription(Oid subid); * Functions in pgstat_xact.c */ -extern void AtEOXact_PgStat(bool isCommit, bool parallel); +extern void AtEOXact_PgStat(bool isCommit, bool parallel, bool count_xact_stats); extern void AtEOSubXact_PgStat(bool isCommit, int nestDepth); extern void AtPrepare_PgStat(void); extern void PostPrepare_PgStat(void); diff --git a/src/test/subscription/meson.build b/src/test/subscription/meson.build index e71e95c6297..268fa8c3e9c 100644 --- a/src/test/subscription/meson.build +++ b/src/test/subscription/meson.build @@ -48,6 +48,7 @@ tests += { 't/036_sequences.pl', 't/037_except.pl', 't/038_walsnd_shutdown_timeout.pl', + 't/039_publisher_xact_rollback.pl', 't/100_bugs.pl', ], }, diff --git a/src/test/subscription/t/039_publisher_xact_rollback.pl b/src/test/subscription/t/039_publisher_xact_rollback.pl new file mode 100644 index 00000000000..1fa72a19712 --- /dev/null +++ b/src/test/subscription/t/039_publisher_xact_rollback.pl @@ -0,0 +1,70 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Check that pg_stat_database.xact_rollback on a logical-replication +# publisher is not inflated by the walsender's internal catalog-cleanup +# aborts. ReorderBufferProcessTXN() ends each decoded transaction with +# AbortCurrentTransaction(); in the walsender that is a top-level abort +# whose counter increment flushes to shared stats on walsender exit. +use strict; +use warnings FATAL => 'all'; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +my $node_publisher = PostgreSQL::Test::Cluster->new('publisher'); +$node_publisher->init(allows_streaming => 'logical'); +$node_publisher->start; + +my $node_subscriber = PostgreSQL::Test::Cluster->new('subscriber'); +$node_subscriber->init; +$node_subscriber->start; + +$node_publisher->safe_psql('postgres', + 'CREATE TABLE t (id int PRIMARY KEY)'); +$node_subscriber->safe_psql('postgres', + 'CREATE TABLE t (id int PRIMARY KEY)'); + +$node_publisher->safe_psql('postgres', 'CREATE PUBLICATION p FOR TABLE t'); + +my $publisher_connstr = $node_publisher->connstr . ' dbname=postgres'; +$node_subscriber->safe_psql('postgres', + "CREATE SUBSCRIPTION s CONNECTION '$publisher_connstr' PUBLICATION p"); + +$node_subscriber->wait_for_subscription_sync($node_publisher, 's'); + +# Use a baseline-delta rather than pg_stat_reset() to tolerate ambient +# rollback activity. +my $base = $node_publisher->safe_psql('postgres', + "SELECT xact_rollback FROM pg_stat_database WHERE datname = 'postgres'"); +chomp $base; + +# Five autocommit INSERTs: each becomes one decoded committed txn on the +# walsender. Without the fix, that's five spurious rollbacks after DISABLE. +my $n = 5; +$node_publisher->safe_psql('postgres', + join('', map { "INSERT INTO t VALUES ($_);\n" } 1 .. $n)); + +$node_publisher->wait_for_catchup('s'); + +# Disabling the subscription terminates the walsender; its shutdown hook +# flushes pgstat counters to shared stats. +$node_subscriber->safe_psql('postgres', 'ALTER SUBSCRIPTION s DISABLE'); + +# Wait for this subscription's walsender (filter by application_name). +$node_publisher->poll_query_until( + 'postgres', q{ + SELECT count(*) = 0 FROM pg_stat_activity + WHERE backend_type = 'walsender' AND application_name = 's' +}) + or die 's walsender did not exit'; + +my $final = $node_publisher->safe_psql('postgres', + "SELECT xact_rollback FROM pg_stat_database WHERE datname = 'postgres'"); +chomp $final; + +cmp_ok( + $final - $base, '==', 0, + 'walsender does not inflate publisher xact_rollback for decoded transactions' +); + +done_testing(); -- 2.50.1 (Apple Git-155) ^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2026-05-13 00:06 UTC | newest] Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-01-05 08:20 Re: Preferred use of macro GetPGProcByNumber David Rowley <[email protected]> 2026-03-25 19:35 [PATCH v44b 4/4] hack deadlock detector Álvaro Herrera <[email protected]> 2026-03-25 19:35 [PATCH v44b 4/4] hack deadlock detector Álvaro Herrera <[email protected]> 2026-04-17 15:15 xact_rollback spikes when logical walsender exits Nikolay Samokhvalov <[email protected]> 2026-04-20 16:35 ` Re: xact_rollback spikes when logical walsender exits Fujii Masao <[email protected]> 2026-04-21 06:38 ` Re: xact_rollback spikes when logical walsender exits vignesh C <[email protected]> 2026-05-13 00:06 ` Re: xact_rollback spikes when logical walsender exits Nikolay Samokhvalov <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox