public inbox for [email protected]
help / color / mirror / Atom feedRe: Avoid possible overflow (src/port/bsearch_arg.c)
10+ messages / 7 participants
[nested] [flat]
* Re: Avoid possible overflow (src/port/bsearch_arg.c)
@ 2024-10-09 16:16 Ranier Vilela <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Ranier Vilela @ 2024-10-09 16:16 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: pgsql-hackers
Em ter., 8 de out. de 2024 às 18:28, Nathan Bossart <
[email protected]> escreveu:
> On Tue, Oct 08, 2024 at 04:09:00PM -0300, Ranier Vilela wrote:
> > The port function *bsearch_arg* mimics the C function
> > *bsearch*.
> >
> > The API signature is:
> > void *
> > bsearch_arg(const void *key, const void *base0,
> > size_t nmemb, size_t size,
> > int (*compar) (const void *, const void *, void *),
> > void *arg)
> >
> > So, the parameter *nmemb* is size_t.
> > Therefore, a call with nmemb greater than INT_MAX is possible.
> >
> > Internally the code uses the *int* type to iterate through the number of
> > members, which makes overflow possible.
>
> I traced this back to commit bfa2cee (v14), which both moved bsearch_arg()
> to its current location and adjusted the style a bit. Your patch looks
> reasonable to me.
>
Thanks for looking.
best regards,
Ranier Vilela
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Avoid possible overflow (src/port/bsearch_arg.c)
@ 2024-10-28 12:13 Heikki Linnakangas <[email protected]>
parent: Ranier Vilela <[email protected]>
0 siblings, 2 replies; 10+ messages in thread
From: Heikki Linnakangas @ 2024-10-28 12:13 UTC (permalink / raw)
To: Ranier Vilela <[email protected]>; Nathan Bossart <[email protected]>; Tomas Vondra <[email protected]>; +Cc: pgsql-hackers
On 09/10/2024 19:16, Ranier Vilela wrote:
> Em ter., 8 de out. de 2024 às 18:28, Nathan Bossart
> <[email protected] <mailto:[email protected]>> escreveu:
>
> On Tue, Oct 08, 2024 at 04:09:00PM -0300, Ranier Vilela wrote:
> > The port function *bsearch_arg* mimics the C function
> > *bsearch*.
> >
> > The API signature is:
> > void *
> > bsearch_arg(const void *key, const void *base0,
> > size_t nmemb, size_t size,
> > int (*compar) (const void *, const void *, void *),
> > void *arg)
> >
> > So, the parameter *nmemb* is size_t.
> > Therefore, a call with nmemb greater than INT_MAX is possible.
> >
> > Internally the code uses the *int* type to iterate through the
> number of
> > members, which makes overflow possible.
>
> I traced this back to commit bfa2cee (v14), which both moved
> bsearch_arg()
> to its current location and adjusted the style a bit. Your patch looks
> reasonable to me.
>
> Thanks for looking.
Committed, thanks.
Based on the original discussion on bfa2cee, I couldn't figure out where
exactly this new bsearch implementation originated from, but googling
around, probably *BSD or libiberty. Tomas, do you remember? Not that it
matters, but I'm curious.
Some of those other implementations have fixed this, others have not.
And they all seem to also have the "involes" typo in the comment that we
fixed in commit 7ef8b52cf07 :-). Ranier, you might want to submit this
fix to those other projects too.
--
Heikki Linnakangas
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Avoid possible overflow (src/port/bsearch_arg.c)
@ 2024-10-28 12:33 Ranier Vilela <[email protected]>
parent: Heikki Linnakangas <[email protected]>
1 sibling, 0 replies; 10+ messages in thread
From: Ranier Vilela @ 2024-10-28 12:33 UTC (permalink / raw)
To: Heikki Linnakangas <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Tomas Vondra <[email protected]>; pgsql-hackers
Em seg., 28 de out. de 2024 às 09:13, Heikki Linnakangas <[email protected]>
escreveu:
> On 09/10/2024 19:16, Ranier Vilela wrote:
> > Em ter., 8 de out. de 2024 às 18:28, Nathan Bossart
> > <[email protected] <mailto:[email protected]>> escreveu:
> >
> > On Tue, Oct 08, 2024 at 04:09:00PM -0300, Ranier Vilela wrote:
> > > The port function *bsearch_arg* mimics the C function
> > > *bsearch*.
> > >
> > > The API signature is:
> > > void *
> > > bsearch_arg(const void *key, const void *base0,
> > > size_t nmemb, size_t size,
> > > int (*compar) (const void *, const void *, void *),
> > > void *arg)
> > >
> > > So, the parameter *nmemb* is size_t.
> > > Therefore, a call with nmemb greater than INT_MAX is possible.
> > >
> > > Internally the code uses the *int* type to iterate through the
> > number of
> > > members, which makes overflow possible.
> >
> > I traced this back to commit bfa2cee (v14), which both moved
> > bsearch_arg()
> > to its current location and adjusted the style a bit. Your patch
> looks
> > reasonable to me.
> >
> > Thanks for looking.
>
> Committed, thanks.
>
Thank you.
>
> Based on the original discussion on bfa2cee, I couldn't figure out where
> exactly this new bsearch implementation originated from, but googling
> around, probably *BSD or libiberty. Tomas, do you remember? Not that it
> matters, but I'm curious.
>
> Some of those other implementations have fixed this, others have not.
> And they all seem to also have the "involes" typo in the comment that we
> fixed in commit 7ef8b52cf07 :-). Ranier, you might want to submit this
> fix to those other projects too.
>
Sure, I can try.
best regards,
Ranier Vilela
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Avoid possible overflow (src/port/bsearch_arg.c)
@ 2024-10-28 15:30 Tomas Vondra <[email protected]>
parent: Heikki Linnakangas <[email protected]>
1 sibling, 1 reply; 10+ messages in thread
From: Tomas Vondra @ 2024-10-28 15:30 UTC (permalink / raw)
To: Heikki Linnakangas <[email protected]>; Ranier Vilela <[email protected]>; Nathan Bossart <[email protected]>; Tomas Vondra <[email protected]>; +Cc: pgsql-hackers
On 10/28/24 13:13, Heikki Linnakangas wrote:
> On 09/10/2024 19:16, Ranier Vilela wrote:
>> Em ter., 8 de out. de 2024 às 18:28, Nathan Bossart
>> <[email protected] <mailto:[email protected]>> escreveu:
>>
>> On Tue, Oct 08, 2024 at 04:09:00PM -0300, Ranier Vilela wrote:
>> > The port function *bsearch_arg* mimics the C function
>> > *bsearch*.
>> >
>> > The API signature is:
>> > void *
>> > bsearch_arg(const void *key, const void *base0,
>> > size_t nmemb, size_t size,
>> > int (*compar) (const void *, const void *, void *),
>> > void *arg)
>> >
>> > So, the parameter *nmemb* is size_t.
>> > Therefore, a call with nmemb greater than INT_MAX is possible.
>> >
>> > Internally the code uses the *int* type to iterate through the
>> number of
>> > members, which makes overflow possible.
>>
>> I traced this back to commit bfa2cee (v14), which both moved
>> bsearch_arg()
>> to its current location and adjusted the style a bit. Your patch
>> looks
>> reasonable to me.
>>
>> Thanks for looking.
>
> Committed, thanks.
>
> Based on the original discussion on bfa2cee, I couldn't figure out where
> exactly this new bsearch implementation originated from, but googling
> around, probably *BSD or libiberty. Tomas, do you remember? Not that it
> matters, but I'm curious.
>
I don't remember, unfortunately :-( I think it was one of the *BSDs,
because of license, but I'm not quite sure why I changed the code at all
during the move.
> Some of those other implementations have fixed this, others have not.
> And they all seem to also have the "involes" typo in the comment that we
> fixed in commit 7ef8b52cf07 :-). Ranier, you might want to submit this
> fix to those other projects too.
>
Thanks for fixing this, although I wonder if we can actually hit this,
as we don't really allocate more than 1GB in most places. But it's
possible, and the pre-bfa2cee code handled it fine.
regards
--
Tomas Vondra
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Avoid possible overflow (src/port/bsearch_arg.c)
@ 2024-10-28 16:52 Heikki Linnakangas <[email protected]>
parent: Tomas Vondra <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: Heikki Linnakangas @ 2024-10-28 16:52 UTC (permalink / raw)
To: Tomas Vondra <[email protected]>; Ranier Vilela <[email protected]>; Nathan Bossart <[email protected]>; Tomas Vondra <[email protected]>; +Cc: pgsql-hackers
On 28/10/2024 17:30, Tomas Vondra wrote:
>> Some of those other implementations have fixed this, others have not.
>> And they all seem to also have the "involes" typo in the comment that we
>> fixed in commit 7ef8b52cf07 :-). Ranier, you might want to submit this
>> fix to those other projects too.
>
> Thanks for fixing this, although I wonder if we can actually hit this,
> as we don't really allocate more than 1GB in most places. But it's
> possible, and the pre-bfa2cee code handled it fine.
Yeah, I didn't check closely I'm pretty sure none of the current
callsites can pass anything near INT_MAX elements.
While we're at it, there's this in dicts/spell.h:
> /*
> * Structure to store Hunspell options. Flag representation depends on flag
> * type. These flags are about support of compound words.
> */
> typedef struct CompoundAffixFlag
> {
> union
> {
> /* Flag name if flagMode is FM_CHAR or FM_LONG */
> const char *s;
> /* Flag name if flagMode is FM_NUM */
> uint32 i;
> } flag;
> /* we don't have a bsearch_arg version, so, copy FlagMode */
> FlagMode flagMode;
> uint32 value;
> } CompoundAffixFlag;
We have bsearch_arg() now, so we could switch to that and remove
'flagMode' from here.
--
Heikki Linnakangas
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 10+ messages in thread
* sequencesync worker race with REFRESH SEQUENCES
@ 2026-07-10 04:52 Noah Misch <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Noah Misch @ 2026-07-10 04:52 UTC (permalink / raw)
To: [email protected]; [email protected]; +Cc: pgsql-hackers
A Fable 5 review of logical replication of sequences found a way to get
subscribed sequences into READY state despite the subscriber side having data
older than the last REFRESH SEQUENCES. I'm attaching the test case it wrote.
I reviewed the test, and I think it identifies a genuine defect.
Fable 5 also wrote a lot more that neither it nor I confirmed by test case
construction. I'm attaching the report; feel free to disregard. Finding-2
about default_transaction_read_only=on looks worth fixing if true, as do
finding-7 assert failure and finding-17 about documenting a PG19+ publisher
requirement. Finding-6, publisher version guard, I'd NOT do or do at warning
level if an older publisher could plausibly work via an extension to provide
the needed function. The other findings are gray areas, perhaps.
commit fe331b9 (HEAD -> seq-refresh-sequences-race)
Author: Noah Misch <[email protected]>
AuthorDate: Thu Jul 9 19:39:45 2026 +0000
Commit: Noah Misch <[email protected]>
CommitDate: Thu Jul 9 19:42:17 2026 +0000
Add test revealing REFRESH SEQUENCES race with sequencesync worker.
ALTER SUBSCRIPTION ... REFRESH SEQUENCES resets all sequence entries in
pg_subscription_rel to INIT (AlterSubscription_refresh_seq) without
stopping or fencing an in-flight sequencesync worker. The worker
fetches publisher values before taking any local lock, and
copy_sequence() later applies them and marks each sequence READY
without rechecking the entry's state. A batch whose values were
fetched before the refresh therefore overwrites the INIT reset
afterwards: the sequences report READY while holding pre-refresh
publisher values, with no error, no warning, and no pending
resynchronization. A user following the documented pre-failover
procedure (REFRESH SEQUENCES, wait for READY) can then fail over to a
subscriber whose sequences are behind the publisher, yielding duplicate
sequence values.
The test constructs the schedule deterministically: it blocks the
worker's batch query on the publisher via AccessExclusiveLock on a
sequence, blocks local application via ShareRowExclusiveLock on the
sequences on the subscriber, and runs REFRESH SEQUENCES inside the
fetch-to-apply window.
The final two assertions check that sequences reporting READY reflect
every value published before REFRESH SEQUENCES was issued. They
currently fail, demonstrating the defect.
Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01YKt6fYFvAnMUMyavJmmueE
---
src/test/subscription/meson.build | 1 +
.../subscription/t/039_sequences_refresh_race.pl | 184 +++++++++++++++++++++
2 files changed, 185 insertions(+)
diff --git a/src/test/subscription/meson.build b/src/test/subscription/meson.build
index e71e95c..067fca2 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_sequences_refresh_race.pl',
't/100_bugs.pl',
],
},
diff --git a/src/test/subscription/t/039_sequences_refresh_race.pl b/src/test/subscription/t/039_sequences_refresh_race.pl
new file mode 100644
index 0000000..5173231
--- /dev/null
+++ b/src/test/subscription/t/039_sequences_refresh_race.pl
@@ -0,0 +1,184 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+# Test that ALTER SUBSCRIPTION ... REFRESH SEQUENCES is not silently
+# overwritten by an in-flight sequencesync worker.
+#
+# The sequencesync worker fetches sequence values from the publisher and
+# only afterwards applies them and marks the sequences READY, without
+# rechecking pg_subscription_rel state. REFRESH SEQUENCES resets all
+# sequences to INIT without stopping or fencing a running worker. Hence a
+# batch whose values were fetched before the refresh can mark sequences
+# READY afterwards, leaving them holding pre-refresh values with no error,
+# no warning, and no pending resynchronization. A user following the
+# documented pre-failover procedure (run REFRESH SEQUENCES, wait for READY)
+# can then fail over to a subscriber whose sequences are behind the
+# publisher, producing duplicate sequence values.
+#
+# This test constructs that schedule deterministically:
+#
+# 1. Hold AccessExclusiveLock on a sequence on the publisher, so the
+# worker's remote batch query blocks inside pg_get_sequence_data()
+# after the worker has committed its scan of pg_subscription_rel.
+# 2. While the fetch is blocked, take ShareRowExclusiveLock on all
+# sequences on the subscriber, so that after the fetch completes the
+# worker blocks at its first try_table_open(), i.e. after fetching
+# values but before updating any pg_subscription_rel row.
+# 3. Release the publisher lock; the worker's fetch completes with the old
+# values and the worker blocks on the subscriber locks.
+# 4. Advance the sequences on the publisher, then run
+# ALTER SUBSCRIPTION ... REFRESH SEQUENCES; it resets both sequences to
+# INIT and commits, not blocked by the in-flight worker.
+# 5. Release the subscriber locks. The worker applies the stale values
+# and flips both sequences INIT -> READY.
+#
+# On an unpatched server the final assertions fail: all sequences report
+# READY but hold the values fetched before the refresh.
+#
+# Note for whoever fixes the race: this choreography assumes REFRESH
+# SEQUENCES continues to complete without waiting for an in-flight
+# sequencesync worker (as it does today). If the fix instead makes the
+# command block until a running worker exits, step 4 will wait behind the
+# sequence locks taken in step 2 and the test will hang there rather than
+# fail; the schedule would need reshaping for a fix of that shape.
+
+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;
+
+# Identical sequence definitions on both nodes.
+my $ddl = qq(
+ CREATE SEQUENCE regress_seq1;
+ CREATE SEQUENCE regress_seq2;
+);
+$node_publisher->safe_psql('postgres', $ddl);
+$node_subscriber->safe_psql('postgres', $ddl);
+
+# Consume some sequence values on the publisher.
+$node_publisher->safe_psql(
+ 'postgres', qq(
+ SELECT nextval('regress_seq1') FROM generate_series(1, 100);
+ SELECT nextval('regress_seq2') FROM generate_series(1, 100);
+));
+
+$node_publisher->safe_psql('postgres',
+ "CREATE PUBLICATION regress_seq_pub FOR ALL SEQUENCES");
+
+# Create the subscription disabled, so the locks below can be positioned
+# before the sequencesync worker starts. The replication slot is created
+# here, which must precede the publisher-side open transaction below
+# because slot creation waits for concurrent transactions to finish.
+my $publisher_connstr = $node_publisher->connstr . ' dbname=postgres';
+$node_subscriber->safe_psql('postgres',
+ "CREATE SUBSCRIPTION regress_seq_sub CONNECTION '$publisher_connstr' PUBLICATION regress_seq_pub WITH (enabled = false)"
+);
+
+my $result = $node_subscriber->safe_psql('postgres',
+ "SELECT count(*) FROM pg_subscription_rel WHERE srsubstate = 'i'");
+is($result, '2', 'both sequences start in INIT state');
+
+# Block the sequencesync worker's batch query on the publisher: an
+# uncommitted DROP SEQUENCE holds AccessExclusiveLock, on which the
+# pg_get_sequence_data() call in the batch query will wait.
+my $pub_session = $node_publisher->background_psql('postgres');
+$pub_session->query_safe(
+ qq(
+ BEGIN;
+ DROP SEQUENCE regress_seq1;
+));
+
+$node_subscriber->safe_psql('postgres',
+ "ALTER SUBSCRIPTION regress_seq_sub ENABLE");
+
+# Wait until the worker's batch query is blocked on the publisher. At
+# this point the worker has committed the transaction that scanned
+# pg_subscription_rel and holds no locks on the subscriber.
+$node_publisher->poll_query_until(
+ 'postgres', qq(
+ SELECT EXISTS (
+ SELECT 1 FROM pg_locks
+ WHERE relation = 'regress_seq1'::regclass AND NOT granted);
+)) or die "timed out waiting for sequencesync worker to block on publisher";
+
+# Take locks conflicting with the worker's try_table_open() on both
+# sequences, so the worker will block after its fetch completes but before
+# it updates any pg_subscription_rel row, whatever order it processes the
+# sequences in. The ALTERs are rolled back later, so the definitions stay
+# identical to the publisher's.
+my $sub_session = $node_subscriber->background_psql('postgres');
+$sub_session->query_safe(
+ qq(
+ BEGIN;
+ ALTER SEQUENCE regress_seq1 MINVALUE 1;
+ ALTER SEQUENCE regress_seq2 MINVALUE 1;
+));
+
+# Release the publisher lock: the fetch completes with the current
+# publisher values, then the worker blocks on the subscriber locks.
+$pub_session->query_safe("ROLLBACK");
+$pub_session->quit;
+
+$node_subscriber->poll_query_until(
+ 'postgres', qq(
+ SELECT EXISTS (
+ SELECT 1 FROM pg_locks
+ WHERE relation IN ('regress_seq1'::regclass, 'regress_seq2'::regclass)
+ AND NOT granted);
+)) or die "timed out waiting for sequencesync worker to block on subscriber";
+
+# The values held by the blocked worker now become stale.
+$node_publisher->safe_psql(
+ 'postgres', qq(
+ SELECT nextval('regress_seq1') FROM generate_series(1, 100);
+ SELECT nextval('regress_seq2') FROM generate_series(1, 100);
+));
+
+# Request resynchronization of all sequences. This resets both sequences
+# to INIT and commits; it does not wait for the in-flight worker.
+$node_subscriber->safe_psql('postgres',
+ "ALTER SUBSCRIPTION regress_seq_sub REFRESH SEQUENCES");
+
+$result = $node_subscriber->safe_psql('postgres',
+ "SELECT count(*) FROM pg_subscription_rel WHERE srsubstate = 'i'");
+is($result, '2', 'REFRESH SEQUENCES reset both sequences to INIT');
+
+# Release the subscriber locks, letting the worker proceed.
+$sub_session->query_safe("ROLLBACK");
+$sub_session->quit;
+
+# Wait until the subscription reports all sequences synchronized.
+$node_subscriber->poll_query_until('postgres',
+ "SELECT count(*) = 0 FROM pg_subscription_rel WHERE srsubstate <> 'r'")
+ or die "timed out waiting for sequences to reach READY state";
+
+# All sequences report READY, so they must not predate the last REFRESH
+# SEQUENCES command: every value published before that command was issued
+# must be reflected on the subscriber. On an unpatched server these fail,
+# with the subscriber sequences left at the values fetched before the
+# refresh.
+my $pub_seq1 = $node_publisher->safe_psql('postgres',
+ "SELECT last_value, is_called FROM regress_seq1");
+my $sub_seq1 = $node_subscriber->safe_psql('postgres',
+ "SELECT last_value, is_called FROM regress_seq1");
+is($sub_seq1, $pub_seq1,
+ 'READY regress_seq1 reflects publisher values from before REFRESH SEQUENCES'
+);
+
+my $pub_seq2 = $node_publisher->safe_psql('postgres',
+ "SELECT last_value, is_called FROM regress_seq2");
+my $sub_seq2 = $node_subscriber->safe_psql('postgres',
+ "SELECT last_value, is_called FROM regress_seq2");
+is($sub_seq2, $pub_seq2,
+ 'READY regress_seq2 reflects publisher values from before REFRESH SEQUENCES'
+);
+
+done_testing();
# Logical replication of sequences: user-visible defects in master
**Scope:** defects introduced by commits f0b3573c3, 5509055d6, 55cefadde (and still present through follow-up fixes), verified against master @ a8c2547. All file:line references are to that tree.
## Executive summary
An adversarial audit of the PG19 sequence-replication feature identified 20 user-visible defects; two pairs share a single root cause and are merged below, leaving 18 items. Every item is **CONFIRMED** — the mechanism was traced end-to-end in source by an independent verifier — but none has been executed as a runtime repro, so the repro sketches should be run before relying on exact symptoms; there are no PLAUSIBLE-grade items in this report. The dominant pattern is missing coordination around the new shared sequencesync worker: no ALTER SUBSCRIPTION path stops or fences it, and it never rechecks subscription or relation state after startup, yielding (worst case) sequences silently marked READY with pre-refresh publisher values in the documented pre-failover `REFRESH SEQUENCES` workflow — i.e., duplicate sequence values after failover, the exact hazard the feature exists to prevent. A second pattern is single-object failures escalating to subscription-wide outages: because one worker serves all sequences and several failure paths bypass the per-sequence warning framework, one misconfigured sequence, a read-only default, a lock-table squeeze, or an older publisher produces an unbounded 5-second error/relaunch loop (or full auto-disable under `disable_on_error`). The remainder are internal XX000 errors reachable from ordinary DDL concurrency, misleading diagnostics, and monitoring/documentation never updated for the new worker type and relation kind.
## Index
| # | Finding | Severity | Confidence | Anchor |
|---|---------|----------|------------|--------|
| 1 | Refresh paths race the in-flight sequencesync worker: stale values marked READY / XX000 + auto-disable *(merged: 2 findings)* | High | CONFIRMED | src/backend/commands/subscriptioncmds.c:1392, :1336 |
| 2 | `default_transaction_read_only=on` permanently blocks sequence sync via setval() read-only check | Medium | CONFIRMED | src/backend/replication/logical/sequencesync.c:407 |
| 3 | `run_as_owner=false`: un-SET-ROLE-able sequence owner kills the shared worker with an unattributed error | Medium | CONFIRMED | src/backend/replication/logical/sequencesync.c:387 |
| 4 | Gathering scan holds RowExclusiveLock on every INIT relation in one unbounded transaction: blocking + lock-table exhaustion loop | Medium | CONFIRMED | src/backend/replication/logical/sequencesync.c:718 |
| 5 | Sequencesync worker starved by tablesync workers; silent deferral of REFRESH SEQUENCES | Medium | CONFIRMED | src/backend/replication/logical/syncutils.c:127 |
| 6 | No publisher-version guard: pre-PG19 publisher yields perpetual cryptic protocol-violation loop | Medium | CONFIRMED | src/backend/replication/logical/sequencesync.c:529 |
| 7 | `has_sequence_privilege()` NULL on concurrent publisher drop: Assert crash / misreported as permission failure | Medium | CONFIRMED | src/backend/replication/logical/sequencesync.c:298 |
| 8 | Permission-denied sequences also listed as "missing on publisher" in mixed batches | Medium | CONFIRMED | src/backend/replication/logical/sequencesync.c:657 |
| 9 | ALTER SUBSCRIPTION ... DISABLE never stops a running sequencesync worker | Low | CONFIRMED | src/backend/replication/logical/sequencesync.c:447 |
| 10 | REFRESH SEQUENCES vs concurrent subscriber DROP SEQUENCE: internal XX000 errors | Low | CONFIRMED | src/backend/commands/subscriptioncmds.c:1387 |
| 11 | Sequence-removal loop placed after irreversible tablesync slot drops, violating stated invariant | Low | CONFIRMED | src/backend/commands/subscriptioncmds.c:1322 |
| 12 | Batch query mixes MVCC definition columns with live `last_value`: validation bypass under concurrent ALTER SEQUENCE | Low | CONFIRMED | src/backend/replication/logical/sequencesync.c:517 |
| 13 | REFRESH SEQUENCES emits "requested copy_data" warning for an option the command does not accept | Low | CONFIRMED | src/backend/commands/subscriptioncmds.c:3272 |
| 14 | psql tab-completion regression after `REFRESH PUBLICATION` (offers publication names / nothing instead of `WITH (`) | Low | CONFIRMED | src/bin/psql/tab-complete.in.c:2356 |
| 15 | pg_stat_subscription row for sequencesync worker: fabricated message times, NULL columns undocumented *(merged: 2 findings)* | Low | CONFIRMED | src/backend/replication/logical/worker.c:5980; doc/src/sgml/monitoring.sgml:2336 |
| 16 | catalogs.sgml `srsublsn` description wrong for sequence rows | Low | CONFIRMED | doc/src/sgml/catalogs.sgml:8894 |
| 17 | Docs never state sequence sync requires a PG19+ publisher; REFRESH SEQUENCES silently no-ops | Low | CONFIRMED | doc/src/sgml/logical-replication.sgml:2359 |
| 18 | Restrictions bullet still claims only tables can be replicated | Low | CONFIRMED | doc/src/sgml/logical-replication.sgml:2401 |
---
### 1. Refresh paths race the in-flight sequencesync worker (High)
**Merged finding.** Two audit findings are merged here because they share one root cause: no ALTER SUBSCRIPTION refresh path stops or fences a running sequencesync worker (the table path explicitly stops tablesync workers, subscriptioncmds.c:1257, and its removal path documents that its `pg_subscription_rel` lock exists to freeze rel states, :1333-1334), and the worker never re-validates a row between deciding to sync it and writing it.
**Defect.** (A, high) `ALTER SUBSCRIPTION ... REFRESH SEQUENCES` can be silently overwritten by the worker's in-flight batch: up to 100 sequences end up `srsubstate='r'` holding publisher values fetched *before* the refresh, with no error, no warning, and no pending resync. (B, medium) The PUBLICATION refresh forms (`REFRESH PUBLICATION`, `SET/ADD/DROP PUBLICATION`) remove de-published sequence rows out from under the worker, which then raises internal XX000 `subscription relation %u in subscription %u does not exist` after having already applied a non-transactional value overwrite; with `disable_on_error=true` the entire subscription (tables included) is disabled.
**Mechanism.** (A) `AlterSubscription_refresh_seq()` resets every sequence row to `SUBREL_STATE_INIT` (subscriptioncmds.c:1392) with no worker interlock. The worker fetches publisher values with `walrcv_exec` before holding any relevant lock (sequencesync.c:529), then `copy_sequence()` unconditionally applies `SetSequence()` (:407) and `UpdateSubscriptionRelState(..., SUBREL_STATE_READY, ...)` (:416) without rechecking `srsubstate`. `UpdateSubscriptionRelState`'s `LockSharedObject` waits out the ALTER and absorbs its invalidations (pg_subscription.c:405; lmgr.c:1102), so the worker *cleanly* flips the freshly-reset rows to READY with stale values; `FetchRelationStates` keys resync off non-READY rows, so the loss is permanent and invisible. (B) The removal loop calls `RemoveSubscriptionRel()` (subscriptioncmds.c:1336) without `logicalrep_worker_stop`; the worker's subsequent `UpdateSubscriptionRelState` hits `elog(ERROR, ...)` at pg_subscription.c:413-415 — after `SetSequence` (non-transactional, like setval) already overwrote the just-de-published local sequence. PG_CATCH at sequencesync.c:803-819 then either error-loops or runs `DisableSubscriptionAndExit`.
**Trigger.** (A) Any `REFRESH SEQUENCES` committing while a sequencesync batch is between its publisher fetch and its first catalog update — probabilistic at every batch boundary under continuous publisher `nextval()` traffic; deterministic with a lock stall. (B) `DROP PUBLICATION pub_seq` (default `refresh=true`) or `REFRESH PUBLICATION` while initial sequence sync is in flight.
**Repro sketch.** Two nodes, 101 sequences, `FOR ALL SEQUENCES` publication, subscription created. Subscriber session A after the worker's initial scan: `BEGIN; ALTER SEQUENCE s101 RESTART 1;` (ShareRowExclusiveLock stalls the worker at `try_table_open`, sequencesync.c:336, *after* it fetched batch-2 values). Publisher: advance `nextval('s101')` repeatedly. Subscriber session B: `ALTER SUBSCRIPTION sub REFRESH SEQUENCES;` (succeeds, all rows → 'i'). Session A: `ROLLBACK;` → worker marks s101 'r' with the pre-refresh value; failover now yields duplicate sequence values. For (B): 300 sequences + `disable_on_error=true`, then `ALTER SUBSCRIPTION sub DROP PUBLICATION pub_seq;` mid-sync → XX000 in the log and the subscription disabled.
**Fix direction.** Stop (or fence) sequencesync workers in all refresh paths, as the tablesync path does, and/or make `copy_sequence()` re-verify under lock that the row still exists in the expected pre-sync state before writing SetSequence/READY, discarding stale fetched values.
---
### 2. `default_transaction_read_only=on` permanently blocks sequence sync (Medium)
**Defect.** On a subscriber with `default_transaction_read_only=on` — the read-only-subscriber deployment logical-replication.sgml itself describes — tables copy and apply normally, but sequences never sync: the worker fails every `wal_retrieve_retry_interval` with the misleading `cannot execute setval() in a read-only transaction` (25006) for a setval() the user never ran; rows stay 'i' forever, `seq_sync_error_count` climbs, and `disable_on_error=true` disables the whole subscription.
**Mechanism.** `copy_sequence()` applies values via `SetSequence()` (sequencesync.c:407), which runs `PreventCommandIfReadOnly("setval()")` for permanent sequences (sequence.c:976-977). Batch transactions are plain `StartTransactionCommand` (sequencesync.c:462), inheriting `XactReadOnly = DefaultXactReadOnly` (xact.c:2172); worker GUC setup overrides only session_replication_role/search_path/synchronous_commit (worker.c:5798-5810). Every other subscriber write path bypasses read-only checks: tablesync calls `BeginCopyFrom`/`CopyFrom` directly (tablesync.c:1209-1213; the only COPY check lives in `DoCopy`, copy.c:365) and apply uses `ExecSimpleRelation*` with no check — so sequences are uniquely broken.
**Trigger.** Set `default_transaction_read_only=on` in subscriber postgresql.conf (or per-database); subscribe to a `FOR ALL TABLES, ALL SEQUENCES` publication.
**Repro sketch.** Publisher: table t + sequence s, `nextval('s')`, publication for both. Subscriber (GUC on; use a session-local `SET default_transaction_read_only=off` for the DDL): create matching objects, `CREATE SUBSCRIPTION`. Observe t replicating while the log repeats the setval error and s stays 'i'. Undocumented workaround exists (`ALTER ROLE <subscription owner> SET default_transaction_read_only = off`), but nothing hints at it.
**Fix direction.** Have the sequencesync worker's transactions clear `XactReadOnly` (or exempt this internal caller from the setval() read-only check), matching the implicit exemption tablesync/apply already enjoy.
---
### 3. `run_as_owner=false`: one un-SET-ROLE-able sequence owner stalls all sequence sync, unattributed (Medium)
**Defect.** With the default `run_as_owner=false`, one sequence whose owner the subscription owner cannot `SET ROLE` to makes the shared worker die with `role "X" cannot SET ROLE to "Y"` (42501) — an error that never names the sequence — permanently preventing *all* remaining sequences (including fully-compliant ones) from leaving state 'i'; `disable_on_error=true` disables the whole subscription.
**Mechanism.** `copy_sequence()` calls `SwitchToUntrustedUser(seqowner, ...)` (sequencesync.c:387) *before* the ACL check; `SwitchToUntrustedUser` raises ERROR on membership failure (usercontext.c:40-45). Unlike the `pg_class_aclcheck` failure two lines later — classified `COPYSEQ_SUBSCRIBER_INSUFFICIENT_PERM` (:396) and reported per-sequence by name via `report_sequence_errors()` (:201-224) while the batch continues — this ERROR is uncaught inside the batch loop (transaction spans :462-:647), rolls back batch-mates' READY updates, kills the worker before later batches, and repeats identically on every relaunch. No `ErrorContextCallback` exists in sequencesync.c, so the log never identifies the culprit. Aggravating: in default mode the graceful path checks the *owner's* privileges on their own sequence (essentially always true), so the warning framework is nearly dead code while the realistic failure takes the worker-killing path; 036_sequences.pl never tests this scenario.
**Trigger.** Subscription owned by non-superuser `bob`; any subscribed sequence owned by `alice` where `bob` lacks SET ROLE membership.
**Repro sketch.** Subscriber: sequences `s_ok` (owner bob) and `s_alice` (owner alice, bob not a member); publisher publishes both `FOR ALL SEQUENCES`; bob creates the subscription. Both sequences stay 'i' forever; log repeats the role error with no sequence name.
**Fix direction.** Classify the SET ROLE failure per-sequence like the other permission failures (new COPYSEQ_* code + warning naming the sequence) instead of letting it kill the worker, and add error context identifying the sequence being synced.
---
### 4. Gathering scan holds a lock on every INIT relation in one unbounded transaction (Medium)
**Defect.** The worker's initial catalog scan opens **every** `srsubstate='i'` relation — tables awaiting tablesync included — with `try_table_open(..., RowExclusiveLock)` *before* checking relkind, in a single transaction, retaining all locks until commit. Consequences: (a) one conflicting lock on any INIT relation stalls all sequence sync indefinitely while the worker retains everything already scanned; (b) with thousands of INIT sequences the transaction exhausts the shared lock table, producing a permanent `out of shared memory` / `increase max_locks_per_transaction` retry loop every 5 s with zero progress, transiently draining the lock pool for unrelated sessions each attempt.
**Mechanism.** `LogicalRepSyncSequences()`: one transaction (sequencesync.c:691-752); `try_table_open` at :718 precedes the relkind check at :725; both close paths use `NoLock` (:727, :745). `try_relation_open` blocks on the lock (relation.c:96-97 — "try" covers only nonexistence). An ERROR here precedes any READY update, so the INIT set never shrinks; relaunch every `wal_retrieve_retry_interval` (syncutils.c:127-147). The locks are demonstrably unnecessary — `FetchRelationStates` distinguishes sequences lock-free via `get_rel_relkind()` (syncutils.c:243) — and `copy_sequences()` already caps itself at 100 locks/transaction. Verifier corrections to the original claim: fresh CREATE SUBSCRIPTION cannot reach the exhaustion (the foreground command's own per-relation locking fails first); the reachable route is `ALTER SUBSCRIPTION ... REFRESH SEQUENCES` (flips all sequences to INIT with zero relation locks, subscriptioncmds.c:1360-1406) combined with a lock-pool squeeze, e.g. a subscriber later restarted with lower `max_connections` — and `max_locks_per_transaction` is PGC_POSTMASTER, so recovery requires a restart or dropping/disabling the subscription.
**Trigger.** Blocking: an open `BEGIN; ALTER SEQUENCE s1 RESTART ...;` (ShareRowExclusiveLock, sequence.c:450) when the scan runs. Exhaustion: ~5000 subscribed sequences, subscriber restarted with small `max_connections`, then `REFRESH SEQUENCES`.
**Repro sketch.** Blocking: 2 sequences subscribed and 'r'; session A `BEGIN; ALTER SEQUENCE s1 RESTART 100;` (leave open); `ALTER SUBSCRIPTION sub REFRESH SEQUENCES;` → worker blocks at `try_table_open(s1)`, s2 also never syncs. Exhaustion: 5000 sequences synced under `max_connections=100`; restart subscriber with `max_connections=20`; `REFRESH SEQUENCES` → perpetual out-of-shared-memory loop; `disable_on_error=true` disables the subscription on first failure.
**Fix direction.** Drop the relation locks from the gathering scan (filter by `get_rel_relkind()` as `FetchRelationStates` does), deferring per-sequence locking to the already-batched copy phase.
---
### 5. Sequencesync worker starved by tablesync workers; REFRESH SEQUENCES silently deferred (Medium)
**Defect.** While ≥ `max_sync_workers_per_subscription` tables are actively copying, the sequencesync worker is never launched — with no log message, no worker row, and no pacing state — so sequences stay 'i' for the full duration of a bulk table copy, even after an explicit `REFRESH SEQUENCES`. A DBA running the documented pre-failover `REFRESH SEQUENCES` during a large copy and then promoting gets never-synced sequences.
**Mechanism.** Both worker types share one budget (`logicalrep_sync_worker_count`, launcher.c:949). `ProcessSyncingRelations()` always runs tables before sequences (syncutils.c:173-175); the table loop launches immediately for any never-started table (`last_start_time==0`, tablesync.c:566-567), retaking every freed slot in the same pass; `launch_sync_worker()` then returns silently at the cap (syncutils.c:127). sequencesync.c:138 is the *only* launch site — the launcher never starts one. Verifier caveats: starvation ends when pending tables drop below the limit (not literally indefinite; failing tables' throttled relaunches leave windows), and logical-replication.sgml:1814-1819 does disclose the budget cap — but the strict tables-first priority is undocumented, and config.sgml:5695-5700's "one additional worker ... for sequence synchronization" implies provisioning +1 helps, which the greedy table loop defeats at any GUC value.
**Trigger.** Default `max_sync_workers_per_subscription=2`, ≥2 large tables mid-copy, then `REFRESH SEQUENCES`.
**Repro sketch.** Publisher: 3 tables × 10M rows + 1 advanced sequence, publications `FOR ALL TABLES` and `FOR ALL SEQUENCES`. Subscriber: subscribe to both; while ≥2 tablesync workers run, `ALTER SUBSCRIPTION sub REFRESH SEQUENCES;` and poll — sequence rows stay 'i' and `pg_stat_subscription` shows no `sequence synchronization` worker until the copy tail.
**Fix direction.** Reserve a slot for (or prioritize) sequence sync when INIT sequences exist — or at minimum log the deferral — and align config.sgml with the actual shared-budget behavior.
---
### 6. No publisher-version guard: pre-PG19 publisher causes a perpetual, cryptic error loop (Medium)
**Defect.** A subscription with registered sequences repointed at a pre-PG19 publisher (e.g. upgrade rollback via `ALTER SUBSCRIPTION ... CONNECTION`, or a DNS/VIP flip mid-sync) enters an endless 5-second error/relaunch loop: against PG18, `ERROR: invalid query response DETAIL: Expected 11 fields, got 10 fields.` (SQLSTATE 08P01, suggesting protocol corruption, not a version problem); against ≤PG17, `function pg_get_sequence_data(oid) does not exist`. Sequences stay 'i'; `disable_on_error=true` disables the whole subscription.
**Mechanism.** sequencesync.c has zero `walrcv_server_version()` checks (the only ≥190000 gates in the sync paths are tablesync.c:802, 1004); `copy_sequences()` unconditionally sends the batch query (:517-527) expecting REMOTE_SEQ_COL_COUNT=11 columns at :529. PG18's `pg_get_sequence_data()` exists but returns 2 output columns (page_lsn added by b93172c for PG19), so the query succeeds remotely with 10 fields and `libpqrcv_processTuples` errors (libpqwalreceiver.c:1083-1088). The broken state is reachable because sequence rows persist across `ALTER SUBSCRIPTION ... CONNECTION`, and `AlterSubscription_refresh_seq` *succeeds* against an old publisher (its origin check returns early for <190000, subscriptioncmds.c:3196-3198), reporting success and then unleashing the storm. `REFRESH PUBLICATION` incidentally recovers by removing the rows, but nothing in the error suggests that.
**Trigger.** PG19→PG19 subscription with synced sequences; repoint the connection at a PG18 host holding the slot; `REFRESH SEQUENCES`.
**Repro sketch.** Node A (PG19) and node B (PG18) with identical schema; subscribe to A (`FOR ALL SEQUENCES`), wait for 'r'; `ALTER SUBSCRIPTION sub1 CONNECTION 'host=B ...'; ALTER SUBSCRIPTION sub1 REFRESH SEQUENCES;` → 5-second loop of worker-start LOG + 08P01 ERROR.
**Fix direction.** Version-gate the sequencesync launch/copy path (and `REFRESH SEQUENCES`) with a clear "publisher (version N) does not support sequence synchronization" error or warning instead of retrying a query that can never succeed.
---
### 7. `has_sequence_privilege()` NULL on concurrent publisher DROP: Assert crash / wrong diagnosis (Medium)
**Defect.** A published sequence dropped on the publisher while the batch query runs yields a result row with NULL in the `has_sequence_privilege` column; assert-enabled subscribers hit `Assert(!isnull)` (sequencesync.c:297-298) — TRAP → SIGABRT → crash-restart of the sequencesync worker; production builds decode NULL as false and misreport the drop as `insufficient privileges on publisher sequence (...)` with a bogus GRANT SELECT hint. This re-introduces the concurrent-drop hazard 1ba3eee fixed for the data columns: d4a657b added the privilege column with an unconditional Assert.
**Mechanism.** The batch query (sequencesync.c:517-527) joins pg_class/pg_sequence under the query snapshot, but `has_sequence_privilege_id()` uses `get_rel_relkind()` on the current catalog snapshot and returns SQL NULL for a vanished relation (acl.c:2246-2248). The row is still emitted (old snapshot), and the LATERAL `pg_get_sequence_data()` — which locks the relation and absorbs the drop's invalidations first (lmgr.c:134-137; commit ordering xact.c:2475/2480) — makes the NULL deterministic, not a narrow race: an uncommitted DROP holds AccessExclusiveLock, the sync query blocks inside it, and COMMIT triggers the condition every time. With NULL data + false privilege, production takes the `COPYSEQ_PUBLISHER_INSUFFICIENT_PERM` path (:300-303) instead of the concurrent-drop `COPYSEQ_SKIPPED` path.
**Trigger.** Publisher-side `DROP SEQUENCE` committing while the batch SELECT executes.
**Repro sketch.** Two sequences subscribed and 'r'. Publisher session A: `BEGIN; DROP SEQUENCE s2;` (hold). Subscriber: `ALTER SUBSCRIPTION sub REFRESH SEQUENCES;` (batch query blocks on A's lock). Publisher: `COMMIT;` → assert build: TRAP at sequencesync.c:298; production: wrong privilege WARNING + sync-failure ERROR.
**Fix direction.** Treat a NULL privilege column as a concurrently-dropped sequence (fold into the existing `COPYSEQ_SKIPPED` handling) rather than asserting non-null.
---
### 8. Permission-denied sequences double-reported as "missing on publisher" (Medium)
**Defect.** When one batch contains both a genuinely missing sequence and a publisher-permission-denied sequence, the perm-denied sequence is listed in *both* warnings: `insufficient privileges on publisher sequence ("public.s2")` and `missing sequences on publisher ("public.s1", "public.s2")` — contradictory diagnoses for an object that exists (the worker even received a row for it). This is the residual gap in d4a657b, whose stated purpose was exactly to stop permission failures being reported as missing.
**Mechanism.** The perm-denied early return (sequencesync.c:300-303) fires before `found_on_pub = true` is set (:334), so perm-denied entries keep `found_on_pub=false` (palloc0 at :737). A genuinely missing sequence produces no row (inner joins, :517-527), making `batch_missing_count > 0` (:633-637), which enables the post-commit scan (:649-660) that appends *every* `!found_on_pub` entry to the missing list (:657) with no dedup in `report_sequence_errors()` (:226-251). d4a657b's own test (036_sequences.pl:228-262) recreated the dropped sequence before the permission test, so the mixed batch was never exercised.
**Trigger.** Same ≤100-sequence batch containing one sequence dropped on the publisher and one with SELECT revoked from the subscription's connection role, then `REFRESH SEQUENCES` (which never prunes publisher-dropped sequences).
**Repro sketch.** Both nodes: s1, s2; subscribe, wait for 'r'. Publisher: `REVOKE SELECT ON SEQUENCE s2 FROM repuser; DROP SEQUENCE s1;`. Subscriber: `ALTER SUBSCRIPTION sub REFRESH SEQUENCES;` → the contradictory warning pair repeats on every relaunch.
**Fix direction.** Set `found_on_pub` before the permission early-return (or exclude entries already classified perm-denied from the missing scan) so each sequence appears in exactly one list.
---
### 9. ALTER SUBSCRIPTION ... DISABLE never stops a running sequencesync worker (Low)
**Defect.** After `DISABLE`, the apply worker exits promptly but the sequencesync worker keeps starting new batch transactions, overwriting subscriber sequence values and flipping `pg_subscription_rel` rows to 'r' after the disable — for minutes normally, indefinitely if lock-blocked — contradicting alter_subscription.sgml's "stopping the logical replication worker" (:270-278). A `nextval()` run locally post-DISABLE can be clobbered back to an older publisher value.
**Mechanism.** The worker's entire lifetime (SequenceSyncWorkerMain → ... → `copy_sequences` loop, sequencesync.c:447-668) contains no `maybe_reread_subscription()` (call sites are only worker.c:740/2565/4183 and applyparallelworker.c:283) and never reads `MySubscriptionValid`/`enabled` after startup; DISABLE only writes `subenabled=false` and signals nothing (`logicalrep_worker_stop` is called only at subscriptioncmds.c:1257 and DropSubscription :2636); no exit path stops it (launcher.c:822-842 handles only parallel apply workers). Related to item 1's root cause (worker never revalidates anything), but the trigger is subscription-level state rather than per-relation rows, so reported separately; a batch-boundary recheck would address both.
**Trigger.** `DISABLE` while a sync of many sequences is in flight, or while the worker is blocked at `try_table_open` (:336/:718) behind an open `ALTER SEQUENCE`.
**Repro sketch.** 300 sequences; subscriber session A holds `BEGIN; ALTER SEQUENCE s150 RESTART 1;`; create subscription; once the worker blocks, `ALTER SUBSCRIPTION sub DISABLE;` (apply worker exits, sequencesync worker persists); `COMMIT;` in A → post-disable, sequences s101-s300 change values and READY count climbs to 300.
**Fix direction.** Recheck subscription validity/enabled at each batch boundary in `copy_sequences()` (and consider having DISABLE stop sync workers explicitly).
---
### 10. REFRESH SEQUENCES vs concurrent subscriber DROP SEQUENCE: internal XX000 errors (Low)
**Defect.** Ordinary DDL concurrency — `REFRESH SEQUENCES` racing a subscriber-side `DROP SEQUENCE` — ends with one command failing on an internal elog: XX000 `subscription relation %u in subscription %u does not exist`, `tuple concurrently deleted`, or the DROP failing `tuple concurrently updated`; assert builds can additionally trip the relkind Assert in `GetSubscriptionRelations` (pg_subscription.c:666). Transient (retry succeeds) but corruption-looking.
**Mechanism.** `AlterSubscription_refresh_seq()` builds its list (subscriptioncmds.c:1387) holding no lock on the sequences and *not* taking the pg_subscription_rel AccessExclusiveLock its sibling removal paths take precisely to freeze rel states (:1333-1334); `DROP SEQUENCE` deletes the row via `heap_drop_with_catalog` → `RemoveSubscriptionRel` with only RowExclusiveLock on pg_subscription_rel (pg_subscription.c:505) and no subscription-object lock, so nothing serializes the commands until they collide on the catalog tuple (`UpdateSubscriptionRelState` syscache miss → elog at pg_subscription.c:413-415; heapam.c:4477/3176 for the wait-then-fail variants). The sync worker itself is protected against this race (it holds the relation open across its update, sequencesync.c:336/416); the refresh command is the unprotected outlier.
**Trigger.** Deterministic: `BEGIN; DROP SEQUENCE s1;` in one session, `REFRESH SEQUENCES` in another, then COMMIT (→ "tuple concurrently deleted"); reverse order inside `BEGIN` for "tuple concurrently updated" (REFRESH SEQUENCES is allowed in a transaction block).
**Repro sketch.** As trigger, on a subscription with s1/s2 already 'r'.
**Fix direction.** Take the same pg_subscription_rel AccessExclusiveLock (or per-sequence locks) in `AlterSubscription_refresh_seq`, and/or tolerate a concurrently-vanished row with a skip instead of elog.
---
### 11. Sequence-removal loop runs after irreversible tablesync slot drops (Low)
**Defect.** f0b3573c3 appended the sequence-removal loop (subscriptioncmds.c:1322-1344) *after* the `ReplicationSlotDropAtPubNode()` loop (:1295-1316), violating the explicit invariant at :1290-1294 that slot drops must come last because they cannot be rolled back. An error or cancel during sequence removal (each `RemoveSubscriptionRel` seqscans pg_subscription_rel with CFI, pg_subscription.c:497-567) aborts the transaction after publisher-side sync slots were dropped; a rolled-back-to-`FINISHEDCOPY` ('f') table then reuses its now-nonexistent slot without recreating it (tablesync.c:1342-1361) and error-loops with `replication slot "pg_%u_sync_%u_..." does not exist`.
**Skeptical caveats (verifier).** Re-running the failed ALTER self-heals (the slot drop is `missing_ok`); the wedge persists only if the publication change is abandoned. And with ≥2 removed tables a cancel between slot drops could already cause this pre-f0b3573c3 — the new loop violates the stated invariant, substantially widens the window, and newly exposes the single-removed-table case.
**Trigger.** `SET statement_timeout='2s'; ALTER SUBSCRIPTION sub SET PUBLICATION pub_new;` where the old set contained a mid-catchup ('f') table plus many sequences, timeout landing in the sequence loop.
**Repro sketch.** Large table held at 'f' via an AccessExclusiveLock on the subscriber; subscription also covers a sequence; ALTER SET PUBLICATION to a set containing neither, cancelled during sequence removal (gdb breakpoint at :1322 + `pg_cancel_backend` for determinism) → t1 wedged at 'f' with its slot gone.
**Fix direction.** Move the sequence-removal loop above the slot-drop loop, restoring the "slot drops last" invariant.
---
### 12. Batch query mixes MVCC definition columns with live `last_value`: validation bypass (Low)
**Defect.** A publisher `ALTER SEQUENCE` + `nextval()` committing while the batch query runs can produce an internally inconsistent row (old seqmin/seqmax/seqcycle, new last_value) that passes definition validation, yielding either a confusing `setval: value N is out of bounds for sequence` ERROR (22003) from a worker that never ran setval (subscription disabled under `disable_on_error`), or — CYCLE added upstream — a *silently* synced wrapped value marked READY despite a CYCLE/NO CYCLE mismatch the documented validation promises to catch.
**Mechanism.** The query (sequencesync.c:517-527) reads pg_sequence under the statement snapshot but `pg_get_sequence_data()` is volatile and reads the live buffer tuple (sequence.c:1831-1836); `ALTER SEQUENCE` takes only ShareRowExclusiveLock (sequence.c:450), not conflicting with the LATERAL's transient AccessShareLock. `get_and_validate_seq_info()` compares only the stale definition columns (:352-358); `SetSequence()`'s local bounds check (sequence.c:989-994) then errors, or the wrapped value fits and is marked READY (:416) with no warning. Same query-consistency family as item 7 (1ba3eee fixed the drop case; the ALTER case remains). Window is one publisher-side query execution — narrow; deterministic only with a paused walsender. The error variant self-corrects into the documented mismatch report on the next relaunch.
**Trigger.** `REFRESH SEQUENCES` concurrent with publisher `ALTER SEQUENCE s MAXVALUE 200; SELECT nextval('s')...` (error variant) or `ALTER SEQUENCE s CYCLE; SELECT nextval('s')` at max (silent variant).
**Repro sketch.** Both nodes `CREATE SEQUENCE s MAXVALUE 100`; subscribe; pause the publisher walsender in `pg_get_sequence_data` (gdb), apply the ALTER+nextval, resume.
**Fix direction.** Return the definition columns from `pg_get_sequence_data()` itself (one consistent read of the live tuple), or re-validate the received `last_value` against the local definition and classify violations as a retryable mismatch rather than letting setval's error surface.
---
### 13. REFRESH SEQUENCES warning misattributes a `copy_data` request the command cannot express (Low)
**Defect.** Plain `ALTER SUBSCRIPTION sub REFRESH SEQUENCES` on an `origin=none` subscription in a bidirectional setup emits `WARNING: subscription "sub" requested copy_data with origin = NONE but might copy data that had a different origin` — but REFRESH SEQUENCES accepts no `WITH` options at all (gram.y:11611-11619; no options parsed at subscriptioncmds.c:1614-1654), so the user "requested" nothing; they will hunt for a nonexistent knob.
**Mechanism.** `AlterSubscription_refresh_seq()` calls `check_publications_origin_sequences(..., true /* copydata hardcoded */, ...)` (subscriptioncmds.c:1383-1384), which reuses the CREATE SUBSCRIPTION/REFRESH PUBLICATION message at :3270-3277 — accurate at those call sites (which pass the user's actual copy_data), inaccurate here. The warning's substance (sequence data will be copied and may have a different origin) and hint remain correct; only the attribution is wrong.
**Trigger/repro sketch.** Two-node bidirectional `origin=none` sequence subscriptions; run `REFRESH SEQUENCES` on either side.
**Fix direction.** Use a REFRESH SEQUENCES-specific message ("... will copy sequence data that might have a different origin") instead of the "requested copy_data" wording.
---
### 14. psql tab-completion regression after `REFRESH PUBLICATION` (Low)
**Defect.** In PG18, `ALTER SUBSCRIPTION sub REFRESH PUBLICATION <TAB>` completed `WITH (`; in master it offers the subscriber's *local* publication names (syntactically invalid if accepted — the command takes no name, gram.y:11601) or nothing when no local publication exists.
**Mechanism.** f0b3573c3 deleted the rule `Matches("ALTER","SUBSCRIPTION",MatchAny,MatchAnyN,"REFRESH","PUBLICATION") → COMPLETE_WITH("WITH (")` (REL_18_0 tab-complete.in.c:2306), replacing it with a REFRESH-terminal rule (master :2355-2356) and never re-adding the follow-up; no remaining pattern matches, so control falls to the `words_after_create` fallback, where prev_wd "PUBLICATION" hits `Query_for_list_of_publications` (:1335, :1212-1215). The surviving `REFRESH PUBLICATION WITH (` → copy_data rule (:2358) shows the path is still intended.
**Trigger/repro sketch.** Subscriber with a local publication (bidirectional setup); type the command and press Tab.
**Fix direction.** Restore the deleted `..."REFRESH","PUBLICATION"` → `COMPLETE_WITH("WITH (")` rule.
---
### 15. pg_stat_subscription row for the sequencesync worker: fabricated times, undocumented NULLs (Low)
**Merged finding.** Two audit findings merged: both stem from adding the new worker type to `pg_stat_subscription` (55cefadde touched only the `worker_type` row in monitoring.sgml) without adjusting either the column semantics in code or the per-column NULL documentation.
**Defect.** For its entire lifetime, a sequencesync worker's row shows `last_msg_send_time`/`last_msg_receipt_time`/`latest_end_time` frozen at the worker's *start* time — masquerading as WAL-sender message times for a worker type that never receives WAL-sender messages — while `received_lsn`/`latest_end_lsn`/`relid`/`leader_pid` are always NULL. monitoring.sgml (:2325-2327, :2336-2337, :2346-2347, :2376-2377) enumerates NULL cases that omit this worker type entirely (e.g. relid "NULL for the leader apply worker and parallel apply workers"; time/LSN columns "NULL for parallel apply workers"). A DBA watching a long sync sees frozen "message" timestamps and an inconsistent row (`latest_end_time` set, `latest_end_lsn` NULL) suggesting a hung connection.
**Mechanism.** `SetupApplyOrSyncWorker()` initializes the three timestamps to `GetCurrentTimestamp()` (worker.c:5979-5981); the only updater, `UpdateWorkerStats()` (worker.c:3987), is called solely from `LogicalRepApplyLoop`, which this worker never enters; `pg_stat_get_subscription()` NULLs timestamps only when exactly 0 (launcher.c:1671-1683), a convention only parallel apply workers satisfy (applyparallelworker.c:958-959), and emits `relid` only for tablesync workers (launcher.c:1656-1659; sequencesync launches with InvalidOid relid, sequencesync.c:138).
**Trigger/repro sketch.** Poll `SELECT worker_type, relid, leader_pid, received_lsn, latest_end_lsn, last_msg_send_time FROM pg_stat_subscription WHERE worker_type='sequence synchronization'` during a sync of a few thousand sequences.
**Fix direction.** Zero the time fields for sequencesync workers so they render NULL (matching the parallel-apply convention), and extend monitoring.sgml's per-column NULL enumerations to cover the new worker type.
---
### 16. catalogs.sgml `srsublsn` description wrong for sequence rows (Low)
**Defect.** catalogs.sgml:8893-8896 still describes `srsublsn` solely as "Remote LSN of the state change ... when in s or r states, otherwise null". For a sequence row it actually holds the publisher sequence's *page LSN* from `pg_get_sequence_data()` — possibly far older than the sync, and precisely the value logical-replication.sgml:1844-1853 tells DBAs to compare in the out-of-sync procedure — and `copy_data=false` creates 'r'-state sequence rows with `srsublsn` NULL, contradicting "otherwise null".
**Mechanism.** `copy_sequence()` writes `UpdateSubscriptionRelState(..., SUBREL_STATE_READY, seqinfo->page_lsn, ...)` (sequencesync.c:416-417), page_lsn being `PageGetLSN()` of the publisher's sequence page (sequence.c:1836); copy_data=false inserts sequences directly READY with InvalidXLogRecPtr → SQL NULL (subscriptioncmds.c:985, :1203-1205; pg_subscription.c:353-356). Caveat: the ('r', NULL) shape has existed for tables since PG10; the feature-specific gap is the undocumented page-LSN meaning, which the sequences docs actively rely on.
**Trigger/repro sketch.** `CREATE SUBSCRIPTION ... WITH (copy_data=false)` → ('r', NULL) rows; after `REFRESH SEQUENCES`, `srsublsn` equals the publisher's `pg_get_sequence_data(...).page_lsn`.
**Fix direction.** Amend the `srsublsn` entry to state its sequence-row meaning (publisher page LSN used for out-of-sync comparison) and the copy_data=false NULL case.
---
### 17. Docs omit the PG19+ publisher requirement for sequence sync; REFRESH SEQUENCES silently no-ops (Low)
**Defect.** The failover-preparation restriction (logical-replication.sgml:2359-2372) recommends `ALTER SUBSCRIPTION ... REFRESH SEQUENCES` with no stated publisher-version precondition, and no SGML file anywhere mentions one; against a pre-PG19 publisher, sequences are silently never registered (`fetch_relation_list` gates the sequence UNION on `server_version >= 190000`, subscriptioncmds.c:3467) and `REFRESH SEQUENCES` succeeds as a fully silent no-op (origin check returns early, :3196-3198; empty relation list, empty loop). A DBA on the standard staged-upgrade topology (PG19 subscriber ← PG18 publisher) follows remedy #1, sees success, and discovers the gap as duplicate keys after failover — when remedies #2/#3 (manual update) would have worked.
**Skeptical caveat.** Severity capped low: `FOR ALL SEQUENCES` cannot be created on a pre-PG19 publisher, so a from-scratch setup fails loudly at step one; the exposed audience is existing cross-version subscriptions. Related to item 6 (same version dependency) but distinct: item 6 is the error loop when sequence rows *do* exist; this is silent success when they don't. In-tree precedent documents comparable cross-version caveats (binary pre-16, row filters pre-15) and `retain_dead_tuples` even errors on old publishers (:3304-3307).
**Trigger/repro sketch.** PG18 publisher `FOR ALL TABLES` with a serial column advancing to ~1000; PG19 subscriber follows the restrictions text: `REFRESH SEQUENCES` succeeds, `last_value` stays 1; post-failover insert → duplicate key.
**Fix direction.** State the PG19+ publisher requirement in the sequences documentation (and consider a NOTICE/WARNING from `REFRESH SEQUENCES` when the subscription has no subscribed sequences or the publisher predates support).
---
### 18. Restrictions bullet still claims only tables can be replicated (Low)
**Defect.** logical-replication.sgml:2399-2405 (unchanged since 17b9e7f) states "Replication is only supported by tables ... Attempts to replicate other types of relations, such as views, materialized views, or foreign tables, will result in an error" — contradicted three sections earlier by "Replicating Sequences" (:1772) and by the adjacent sequence-restrictions bullet (:2357-2372) added by 55cefadde.
**Mechanism.** Sequences are publishable (`is_publishable_class` accepts RELKIND_SEQUENCE, pg_publication.c:163-172) and valid subscription targets (`CheckSubscriptionRelkind`, execReplication.c:1140-1166); `FOR ALL SEQUENCES` grammar exists (gram.y:11422); 036_sequences.pl demonstrates successful sequence sync with no error. The only defense — reading "Replication" as strictly incremental streaming — fails against the bullet's categorical second sentence and the docs' own "Replicating Sequences" terminology.
**Trigger/repro sketch.** Read the Restrictions list, then observe `CREATE PUBLICATION p FOR ALL SEQUENCES` + subscription syncing values without error.
**Fix direction.** Reword the bullet to say tables and sequences are supported (sequences synchronized on request rather than streamed), keeping the error claim accurate for views/matviews/foreign tables.
---
## Appendix A: findings dropped unverified (verifier cap of 23)
These 5 ranked below the cap and were **not** adversarially verified; treat as leads only.
- ProcessSequencesForSync's only_running=true check allows two concurrent sequencesync workers for one subscription, causing "tuple concurrently updated" errors and disable_on_error trips
- Unlogged subscriber sequence reaches durable READY state while its synced value is lost on crash, permanently stale and reported in-sync
- create_subscription.sgml claims the origin parameter "has no effect for sequences", yet origin=none triggers sequence-specific origin warnings and sequence values of foreign origin are copied regardless, which is documented nowhere
- Docs recommend ALTER SUBSCRIPTION ... REFRESH SEQUENCES to update sequences at failover, but the command cannot run once the publisher is down and the duplicate-value consequence of lagging sequences is never stated
- Connect-failure message uses internal jargon "sequencesync worker", inconsistent with the same worker's other messages and the tablesync counterpart
## Appendix B: findings refuted by adversarial verification
- **REFRESH SEQUENCES is allowed inside a transaction block and can deadlock with the sequencesync worker when combined with ALTER SEQUENCE**
- Refutation: The individual lock sites are all real (verified in master: no PreventInTransactionBlock in the ALTER_SUBSCRIPTION_REFRESH_SEQUENCES case at src/backend/commands/subscriptioncmds.c:2286-2297; AccessExclusiveLock on the subscription object at subscriptioncmds.c:1714 for ALL ALTER SUBSCRIPTION forms; AccessShareLock on the subscription object in UpdateSubscriptionRelState at src/backend/catalog/pg_subscription.c:405 held to batch commit; try_table_open(RowExclusiveLock) at src/backend/replication/logical/sequencesync.c:336; ShareRowExclusiveLock for ALTER SEQUENCE at src/backend/commands/sequence.c:449-453). But the finding fails on four counts. (1) Its trigger is unreachable: copy_sequence -> UpdateSubscriptionRelState is called ONLY on COPYSEQ_SUCCESS (sequencesync.c:554-556, 416), and per-batch successes are committed (line 647) before report_sequence_errors raises the mismatch ERROR (line 671), so in the claimed 'worker retries every 5s due to a definition mismatch' steady state the retry batch contains only failing INIT sequences, which never acquire the subscription-object lock -- no cycle can form in the documented mismatch-repair workflow; a worker parked on the user's ALTERed sequence holds nothing the user's REFRESH SEQUENCES needs. (2) The causal premise is wrong: the other refresh forms forbid transaction blocks because AlterSubscription_refresh irreversibly drops table-synchronization slots on the publisher (per alter_subscription.sgml), not to avoid deadlocks; AlterSubscription_refresh_seq (subscriptioncmds.c:1360-1406) performs only read-only publisher queries and rollbackable local catalog updates, and the docs' explicit list of forms that 'cannot be executed inside a transaction block' intentionally omits REFRESH SEQUENCES -- code and documentation agree, so this is designed behavior. (3) Adding PreventInTransactionBlock to REFRESH SEQUENCES would not eliminate the deadlock: line 1714 is reached by every ALTER SUBSCRIPTION form, and ENABLE/DISABLE/SET/SKIP are deliberately allowed in transaction blocks (ENABLE docs even say the worker starts 'at the end of the transaction'), so BEGIN; ALTER SEQUENCE s; ALTER SUBSCRIPTION sub DISABLE; forms the identical cycle, as does BEGIN; ALTER SEQUENCE s1; ALTER SEQUENCE s2; against a worker retaining RowExclusiveLock on s2 from earlier in the batch (table_close NoLock, line 625). (4) The deadlock class is pre-existing and accepted: tablesync has held the target-table RowExclusiveLock across COPY (tablesync.c:1401) and then taken the subscription-object AccessShareLock via UpdateSubscriptionRelState (tablesync.c:1503) in one transaction since commit cb9079c (2017), giving the same detector-resolved cycle against transaction-block ALTER SUBSCRIPTION forms; and the comment at sequencesync.c:485-509 shows the developers explicitly analyzed worker-vs-ALTER SEQUENCE deadlocks and guarded only against the undetectable cross-node variant, accepting local detectable ones. The only constructible outcome (requires a multi-sequence batch with at least one success before the contended sequence, i.e. mid initial-sync or mid full re-sync, plus a deliberately open transaction mixing sequence DDL with ALTER SUBSCRIPTION) is a transient 'ERROR: deadlock detected' that the deadlock detector resolves and the respawned worker recovers from -- standard, accepted PostgreSQL DDL concurrency behavior, not a user-visible defect of the sequence-replication feature.
- **Sequencesync batches accumulate RowExclusiveLock on up to 100 sequences until commit, creating deadlocks with ordinary multi-sequence DDL or ALTER SUBSCRIPTION in the same transaction**
- Refutation: The mechanism is factually accurate but describes ordinary, deliberate, detected-and-resolved PostgreSQL locking behavior, not a defect. Verified: sequencesync batches take try_table_open(seq, RowExclusiveLock) per row (sequencesync.c:336), retain locks via table_close(NoLock) (line 625) until CommitTransactionCommand (line 647) for up to 100 sequences (line 441); ALTER SEQUENCE takes conflicting ShareRowExclusiveLock (sequence.c:449-450); UpdateSubscriptionRelState takes AccessShareLock on the subscription held to commit (pg_subscription.c:405); ALTER SUBSCRIPTION takes AccessExclusiveLock (subscriptioncmds.c:1714). So the claimed lock cycles can occur — but both parties are ordinary backends on heavyweight locks, so the local deadlock detector resolves the cycle after deadlock_timeout with a clean, retryable "deadlock detected" error. That disqualifies it as a defect on four grounds: (1) The trigger requires the user's own transaction to acquire conflicting locks on multiple objects in inconsistent order — the canonical application-responsibility scenario per the docs (mvcc.sgml "Deadlocks"); two plain user sessions doing the same two ALTER SEQUENCEs in opposite order deadlock identically with no replication involved. (2) The finding's novelty claim ("unlike tablesync ... this wait-while-holding pattern is new") is wrong: the apply worker has accumulated RowExclusiveLocks on many relations per applied transaction, in remote-determined order the local user cannot predict, since PG10 (worker.c:2673/2730, 2834/2918, 3057/3117 open with RowExclusiveLock, close with NoLock), and applyparallelworker.c:62-67 explicitly states such deadlocks "can happen even without parallel mode when there are concurrent operations on the subscriber" — the project's design bar is detectability, not prevention. (3) sequencesync.c deliberately implements that doctrine: the header comment (lines 42-44) documents batch-duration lock retention, and the comment at 485-509 orders the remote fetch (walrcv_exec, line 529) before any local sequence locks so the worker never waits on the network while holding local locks, eliminating undetectable cross-node deadlocks and knowingly accepting locally detectable ones. (4) The outcome is transient, atomic, and self-healing: a victim worker's batch aborts atomically (SetSequence and READY-state updates roll back together, sequences stay INIT, no inconsistent catalog state), start_sequence_sync reports stats and rethrows, and the apply worker relaunches a sequencesync worker after wal_retrieve_retry_interval (syncutils.c:117-148, 175), which then succeeds; disable_on_error disabling the subscription on any error, including deadlocks, is that option's documented contract and applies equally to apply-worker deadlocks today. There is also no actionable fix consistent with the design: the lock protecting the WAL-logged SetSequence update cannot be released before commit, batching is a documented performance choice, and no ordering discipline can be consistent with arbitrary user DDL order. Not covered by any already-fixed commit (git log on sequencesync.c shows no locking changes). A report of this would be answered "working as intended; retry on deadlock", so confirming it would waste committer time.
- **A failed or interrupted sync batch leaves subscriber sequence values silently overwritten (non-transactionally) while pg_subscription_rel still says INIT**
- Refutation: Every code citation in the finding is accurate — I traced them all in master: copy_sequences() batches up to 100 sequences per transaction (src/backend/replication/logical/sequencesync.c:441, StartTransactionCommand :462, CommitTransactionCommand :647), accumulates RowExclusiveLocks to commit (:336, table_close(NoLock) :625), copy_sequence() calls SetSequence at :407 followed by transactional UpdateSubscriptionRelState(SUBREL_STATE_READY) at :416-417, and SetSequence (src/backend/commands/sequence.c:946-1043) updates the sequence page in place in a critical section with XLOG_SEQ_LOG (:1011-1038) — no relfilenode swap, so the change is immediately visible (frozen-xmin tuple) and survives transaction abort. A pg_terminate_backend hitting CHECK_FOR_INTERRUPTS (:544) mid-batch does leave earlier sequences' values applied while their catalog rows revert to 'i' with NULL srsublsn (REFRESH SEQUENCES writes InvalidXLogRecPtr, subscriptioncmds.c:1392-1393). Nothing in git history changed this. However, the behavior is not a defect: (1) SetSequence IS setval()'s implementation, and setval's non-transactionality is explicitly documented (doc/src/sgml/func/func-sequence.sgml:199-201: changes "are immediately visible to other transactions, and are not undone if the calling transaction rolls back") — the feature inherits documented sequence semantics; (2) "failed sync => no persisted mutation" was never this feature's contract even absent aborts: report_sequence_errors() raises the overall failure ERROR only AFTER all batches have committed (sequencesync.c:670-673), so a sync failing on one mismatched/missing/no-permission sequence has already permanently committed publisher values AND READY state for every other sequence — the finding's within-batch-abort case (values applied, state 'i') is a strictly milder variant of designed behavior (values applied, state 'r', command reports failure); (3) the leftover 'i' state is the conservative, correct-by-contract state: the apply worker respawns the sequencesync worker (ProcessSequencesForSync, sequencesync.c:96-140; syncutils.c wal_retrieve_retry_interval throttling) and re-copy via SetSequence is idempotent, a retry loop the docs explicitly describe (logical-replication.sgml:1824-1829); the ordering (non-rollbackable value first, transactional catalog second) is the only crash-safe ordering for a non-MVCC object — the reverse would produce READY-without-copy, a real data-loss bug; (4) the claimed harm ("regressing a locally-advanced sequence" to the publisher value) is exactly what the user commanded — a successful REFRESH SEQUENCES installs the same value — and no wrong final value can persist while the subscription exists; (5) no documentation promises atomicity or rollback of sequence values on sync failure, so there is no documented-vs-actual divergence, and pg_subscription_rel showing 'i' is accurate per its meaning ("needs synchronization"; resync is pending and harmless). The tablesync comparison establishes no contract: heap data is MVCC-rollbackable, sequence data fundamentally is not. A report of this to pgsql-hackers would be closed as works-as-intended/inherent setval semantics. The mechanism is real but it is intended, documented-primitive, self-healing behavior — not a user-visible defect.
## Appendix C: provenance
Produced by a 45-agent workflow (20 finder lenses over master @ a8c2547 -> dedup/rank of 63 raw findings -> 23 adversarial verifiers -> synthesis). All verification is static code tracing; no repro was executed. Per-agent transcripts: ~/.claude/projects/-home-nm-src-pg-postgresql/3a1250f2-41e8-46fd-afe1-08ff8e4dace0/subagents/workflows/wf_e3d25a7b-189/journal.jsonl
Attachments:
[text/plain] test-refresh-race-sequencesync-v0.patch (10.4K, ../../[email protected]/2-test-refresh-race-sequencesync-v0.patch)
download | inline diff:
commit fe331b9 (HEAD -> seq-refresh-sequences-race)
Author: Noah Misch <[email protected]>
AuthorDate: Thu Jul 9 19:39:45 2026 +0000
Commit: Noah Misch <[email protected]>
CommitDate: Thu Jul 9 19:42:17 2026 +0000
Add test revealing REFRESH SEQUENCES race with sequencesync worker.
ALTER SUBSCRIPTION ... REFRESH SEQUENCES resets all sequence entries in
pg_subscription_rel to INIT (AlterSubscription_refresh_seq) without
stopping or fencing an in-flight sequencesync worker. The worker
fetches publisher values before taking any local lock, and
copy_sequence() later applies them and marks each sequence READY
without rechecking the entry's state. A batch whose values were
fetched before the refresh therefore overwrites the INIT reset
afterwards: the sequences report READY while holding pre-refresh
publisher values, with no error, no warning, and no pending
resynchronization. A user following the documented pre-failover
procedure (REFRESH SEQUENCES, wait for READY) can then fail over to a
subscriber whose sequences are behind the publisher, yielding duplicate
sequence values.
The test constructs the schedule deterministically: it blocks the
worker's batch query on the publisher via AccessExclusiveLock on a
sequence, blocks local application via ShareRowExclusiveLock on the
sequences on the subscriber, and runs REFRESH SEQUENCES inside the
fetch-to-apply window.
The final two assertions check that sequences reporting READY reflect
every value published before REFRESH SEQUENCES was issued. They
currently fail, demonstrating the defect.
Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01YKt6fYFvAnMUMyavJmmueE
---
src/test/subscription/meson.build | 1 +
.../subscription/t/039_sequences_refresh_race.pl | 184 +++++++++++++++++++++
2 files changed, 185 insertions(+)
diff --git a/src/test/subscription/meson.build b/src/test/subscription/meson.build
index e71e95c..067fca2 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_sequences_refresh_race.pl',
't/100_bugs.pl',
],
},
diff --git a/src/test/subscription/t/039_sequences_refresh_race.pl b/src/test/subscription/t/039_sequences_refresh_race.pl
new file mode 100644
index 0000000..5173231
--- /dev/null
+++ b/src/test/subscription/t/039_sequences_refresh_race.pl
@@ -0,0 +1,184 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+# Test that ALTER SUBSCRIPTION ... REFRESH SEQUENCES is not silently
+# overwritten by an in-flight sequencesync worker.
+#
+# The sequencesync worker fetches sequence values from the publisher and
+# only afterwards applies them and marks the sequences READY, without
+# rechecking pg_subscription_rel state. REFRESH SEQUENCES resets all
+# sequences to INIT without stopping or fencing a running worker. Hence a
+# batch whose values were fetched before the refresh can mark sequences
+# READY afterwards, leaving them holding pre-refresh values with no error,
+# no warning, and no pending resynchronization. A user following the
+# documented pre-failover procedure (run REFRESH SEQUENCES, wait for READY)
+# can then fail over to a subscriber whose sequences are behind the
+# publisher, producing duplicate sequence values.
+#
+# This test constructs that schedule deterministically:
+#
+# 1. Hold AccessExclusiveLock on a sequence on the publisher, so the
+# worker's remote batch query blocks inside pg_get_sequence_data()
+# after the worker has committed its scan of pg_subscription_rel.
+# 2. While the fetch is blocked, take ShareRowExclusiveLock on all
+# sequences on the subscriber, so that after the fetch completes the
+# worker blocks at its first try_table_open(), i.e. after fetching
+# values but before updating any pg_subscription_rel row.
+# 3. Release the publisher lock; the worker's fetch completes with the old
+# values and the worker blocks on the subscriber locks.
+# 4. Advance the sequences on the publisher, then run
+# ALTER SUBSCRIPTION ... REFRESH SEQUENCES; it resets both sequences to
+# INIT and commits, not blocked by the in-flight worker.
+# 5. Release the subscriber locks. The worker applies the stale values
+# and flips both sequences INIT -> READY.
+#
+# On an unpatched server the final assertions fail: all sequences report
+# READY but hold the values fetched before the refresh.
+#
+# Note for whoever fixes the race: this choreography assumes REFRESH
+# SEQUENCES continues to complete without waiting for an in-flight
+# sequencesync worker (as it does today). If the fix instead makes the
+# command block until a running worker exits, step 4 will wait behind the
+# sequence locks taken in step 2 and the test will hang there rather than
+# fail; the schedule would need reshaping for a fix of that shape.
+
+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;
+
+# Identical sequence definitions on both nodes.
+my $ddl = qq(
+ CREATE SEQUENCE regress_seq1;
+ CREATE SEQUENCE regress_seq2;
+);
+$node_publisher->safe_psql('postgres', $ddl);
+$node_subscriber->safe_psql('postgres', $ddl);
+
+# Consume some sequence values on the publisher.
+$node_publisher->safe_psql(
+ 'postgres', qq(
+ SELECT nextval('regress_seq1') FROM generate_series(1, 100);
+ SELECT nextval('regress_seq2') FROM generate_series(1, 100);
+));
+
+$node_publisher->safe_psql('postgres',
+ "CREATE PUBLICATION regress_seq_pub FOR ALL SEQUENCES");
+
+# Create the subscription disabled, so the locks below can be positioned
+# before the sequencesync worker starts. The replication slot is created
+# here, which must precede the publisher-side open transaction below
+# because slot creation waits for concurrent transactions to finish.
+my $publisher_connstr = $node_publisher->connstr . ' dbname=postgres';
+$node_subscriber->safe_psql('postgres',
+ "CREATE SUBSCRIPTION regress_seq_sub CONNECTION '$publisher_connstr' PUBLICATION regress_seq_pub WITH (enabled = false)"
+);
+
+my $result = $node_subscriber->safe_psql('postgres',
+ "SELECT count(*) FROM pg_subscription_rel WHERE srsubstate = 'i'");
+is($result, '2', 'both sequences start in INIT state');
+
+# Block the sequencesync worker's batch query on the publisher: an
+# uncommitted DROP SEQUENCE holds AccessExclusiveLock, on which the
+# pg_get_sequence_data() call in the batch query will wait.
+my $pub_session = $node_publisher->background_psql('postgres');
+$pub_session->query_safe(
+ qq(
+ BEGIN;
+ DROP SEQUENCE regress_seq1;
+));
+
+$node_subscriber->safe_psql('postgres',
+ "ALTER SUBSCRIPTION regress_seq_sub ENABLE");
+
+# Wait until the worker's batch query is blocked on the publisher. At
+# this point the worker has committed the transaction that scanned
+# pg_subscription_rel and holds no locks on the subscriber.
+$node_publisher->poll_query_until(
+ 'postgres', qq(
+ SELECT EXISTS (
+ SELECT 1 FROM pg_locks
+ WHERE relation = 'regress_seq1'::regclass AND NOT granted);
+)) or die "timed out waiting for sequencesync worker to block on publisher";
+
+# Take locks conflicting with the worker's try_table_open() on both
+# sequences, so the worker will block after its fetch completes but before
+# it updates any pg_subscription_rel row, whatever order it processes the
+# sequences in. The ALTERs are rolled back later, so the definitions stay
+# identical to the publisher's.
+my $sub_session = $node_subscriber->background_psql('postgres');
+$sub_session->query_safe(
+ qq(
+ BEGIN;
+ ALTER SEQUENCE regress_seq1 MINVALUE 1;
+ ALTER SEQUENCE regress_seq2 MINVALUE 1;
+));
+
+# Release the publisher lock: the fetch completes with the current
+# publisher values, then the worker blocks on the subscriber locks.
+$pub_session->query_safe("ROLLBACK");
+$pub_session->quit;
+
+$node_subscriber->poll_query_until(
+ 'postgres', qq(
+ SELECT EXISTS (
+ SELECT 1 FROM pg_locks
+ WHERE relation IN ('regress_seq1'::regclass, 'regress_seq2'::regclass)
+ AND NOT granted);
+)) or die "timed out waiting for sequencesync worker to block on subscriber";
+
+# The values held by the blocked worker now become stale.
+$node_publisher->safe_psql(
+ 'postgres', qq(
+ SELECT nextval('regress_seq1') FROM generate_series(1, 100);
+ SELECT nextval('regress_seq2') FROM generate_series(1, 100);
+));
+
+# Request resynchronization of all sequences. This resets both sequences
+# to INIT and commits; it does not wait for the in-flight worker.
+$node_subscriber->safe_psql('postgres',
+ "ALTER SUBSCRIPTION regress_seq_sub REFRESH SEQUENCES");
+
+$result = $node_subscriber->safe_psql('postgres',
+ "SELECT count(*) FROM pg_subscription_rel WHERE srsubstate = 'i'");
+is($result, '2', 'REFRESH SEQUENCES reset both sequences to INIT');
+
+# Release the subscriber locks, letting the worker proceed.
+$sub_session->query_safe("ROLLBACK");
+$sub_session->quit;
+
+# Wait until the subscription reports all sequences synchronized.
+$node_subscriber->poll_query_until('postgres',
+ "SELECT count(*) = 0 FROM pg_subscription_rel WHERE srsubstate <> 'r'")
+ or die "timed out waiting for sequences to reach READY state";
+
+# All sequences report READY, so they must not predate the last REFRESH
+# SEQUENCES command: every value published before that command was issued
+# must be reflected on the subscriber. On an unpatched server these fail,
+# with the subscriber sequences left at the values fetched before the
+# refresh.
+my $pub_seq1 = $node_publisher->safe_psql('postgres',
+ "SELECT last_value, is_called FROM regress_seq1");
+my $sub_seq1 = $node_subscriber->safe_psql('postgres',
+ "SELECT last_value, is_called FROM regress_seq1");
+is($sub_seq1, $pub_seq1,
+ 'READY regress_seq1 reflects publisher values from before REFRESH SEQUENCES'
+);
+
+my $pub_seq2 = $node_publisher->safe_psql('postgres',
+ "SELECT last_value, is_called FROM regress_seq2");
+my $sub_seq2 = $node_subscriber->safe_psql('postgres',
+ "SELECT last_value, is_called FROM regress_seq2");
+is($sub_seq2, $pub_seq2,
+ 'READY regress_seq2 reflects publisher values from before REFRESH SEQUENCES'
+);
+
+done_testing();
[text/plain] sequence-logrep-defects.md (52.6K, ../../[email protected]/3-sequence-logrep-defects.md)
download | inline:
# Logical replication of sequences: user-visible defects in master
**Scope:** defects introduced by commits f0b3573c3, 5509055d6, 55cefadde (and still present through follow-up fixes), verified against master @ a8c2547. All file:line references are to that tree.
## Executive summary
An adversarial audit of the PG19 sequence-replication feature identified 20 user-visible defects; two pairs share a single root cause and are merged below, leaving 18 items. Every item is **CONFIRMED** — the mechanism was traced end-to-end in source by an independent verifier — but none has been executed as a runtime repro, so the repro sketches should be run before relying on exact symptoms; there are no PLAUSIBLE-grade items in this report. The dominant pattern is missing coordination around the new shared sequencesync worker: no ALTER SUBSCRIPTION path stops or fences it, and it never rechecks subscription or relation state after startup, yielding (worst case) sequences silently marked READY with pre-refresh publisher values in the documented pre-failover `REFRESH SEQUENCES` workflow — i.e., duplicate sequence values after failover, the exact hazard the feature exists to prevent. A second pattern is single-object failures escalating to subscription-wide outages: because one worker serves all sequences and several failure paths bypass the per-sequence warning framework, one misconfigured sequence, a read-only default, a lock-table squeeze, or an older publisher produces an unbounded 5-second error/relaunch loop (or full auto-disable under `disable_on_error`). The remainder are internal XX000 errors reachable from ordinary DDL concurrency, misleading diagnostics, and monitoring/documentation never updated for the new worker type and relation kind.
## Index
| # | Finding | Severity | Confidence | Anchor |
|---|---------|----------|------------|--------|
| 1 | Refresh paths race the in-flight sequencesync worker: stale values marked READY / XX000 + auto-disable *(merged: 2 findings)* | High | CONFIRMED | src/backend/commands/subscriptioncmds.c:1392, :1336 |
| 2 | `default_transaction_read_only=on` permanently blocks sequence sync via setval() read-only check | Medium | CONFIRMED | src/backend/replication/logical/sequencesync.c:407 |
| 3 | `run_as_owner=false`: un-SET-ROLE-able sequence owner kills the shared worker with an unattributed error | Medium | CONFIRMED | src/backend/replication/logical/sequencesync.c:387 |
| 4 | Gathering scan holds RowExclusiveLock on every INIT relation in one unbounded transaction: blocking + lock-table exhaustion loop | Medium | CONFIRMED | src/backend/replication/logical/sequencesync.c:718 |
| 5 | Sequencesync worker starved by tablesync workers; silent deferral of REFRESH SEQUENCES | Medium | CONFIRMED | src/backend/replication/logical/syncutils.c:127 |
| 6 | No publisher-version guard: pre-PG19 publisher yields perpetual cryptic protocol-violation loop | Medium | CONFIRMED | src/backend/replication/logical/sequencesync.c:529 |
| 7 | `has_sequence_privilege()` NULL on concurrent publisher drop: Assert crash / misreported as permission failure | Medium | CONFIRMED | src/backend/replication/logical/sequencesync.c:298 |
| 8 | Permission-denied sequences also listed as "missing on publisher" in mixed batches | Medium | CONFIRMED | src/backend/replication/logical/sequencesync.c:657 |
| 9 | ALTER SUBSCRIPTION ... DISABLE never stops a running sequencesync worker | Low | CONFIRMED | src/backend/replication/logical/sequencesync.c:447 |
| 10 | REFRESH SEQUENCES vs concurrent subscriber DROP SEQUENCE: internal XX000 errors | Low | CONFIRMED | src/backend/commands/subscriptioncmds.c:1387 |
| 11 | Sequence-removal loop placed after irreversible tablesync slot drops, violating stated invariant | Low | CONFIRMED | src/backend/commands/subscriptioncmds.c:1322 |
| 12 | Batch query mixes MVCC definition columns with live `last_value`: validation bypass under concurrent ALTER SEQUENCE | Low | CONFIRMED | src/backend/replication/logical/sequencesync.c:517 |
| 13 | REFRESH SEQUENCES emits "requested copy_data" warning for an option the command does not accept | Low | CONFIRMED | src/backend/commands/subscriptioncmds.c:3272 |
| 14 | psql tab-completion regression after `REFRESH PUBLICATION` (offers publication names / nothing instead of `WITH (`) | Low | CONFIRMED | src/bin/psql/tab-complete.in.c:2356 |
| 15 | pg_stat_subscription row for sequencesync worker: fabricated message times, NULL columns undocumented *(merged: 2 findings)* | Low | CONFIRMED | src/backend/replication/logical/worker.c:5980; doc/src/sgml/monitoring.sgml:2336 |
| 16 | catalogs.sgml `srsublsn` description wrong for sequence rows | Low | CONFIRMED | doc/src/sgml/catalogs.sgml:8894 |
| 17 | Docs never state sequence sync requires a PG19+ publisher; REFRESH SEQUENCES silently no-ops | Low | CONFIRMED | doc/src/sgml/logical-replication.sgml:2359 |
| 18 | Restrictions bullet still claims only tables can be replicated | Low | CONFIRMED | doc/src/sgml/logical-replication.sgml:2401 |
---
### 1. Refresh paths race the in-flight sequencesync worker (High)
**Merged finding.** Two audit findings are merged here because they share one root cause: no ALTER SUBSCRIPTION refresh path stops or fences a running sequencesync worker (the table path explicitly stops tablesync workers, subscriptioncmds.c:1257, and its removal path documents that its `pg_subscription_rel` lock exists to freeze rel states, :1333-1334), and the worker never re-validates a row between deciding to sync it and writing it.
**Defect.** (A, high) `ALTER SUBSCRIPTION ... REFRESH SEQUENCES` can be silently overwritten by the worker's in-flight batch: up to 100 sequences end up `srsubstate='r'` holding publisher values fetched *before* the refresh, with no error, no warning, and no pending resync. (B, medium) The PUBLICATION refresh forms (`REFRESH PUBLICATION`, `SET/ADD/DROP PUBLICATION`) remove de-published sequence rows out from under the worker, which then raises internal XX000 `subscription relation %u in subscription %u does not exist` after having already applied a non-transactional value overwrite; with `disable_on_error=true` the entire subscription (tables included) is disabled.
**Mechanism.** (A) `AlterSubscription_refresh_seq()` resets every sequence row to `SUBREL_STATE_INIT` (subscriptioncmds.c:1392) with no worker interlock. The worker fetches publisher values with `walrcv_exec` before holding any relevant lock (sequencesync.c:529), then `copy_sequence()` unconditionally applies `SetSequence()` (:407) and `UpdateSubscriptionRelState(..., SUBREL_STATE_READY, ...)` (:416) without rechecking `srsubstate`. `UpdateSubscriptionRelState`'s `LockSharedObject` waits out the ALTER and absorbs its invalidations (pg_subscription.c:405; lmgr.c:1102), so the worker *cleanly* flips the freshly-reset rows to READY with stale values; `FetchRelationStates` keys resync off non-READY rows, so the loss is permanent and invisible. (B) The removal loop calls `RemoveSubscriptionRel()` (subscriptioncmds.c:1336) without `logicalrep_worker_stop`; the worker's subsequent `UpdateSubscriptionRelState` hits `elog(ERROR, ...)` at pg_subscription.c:413-415 — after `SetSequence` (non-transactional, like setval) already overwrote the just-de-published local sequence. PG_CATCH at sequencesync.c:803-819 then either error-loops or runs `DisableSubscriptionAndExit`.
**Trigger.** (A) Any `REFRESH SEQUENCES` committing while a sequencesync batch is between its publisher fetch and its first catalog update — probabilistic at every batch boundary under continuous publisher `nextval()` traffic; deterministic with a lock stall. (B) `DROP PUBLICATION pub_seq` (default `refresh=true`) or `REFRESH PUBLICATION` while initial sequence sync is in flight.
**Repro sketch.** Two nodes, 101 sequences, `FOR ALL SEQUENCES` publication, subscription created. Subscriber session A after the worker's initial scan: `BEGIN; ALTER SEQUENCE s101 RESTART 1;` (ShareRowExclusiveLock stalls the worker at `try_table_open`, sequencesync.c:336, *after* it fetched batch-2 values). Publisher: advance `nextval('s101')` repeatedly. Subscriber session B: `ALTER SUBSCRIPTION sub REFRESH SEQUENCES;` (succeeds, all rows → 'i'). Session A: `ROLLBACK;` → worker marks s101 'r' with the pre-refresh value; failover now yields duplicate sequence values. For (B): 300 sequences + `disable_on_error=true`, then `ALTER SUBSCRIPTION sub DROP PUBLICATION pub_seq;` mid-sync → XX000 in the log and the subscription disabled.
**Fix direction.** Stop (or fence) sequencesync workers in all refresh paths, as the tablesync path does, and/or make `copy_sequence()` re-verify under lock that the row still exists in the expected pre-sync state before writing SetSequence/READY, discarding stale fetched values.
---
### 2. `default_transaction_read_only=on` permanently blocks sequence sync (Medium)
**Defect.** On a subscriber with `default_transaction_read_only=on` — the read-only-subscriber deployment logical-replication.sgml itself describes — tables copy and apply normally, but sequences never sync: the worker fails every `wal_retrieve_retry_interval` with the misleading `cannot execute setval() in a read-only transaction` (25006) for a setval() the user never ran; rows stay 'i' forever, `seq_sync_error_count` climbs, and `disable_on_error=true` disables the whole subscription.
**Mechanism.** `copy_sequence()` applies values via `SetSequence()` (sequencesync.c:407), which runs `PreventCommandIfReadOnly("setval()")` for permanent sequences (sequence.c:976-977). Batch transactions are plain `StartTransactionCommand` (sequencesync.c:462), inheriting `XactReadOnly = DefaultXactReadOnly` (xact.c:2172); worker GUC setup overrides only session_replication_role/search_path/synchronous_commit (worker.c:5798-5810). Every other subscriber write path bypasses read-only checks: tablesync calls `BeginCopyFrom`/`CopyFrom` directly (tablesync.c:1209-1213; the only COPY check lives in `DoCopy`, copy.c:365) and apply uses `ExecSimpleRelation*` with no check — so sequences are uniquely broken.
**Trigger.** Set `default_transaction_read_only=on` in subscriber postgresql.conf (or per-database); subscribe to a `FOR ALL TABLES, ALL SEQUENCES` publication.
**Repro sketch.** Publisher: table t + sequence s, `nextval('s')`, publication for both. Subscriber (GUC on; use a session-local `SET default_transaction_read_only=off` for the DDL): create matching objects, `CREATE SUBSCRIPTION`. Observe t replicating while the log repeats the setval error and s stays 'i'. Undocumented workaround exists (`ALTER ROLE <subscription owner> SET default_transaction_read_only = off`), but nothing hints at it.
**Fix direction.** Have the sequencesync worker's transactions clear `XactReadOnly` (or exempt this internal caller from the setval() read-only check), matching the implicit exemption tablesync/apply already enjoy.
---
### 3. `run_as_owner=false`: one un-SET-ROLE-able sequence owner stalls all sequence sync, unattributed (Medium)
**Defect.** With the default `run_as_owner=false`, one sequence whose owner the subscription owner cannot `SET ROLE` to makes the shared worker die with `role "X" cannot SET ROLE to "Y"` (42501) — an error that never names the sequence — permanently preventing *all* remaining sequences (including fully-compliant ones) from leaving state 'i'; `disable_on_error=true` disables the whole subscription.
**Mechanism.** `copy_sequence()` calls `SwitchToUntrustedUser(seqowner, ...)` (sequencesync.c:387) *before* the ACL check; `SwitchToUntrustedUser` raises ERROR on membership failure (usercontext.c:40-45). Unlike the `pg_class_aclcheck` failure two lines later — classified `COPYSEQ_SUBSCRIBER_INSUFFICIENT_PERM` (:396) and reported per-sequence by name via `report_sequence_errors()` (:201-224) while the batch continues — this ERROR is uncaught inside the batch loop (transaction spans :462-:647), rolls back batch-mates' READY updates, kills the worker before later batches, and repeats identically on every relaunch. No `ErrorContextCallback` exists in sequencesync.c, so the log never identifies the culprit. Aggravating: in default mode the graceful path checks the *owner's* privileges on their own sequence (essentially always true), so the warning framework is nearly dead code while the realistic failure takes the worker-killing path; 036_sequences.pl never tests this scenario.
**Trigger.** Subscription owned by non-superuser `bob`; any subscribed sequence owned by `alice` where `bob` lacks SET ROLE membership.
**Repro sketch.** Subscriber: sequences `s_ok` (owner bob) and `s_alice` (owner alice, bob not a member); publisher publishes both `FOR ALL SEQUENCES`; bob creates the subscription. Both sequences stay 'i' forever; log repeats the role error with no sequence name.
**Fix direction.** Classify the SET ROLE failure per-sequence like the other permission failures (new COPYSEQ_* code + warning naming the sequence) instead of letting it kill the worker, and add error context identifying the sequence being synced.
---
### 4. Gathering scan holds a lock on every INIT relation in one unbounded transaction (Medium)
**Defect.** The worker's initial catalog scan opens **every** `srsubstate='i'` relation — tables awaiting tablesync included — with `try_table_open(..., RowExclusiveLock)` *before* checking relkind, in a single transaction, retaining all locks until commit. Consequences: (a) one conflicting lock on any INIT relation stalls all sequence sync indefinitely while the worker retains everything already scanned; (b) with thousands of INIT sequences the transaction exhausts the shared lock table, producing a permanent `out of shared memory` / `increase max_locks_per_transaction` retry loop every 5 s with zero progress, transiently draining the lock pool for unrelated sessions each attempt.
**Mechanism.** `LogicalRepSyncSequences()`: one transaction (sequencesync.c:691-752); `try_table_open` at :718 precedes the relkind check at :725; both close paths use `NoLock` (:727, :745). `try_relation_open` blocks on the lock (relation.c:96-97 — "try" covers only nonexistence). An ERROR here precedes any READY update, so the INIT set never shrinks; relaunch every `wal_retrieve_retry_interval` (syncutils.c:127-147). The locks are demonstrably unnecessary — `FetchRelationStates` distinguishes sequences lock-free via `get_rel_relkind()` (syncutils.c:243) — and `copy_sequences()` already caps itself at 100 locks/transaction. Verifier corrections to the original claim: fresh CREATE SUBSCRIPTION cannot reach the exhaustion (the foreground command's own per-relation locking fails first); the reachable route is `ALTER SUBSCRIPTION ... REFRESH SEQUENCES` (flips all sequences to INIT with zero relation locks, subscriptioncmds.c:1360-1406) combined with a lock-pool squeeze, e.g. a subscriber later restarted with lower `max_connections` — and `max_locks_per_transaction` is PGC_POSTMASTER, so recovery requires a restart or dropping/disabling the subscription.
**Trigger.** Blocking: an open `BEGIN; ALTER SEQUENCE s1 RESTART ...;` (ShareRowExclusiveLock, sequence.c:450) when the scan runs. Exhaustion: ~5000 subscribed sequences, subscriber restarted with small `max_connections`, then `REFRESH SEQUENCES`.
**Repro sketch.** Blocking: 2 sequences subscribed and 'r'; session A `BEGIN; ALTER SEQUENCE s1 RESTART 100;` (leave open); `ALTER SUBSCRIPTION sub REFRESH SEQUENCES;` → worker blocks at `try_table_open(s1)`, s2 also never syncs. Exhaustion: 5000 sequences synced under `max_connections=100`; restart subscriber with `max_connections=20`; `REFRESH SEQUENCES` → perpetual out-of-shared-memory loop; `disable_on_error=true` disables the subscription on first failure.
**Fix direction.** Drop the relation locks from the gathering scan (filter by `get_rel_relkind()` as `FetchRelationStates` does), deferring per-sequence locking to the already-batched copy phase.
---
### 5. Sequencesync worker starved by tablesync workers; REFRESH SEQUENCES silently deferred (Medium)
**Defect.** While ≥ `max_sync_workers_per_subscription` tables are actively copying, the sequencesync worker is never launched — with no log message, no worker row, and no pacing state — so sequences stay 'i' for the full duration of a bulk table copy, even after an explicit `REFRESH SEQUENCES`. A DBA running the documented pre-failover `REFRESH SEQUENCES` during a large copy and then promoting gets never-synced sequences.
**Mechanism.** Both worker types share one budget (`logicalrep_sync_worker_count`, launcher.c:949). `ProcessSyncingRelations()` always runs tables before sequences (syncutils.c:173-175); the table loop launches immediately for any never-started table (`last_start_time==0`, tablesync.c:566-567), retaking every freed slot in the same pass; `launch_sync_worker()` then returns silently at the cap (syncutils.c:127). sequencesync.c:138 is the *only* launch site — the launcher never starts one. Verifier caveats: starvation ends when pending tables drop below the limit (not literally indefinite; failing tables' throttled relaunches leave windows), and logical-replication.sgml:1814-1819 does disclose the budget cap — but the strict tables-first priority is undocumented, and config.sgml:5695-5700's "one additional worker ... for sequence synchronization" implies provisioning +1 helps, which the greedy table loop defeats at any GUC value.
**Trigger.** Default `max_sync_workers_per_subscription=2`, ≥2 large tables mid-copy, then `REFRESH SEQUENCES`.
**Repro sketch.** Publisher: 3 tables × 10M rows + 1 advanced sequence, publications `FOR ALL TABLES` and `FOR ALL SEQUENCES`. Subscriber: subscribe to both; while ≥2 tablesync workers run, `ALTER SUBSCRIPTION sub REFRESH SEQUENCES;` and poll — sequence rows stay 'i' and `pg_stat_subscription` shows no `sequence synchronization` worker until the copy tail.
**Fix direction.** Reserve a slot for (or prioritize) sequence sync when INIT sequences exist — or at minimum log the deferral — and align config.sgml with the actual shared-budget behavior.
---
### 6. No publisher-version guard: pre-PG19 publisher causes a perpetual, cryptic error loop (Medium)
**Defect.** A subscription with registered sequences repointed at a pre-PG19 publisher (e.g. upgrade rollback via `ALTER SUBSCRIPTION ... CONNECTION`, or a DNS/VIP flip mid-sync) enters an endless 5-second error/relaunch loop: against PG18, `ERROR: invalid query response DETAIL: Expected 11 fields, got 10 fields.` (SQLSTATE 08P01, suggesting protocol corruption, not a version problem); against ≤PG17, `function pg_get_sequence_data(oid) does not exist`. Sequences stay 'i'; `disable_on_error=true` disables the whole subscription.
**Mechanism.** sequencesync.c has zero `walrcv_server_version()` checks (the only ≥190000 gates in the sync paths are tablesync.c:802, 1004); `copy_sequences()` unconditionally sends the batch query (:517-527) expecting REMOTE_SEQ_COL_COUNT=11 columns at :529. PG18's `pg_get_sequence_data()` exists but returns 2 output columns (page_lsn added by b93172c for PG19), so the query succeeds remotely with 10 fields and `libpqrcv_processTuples` errors (libpqwalreceiver.c:1083-1088). The broken state is reachable because sequence rows persist across `ALTER SUBSCRIPTION ... CONNECTION`, and `AlterSubscription_refresh_seq` *succeeds* against an old publisher (its origin check returns early for <190000, subscriptioncmds.c:3196-3198), reporting success and then unleashing the storm. `REFRESH PUBLICATION` incidentally recovers by removing the rows, but nothing in the error suggests that.
**Trigger.** PG19→PG19 subscription with synced sequences; repoint the connection at a PG18 host holding the slot; `REFRESH SEQUENCES`.
**Repro sketch.** Node A (PG19) and node B (PG18) with identical schema; subscribe to A (`FOR ALL SEQUENCES`), wait for 'r'; `ALTER SUBSCRIPTION sub1 CONNECTION 'host=B ...'; ALTER SUBSCRIPTION sub1 REFRESH SEQUENCES;` → 5-second loop of worker-start LOG + 08P01 ERROR.
**Fix direction.** Version-gate the sequencesync launch/copy path (and `REFRESH SEQUENCES`) with a clear "publisher (version N) does not support sequence synchronization" error or warning instead of retrying a query that can never succeed.
---
### 7. `has_sequence_privilege()` NULL on concurrent publisher DROP: Assert crash / wrong diagnosis (Medium)
**Defect.** A published sequence dropped on the publisher while the batch query runs yields a result row with NULL in the `has_sequence_privilege` column; assert-enabled subscribers hit `Assert(!isnull)` (sequencesync.c:297-298) — TRAP → SIGABRT → crash-restart of the sequencesync worker; production builds decode NULL as false and misreport the drop as `insufficient privileges on publisher sequence (...)` with a bogus GRANT SELECT hint. This re-introduces the concurrent-drop hazard 1ba3eee fixed for the data columns: d4a657b added the privilege column with an unconditional Assert.
**Mechanism.** The batch query (sequencesync.c:517-527) joins pg_class/pg_sequence under the query snapshot, but `has_sequence_privilege_id()` uses `get_rel_relkind()` on the current catalog snapshot and returns SQL NULL for a vanished relation (acl.c:2246-2248). The row is still emitted (old snapshot), and the LATERAL `pg_get_sequence_data()` — which locks the relation and absorbs the drop's invalidations first (lmgr.c:134-137; commit ordering xact.c:2475/2480) — makes the NULL deterministic, not a narrow race: an uncommitted DROP holds AccessExclusiveLock, the sync query blocks inside it, and COMMIT triggers the condition every time. With NULL data + false privilege, production takes the `COPYSEQ_PUBLISHER_INSUFFICIENT_PERM` path (:300-303) instead of the concurrent-drop `COPYSEQ_SKIPPED` path.
**Trigger.** Publisher-side `DROP SEQUENCE` committing while the batch SELECT executes.
**Repro sketch.** Two sequences subscribed and 'r'. Publisher session A: `BEGIN; DROP SEQUENCE s2;` (hold). Subscriber: `ALTER SUBSCRIPTION sub REFRESH SEQUENCES;` (batch query blocks on A's lock). Publisher: `COMMIT;` → assert build: TRAP at sequencesync.c:298; production: wrong privilege WARNING + sync-failure ERROR.
**Fix direction.** Treat a NULL privilege column as a concurrently-dropped sequence (fold into the existing `COPYSEQ_SKIPPED` handling) rather than asserting non-null.
---
### 8. Permission-denied sequences double-reported as "missing on publisher" (Medium)
**Defect.** When one batch contains both a genuinely missing sequence and a publisher-permission-denied sequence, the perm-denied sequence is listed in *both* warnings: `insufficient privileges on publisher sequence ("public.s2")` and `missing sequences on publisher ("public.s1", "public.s2")` — contradictory diagnoses for an object that exists (the worker even received a row for it). This is the residual gap in d4a657b, whose stated purpose was exactly to stop permission failures being reported as missing.
**Mechanism.** The perm-denied early return (sequencesync.c:300-303) fires before `found_on_pub = true` is set (:334), so perm-denied entries keep `found_on_pub=false` (palloc0 at :737). A genuinely missing sequence produces no row (inner joins, :517-527), making `batch_missing_count > 0` (:633-637), which enables the post-commit scan (:649-660) that appends *every* `!found_on_pub` entry to the missing list (:657) with no dedup in `report_sequence_errors()` (:226-251). d4a657b's own test (036_sequences.pl:228-262) recreated the dropped sequence before the permission test, so the mixed batch was never exercised.
**Trigger.** Same ≤100-sequence batch containing one sequence dropped on the publisher and one with SELECT revoked from the subscription's connection role, then `REFRESH SEQUENCES` (which never prunes publisher-dropped sequences).
**Repro sketch.** Both nodes: s1, s2; subscribe, wait for 'r'. Publisher: `REVOKE SELECT ON SEQUENCE s2 FROM repuser; DROP SEQUENCE s1;`. Subscriber: `ALTER SUBSCRIPTION sub REFRESH SEQUENCES;` → the contradictory warning pair repeats on every relaunch.
**Fix direction.** Set `found_on_pub` before the permission early-return (or exclude entries already classified perm-denied from the missing scan) so each sequence appears in exactly one list.
---
### 9. ALTER SUBSCRIPTION ... DISABLE never stops a running sequencesync worker (Low)
**Defect.** After `DISABLE`, the apply worker exits promptly but the sequencesync worker keeps starting new batch transactions, overwriting subscriber sequence values and flipping `pg_subscription_rel` rows to 'r' after the disable — for minutes normally, indefinitely if lock-blocked — contradicting alter_subscription.sgml's "stopping the logical replication worker" (:270-278). A `nextval()` run locally post-DISABLE can be clobbered back to an older publisher value.
**Mechanism.** The worker's entire lifetime (SequenceSyncWorkerMain → ... → `copy_sequences` loop, sequencesync.c:447-668) contains no `maybe_reread_subscription()` (call sites are only worker.c:740/2565/4183 and applyparallelworker.c:283) and never reads `MySubscriptionValid`/`enabled` after startup; DISABLE only writes `subenabled=false` and signals nothing (`logicalrep_worker_stop` is called only at subscriptioncmds.c:1257 and DropSubscription :2636); no exit path stops it (launcher.c:822-842 handles only parallel apply workers). Related to item 1's root cause (worker never revalidates anything), but the trigger is subscription-level state rather than per-relation rows, so reported separately; a batch-boundary recheck would address both.
**Trigger.** `DISABLE` while a sync of many sequences is in flight, or while the worker is blocked at `try_table_open` (:336/:718) behind an open `ALTER SEQUENCE`.
**Repro sketch.** 300 sequences; subscriber session A holds `BEGIN; ALTER SEQUENCE s150 RESTART 1;`; create subscription; once the worker blocks, `ALTER SUBSCRIPTION sub DISABLE;` (apply worker exits, sequencesync worker persists); `COMMIT;` in A → post-disable, sequences s101-s300 change values and READY count climbs to 300.
**Fix direction.** Recheck subscription validity/enabled at each batch boundary in `copy_sequences()` (and consider having DISABLE stop sync workers explicitly).
---
### 10. REFRESH SEQUENCES vs concurrent subscriber DROP SEQUENCE: internal XX000 errors (Low)
**Defect.** Ordinary DDL concurrency — `REFRESH SEQUENCES` racing a subscriber-side `DROP SEQUENCE` — ends with one command failing on an internal elog: XX000 `subscription relation %u in subscription %u does not exist`, `tuple concurrently deleted`, or the DROP failing `tuple concurrently updated`; assert builds can additionally trip the relkind Assert in `GetSubscriptionRelations` (pg_subscription.c:666). Transient (retry succeeds) but corruption-looking.
**Mechanism.** `AlterSubscription_refresh_seq()` builds its list (subscriptioncmds.c:1387) holding no lock on the sequences and *not* taking the pg_subscription_rel AccessExclusiveLock its sibling removal paths take precisely to freeze rel states (:1333-1334); `DROP SEQUENCE` deletes the row via `heap_drop_with_catalog` → `RemoveSubscriptionRel` with only RowExclusiveLock on pg_subscription_rel (pg_subscription.c:505) and no subscription-object lock, so nothing serializes the commands until they collide on the catalog tuple (`UpdateSubscriptionRelState` syscache miss → elog at pg_subscription.c:413-415; heapam.c:4477/3176 for the wait-then-fail variants). The sync worker itself is protected against this race (it holds the relation open across its update, sequencesync.c:336/416); the refresh command is the unprotected outlier.
**Trigger.** Deterministic: `BEGIN; DROP SEQUENCE s1;` in one session, `REFRESH SEQUENCES` in another, then COMMIT (→ "tuple concurrently deleted"); reverse order inside `BEGIN` for "tuple concurrently updated" (REFRESH SEQUENCES is allowed in a transaction block).
**Repro sketch.** As trigger, on a subscription with s1/s2 already 'r'.
**Fix direction.** Take the same pg_subscription_rel AccessExclusiveLock (or per-sequence locks) in `AlterSubscription_refresh_seq`, and/or tolerate a concurrently-vanished row with a skip instead of elog.
---
### 11. Sequence-removal loop runs after irreversible tablesync slot drops (Low)
**Defect.** f0b3573c3 appended the sequence-removal loop (subscriptioncmds.c:1322-1344) *after* the `ReplicationSlotDropAtPubNode()` loop (:1295-1316), violating the explicit invariant at :1290-1294 that slot drops must come last because they cannot be rolled back. An error or cancel during sequence removal (each `RemoveSubscriptionRel` seqscans pg_subscription_rel with CFI, pg_subscription.c:497-567) aborts the transaction after publisher-side sync slots were dropped; a rolled-back-to-`FINISHEDCOPY` ('f') table then reuses its now-nonexistent slot without recreating it (tablesync.c:1342-1361) and error-loops with `replication slot "pg_%u_sync_%u_..." does not exist`.
**Skeptical caveats (verifier).** Re-running the failed ALTER self-heals (the slot drop is `missing_ok`); the wedge persists only if the publication change is abandoned. And with ≥2 removed tables a cancel between slot drops could already cause this pre-f0b3573c3 — the new loop violates the stated invariant, substantially widens the window, and newly exposes the single-removed-table case.
**Trigger.** `SET statement_timeout='2s'; ALTER SUBSCRIPTION sub SET PUBLICATION pub_new;` where the old set contained a mid-catchup ('f') table plus many sequences, timeout landing in the sequence loop.
**Repro sketch.** Large table held at 'f' via an AccessExclusiveLock on the subscriber; subscription also covers a sequence; ALTER SET PUBLICATION to a set containing neither, cancelled during sequence removal (gdb breakpoint at :1322 + `pg_cancel_backend` for determinism) → t1 wedged at 'f' with its slot gone.
**Fix direction.** Move the sequence-removal loop above the slot-drop loop, restoring the "slot drops last" invariant.
---
### 12. Batch query mixes MVCC definition columns with live `last_value`: validation bypass (Low)
**Defect.** A publisher `ALTER SEQUENCE` + `nextval()` committing while the batch query runs can produce an internally inconsistent row (old seqmin/seqmax/seqcycle, new last_value) that passes definition validation, yielding either a confusing `setval: value N is out of bounds for sequence` ERROR (22003) from a worker that never ran setval (subscription disabled under `disable_on_error`), or — CYCLE added upstream — a *silently* synced wrapped value marked READY despite a CYCLE/NO CYCLE mismatch the documented validation promises to catch.
**Mechanism.** The query (sequencesync.c:517-527) reads pg_sequence under the statement snapshot but `pg_get_sequence_data()` is volatile and reads the live buffer tuple (sequence.c:1831-1836); `ALTER SEQUENCE` takes only ShareRowExclusiveLock (sequence.c:450), not conflicting with the LATERAL's transient AccessShareLock. `get_and_validate_seq_info()` compares only the stale definition columns (:352-358); `SetSequence()`'s local bounds check (sequence.c:989-994) then errors, or the wrapped value fits and is marked READY (:416) with no warning. Same query-consistency family as item 7 (1ba3eee fixed the drop case; the ALTER case remains). Window is one publisher-side query execution — narrow; deterministic only with a paused walsender. The error variant self-corrects into the documented mismatch report on the next relaunch.
**Trigger.** `REFRESH SEQUENCES` concurrent with publisher `ALTER SEQUENCE s MAXVALUE 200; SELECT nextval('s')...` (error variant) or `ALTER SEQUENCE s CYCLE; SELECT nextval('s')` at max (silent variant).
**Repro sketch.** Both nodes `CREATE SEQUENCE s MAXVALUE 100`; subscribe; pause the publisher walsender in `pg_get_sequence_data` (gdb), apply the ALTER+nextval, resume.
**Fix direction.** Return the definition columns from `pg_get_sequence_data()` itself (one consistent read of the live tuple), or re-validate the received `last_value` against the local definition and classify violations as a retryable mismatch rather than letting setval's error surface.
---
### 13. REFRESH SEQUENCES warning misattributes a `copy_data` request the command cannot express (Low)
**Defect.** Plain `ALTER SUBSCRIPTION sub REFRESH SEQUENCES` on an `origin=none` subscription in a bidirectional setup emits `WARNING: subscription "sub" requested copy_data with origin = NONE but might copy data that had a different origin` — but REFRESH SEQUENCES accepts no `WITH` options at all (gram.y:11611-11619; no options parsed at subscriptioncmds.c:1614-1654), so the user "requested" nothing; they will hunt for a nonexistent knob.
**Mechanism.** `AlterSubscription_refresh_seq()` calls `check_publications_origin_sequences(..., true /* copydata hardcoded */, ...)` (subscriptioncmds.c:1383-1384), which reuses the CREATE SUBSCRIPTION/REFRESH PUBLICATION message at :3270-3277 — accurate at those call sites (which pass the user's actual copy_data), inaccurate here. The warning's substance (sequence data will be copied and may have a different origin) and hint remain correct; only the attribution is wrong.
**Trigger/repro sketch.** Two-node bidirectional `origin=none` sequence subscriptions; run `REFRESH SEQUENCES` on either side.
**Fix direction.** Use a REFRESH SEQUENCES-specific message ("... will copy sequence data that might have a different origin") instead of the "requested copy_data" wording.
---
### 14. psql tab-completion regression after `REFRESH PUBLICATION` (Low)
**Defect.** In PG18, `ALTER SUBSCRIPTION sub REFRESH PUBLICATION <TAB>` completed `WITH (`; in master it offers the subscriber's *local* publication names (syntactically invalid if accepted — the command takes no name, gram.y:11601) or nothing when no local publication exists.
**Mechanism.** f0b3573c3 deleted the rule `Matches("ALTER","SUBSCRIPTION",MatchAny,MatchAnyN,"REFRESH","PUBLICATION") → COMPLETE_WITH("WITH (")` (REL_18_0 tab-complete.in.c:2306), replacing it with a REFRESH-terminal rule (master :2355-2356) and never re-adding the follow-up; no remaining pattern matches, so control falls to the `words_after_create` fallback, where prev_wd "PUBLICATION" hits `Query_for_list_of_publications` (:1335, :1212-1215). The surviving `REFRESH PUBLICATION WITH (` → copy_data rule (:2358) shows the path is still intended.
**Trigger/repro sketch.** Subscriber with a local publication (bidirectional setup); type the command and press Tab.
**Fix direction.** Restore the deleted `..."REFRESH","PUBLICATION"` → `COMPLETE_WITH("WITH (")` rule.
---
### 15. pg_stat_subscription row for the sequencesync worker: fabricated times, undocumented NULLs (Low)
**Merged finding.** Two audit findings merged: both stem from adding the new worker type to `pg_stat_subscription` (55cefadde touched only the `worker_type` row in monitoring.sgml) without adjusting either the column semantics in code or the per-column NULL documentation.
**Defect.** For its entire lifetime, a sequencesync worker's row shows `last_msg_send_time`/`last_msg_receipt_time`/`latest_end_time` frozen at the worker's *start* time — masquerading as WAL-sender message times for a worker type that never receives WAL-sender messages — while `received_lsn`/`latest_end_lsn`/`relid`/`leader_pid` are always NULL. monitoring.sgml (:2325-2327, :2336-2337, :2346-2347, :2376-2377) enumerates NULL cases that omit this worker type entirely (e.g. relid "NULL for the leader apply worker and parallel apply workers"; time/LSN columns "NULL for parallel apply workers"). A DBA watching a long sync sees frozen "message" timestamps and an inconsistent row (`latest_end_time` set, `latest_end_lsn` NULL) suggesting a hung connection.
**Mechanism.** `SetupApplyOrSyncWorker()` initializes the three timestamps to `GetCurrentTimestamp()` (worker.c:5979-5981); the only updater, `UpdateWorkerStats()` (worker.c:3987), is called solely from `LogicalRepApplyLoop`, which this worker never enters; `pg_stat_get_subscription()` NULLs timestamps only when exactly 0 (launcher.c:1671-1683), a convention only parallel apply workers satisfy (applyparallelworker.c:958-959), and emits `relid` only for tablesync workers (launcher.c:1656-1659; sequencesync launches with InvalidOid relid, sequencesync.c:138).
**Trigger/repro sketch.** Poll `SELECT worker_type, relid, leader_pid, received_lsn, latest_end_lsn, last_msg_send_time FROM pg_stat_subscription WHERE worker_type='sequence synchronization'` during a sync of a few thousand sequences.
**Fix direction.** Zero the time fields for sequencesync workers so they render NULL (matching the parallel-apply convention), and extend monitoring.sgml's per-column NULL enumerations to cover the new worker type.
---
### 16. catalogs.sgml `srsublsn` description wrong for sequence rows (Low)
**Defect.** catalogs.sgml:8893-8896 still describes `srsublsn` solely as "Remote LSN of the state change ... when in s or r states, otherwise null". For a sequence row it actually holds the publisher sequence's *page LSN* from `pg_get_sequence_data()` — possibly far older than the sync, and precisely the value logical-replication.sgml:1844-1853 tells DBAs to compare in the out-of-sync procedure — and `copy_data=false` creates 'r'-state sequence rows with `srsublsn` NULL, contradicting "otherwise null".
**Mechanism.** `copy_sequence()` writes `UpdateSubscriptionRelState(..., SUBREL_STATE_READY, seqinfo->page_lsn, ...)` (sequencesync.c:416-417), page_lsn being `PageGetLSN()` of the publisher's sequence page (sequence.c:1836); copy_data=false inserts sequences directly READY with InvalidXLogRecPtr → SQL NULL (subscriptioncmds.c:985, :1203-1205; pg_subscription.c:353-356). Caveat: the ('r', NULL) shape has existed for tables since PG10; the feature-specific gap is the undocumented page-LSN meaning, which the sequences docs actively rely on.
**Trigger/repro sketch.** `CREATE SUBSCRIPTION ... WITH (copy_data=false)` → ('r', NULL) rows; after `REFRESH SEQUENCES`, `srsublsn` equals the publisher's `pg_get_sequence_data(...).page_lsn`.
**Fix direction.** Amend the `srsublsn` entry to state its sequence-row meaning (publisher page LSN used for out-of-sync comparison) and the copy_data=false NULL case.
---
### 17. Docs omit the PG19+ publisher requirement for sequence sync; REFRESH SEQUENCES silently no-ops (Low)
**Defect.** The failover-preparation restriction (logical-replication.sgml:2359-2372) recommends `ALTER SUBSCRIPTION ... REFRESH SEQUENCES` with no stated publisher-version precondition, and no SGML file anywhere mentions one; against a pre-PG19 publisher, sequences are silently never registered (`fetch_relation_list` gates the sequence UNION on `server_version >= 190000`, subscriptioncmds.c:3467) and `REFRESH SEQUENCES` succeeds as a fully silent no-op (origin check returns early, :3196-3198; empty relation list, empty loop). A DBA on the standard staged-upgrade topology (PG19 subscriber ← PG18 publisher) follows remedy #1, sees success, and discovers the gap as duplicate keys after failover — when remedies #2/#3 (manual update) would have worked.
**Skeptical caveat.** Severity capped low: `FOR ALL SEQUENCES` cannot be created on a pre-PG19 publisher, so a from-scratch setup fails loudly at step one; the exposed audience is existing cross-version subscriptions. Related to item 6 (same version dependency) but distinct: item 6 is the error loop when sequence rows *do* exist; this is silent success when they don't. In-tree precedent documents comparable cross-version caveats (binary pre-16, row filters pre-15) and `retain_dead_tuples` even errors on old publishers (:3304-3307).
**Trigger/repro sketch.** PG18 publisher `FOR ALL TABLES` with a serial column advancing to ~1000; PG19 subscriber follows the restrictions text: `REFRESH SEQUENCES` succeeds, `last_value` stays 1; post-failover insert → duplicate key.
**Fix direction.** State the PG19+ publisher requirement in the sequences documentation (and consider a NOTICE/WARNING from `REFRESH SEQUENCES` when the subscription has no subscribed sequences or the publisher predates support).
---
### 18. Restrictions bullet still claims only tables can be replicated (Low)
**Defect.** logical-replication.sgml:2399-2405 (unchanged since 17b9e7f) states "Replication is only supported by tables ... Attempts to replicate other types of relations, such as views, materialized views, or foreign tables, will result in an error" — contradicted three sections earlier by "Replicating Sequences" (:1772) and by the adjacent sequence-restrictions bullet (:2357-2372) added by 55cefadde.
**Mechanism.** Sequences are publishable (`is_publishable_class` accepts RELKIND_SEQUENCE, pg_publication.c:163-172) and valid subscription targets (`CheckSubscriptionRelkind`, execReplication.c:1140-1166); `FOR ALL SEQUENCES` grammar exists (gram.y:11422); 036_sequences.pl demonstrates successful sequence sync with no error. The only defense — reading "Replication" as strictly incremental streaming — fails against the bullet's categorical second sentence and the docs' own "Replicating Sequences" terminology.
**Trigger/repro sketch.** Read the Restrictions list, then observe `CREATE PUBLICATION p FOR ALL SEQUENCES` + subscription syncing values without error.
**Fix direction.** Reword the bullet to say tables and sequences are supported (sequences synchronized on request rather than streamed), keeping the error claim accurate for views/matviews/foreign tables.
---
## Appendix A: findings dropped unverified (verifier cap of 23)
These 5 ranked below the cap and were **not** adversarially verified; treat as leads only.
- ProcessSequencesForSync's only_running=true check allows two concurrent sequencesync workers for one subscription, causing "tuple concurrently updated" errors and disable_on_error trips
- Unlogged subscriber sequence reaches durable READY state while its synced value is lost on crash, permanently stale and reported in-sync
- create_subscription.sgml claims the origin parameter "has no effect for sequences", yet origin=none triggers sequence-specific origin warnings and sequence values of foreign origin are copied regardless, which is documented nowhere
- Docs recommend ALTER SUBSCRIPTION ... REFRESH SEQUENCES to update sequences at failover, but the command cannot run once the publisher is down and the duplicate-value consequence of lagging sequences is never stated
- Connect-failure message uses internal jargon "sequencesync worker", inconsistent with the same worker's other messages and the tablesync counterpart
## Appendix B: findings refuted by adversarial verification
- **REFRESH SEQUENCES is allowed inside a transaction block and can deadlock with the sequencesync worker when combined with ALTER SEQUENCE**
- Refutation: The individual lock sites are all real (verified in master: no PreventInTransactionBlock in the ALTER_SUBSCRIPTION_REFRESH_SEQUENCES case at src/backend/commands/subscriptioncmds.c:2286-2297; AccessExclusiveLock on the subscription object at subscriptioncmds.c:1714 for ALL ALTER SUBSCRIPTION forms; AccessShareLock on the subscription object in UpdateSubscriptionRelState at src/backend/catalog/pg_subscription.c:405 held to batch commit; try_table_open(RowExclusiveLock) at src/backend/replication/logical/sequencesync.c:336; ShareRowExclusiveLock for ALTER SEQUENCE at src/backend/commands/sequence.c:449-453). But the finding fails on four counts. (1) Its trigger is unreachable: copy_sequence -> UpdateSubscriptionRelState is called ONLY on COPYSEQ_SUCCESS (sequencesync.c:554-556, 416), and per-batch successes are committed (line 647) before report_sequence_errors raises the mismatch ERROR (line 671), so in the claimed 'worker retries every 5s due to a definition mismatch' steady state the retry batch contains only failing INIT sequences, which never acquire the subscription-object lock -- no cycle can form in the documented mismatch-repair workflow; a worker parked on the user's ALTERed sequence holds nothing the user's REFRESH SEQUENCES needs. (2) The causal premise is wrong: the other refresh forms forbid transaction blocks because AlterSubscription_refresh irreversibly drops table-synchronization slots on the publisher (per alter_subscription.sgml), not to avoid deadlocks; AlterSubscription_refresh_seq (subscriptioncmds.c:1360-1406) performs only read-only publisher queries and rollbackable local catalog updates, and the docs' explicit list of forms that 'cannot be executed inside a transaction block' intentionally omits REFRESH SEQUENCES -- code and documentation agree, so this is designed behavior. (3) Adding PreventInTransactionBlock to REFRESH SEQUENCES would not eliminate the deadlock: line 1714 is reached by every ALTER SUBSCRIPTION form, and ENABLE/DISABLE/SET/SKIP are deliberately allowed in transaction blocks (ENABLE docs even say the worker starts 'at the end of the transaction'), so BEGIN; ALTER SEQUENCE s; ALTER SUBSCRIPTION sub DISABLE; forms the identical cycle, as does BEGIN; ALTER SEQUENCE s1; ALTER SEQUENCE s2; against a worker retaining RowExclusiveLock on s2 from earlier in the batch (table_close NoLock, line 625). (4) The deadlock class is pre-existing and accepted: tablesync has held the target-table RowExclusiveLock across COPY (tablesync.c:1401) and then taken the subscription-object AccessShareLock via UpdateSubscriptionRelState (tablesync.c:1503) in one transaction since commit cb9079c (2017), giving the same detector-resolved cycle against transaction-block ALTER SUBSCRIPTION forms; and the comment at sequencesync.c:485-509 shows the developers explicitly analyzed worker-vs-ALTER SEQUENCE deadlocks and guarded only against the undetectable cross-node variant, accepting local detectable ones. The only constructible outcome (requires a multi-sequence batch with at least one success before the contended sequence, i.e. mid initial-sync or mid full re-sync, plus a deliberately open transaction mixing sequence DDL with ALTER SUBSCRIPTION) is a transient 'ERROR: deadlock detected' that the deadlock detector resolves and the respawned worker recovers from -- standard, accepted PostgreSQL DDL concurrency behavior, not a user-visible defect of the sequence-replication feature.
- **Sequencesync batches accumulate RowExclusiveLock on up to 100 sequences until commit, creating deadlocks with ordinary multi-sequence DDL or ALTER SUBSCRIPTION in the same transaction**
- Refutation: The mechanism is factually accurate but describes ordinary, deliberate, detected-and-resolved PostgreSQL locking behavior, not a defect. Verified: sequencesync batches take try_table_open(seq, RowExclusiveLock) per row (sequencesync.c:336), retain locks via table_close(NoLock) (line 625) until CommitTransactionCommand (line 647) for up to 100 sequences (line 441); ALTER SEQUENCE takes conflicting ShareRowExclusiveLock (sequence.c:449-450); UpdateSubscriptionRelState takes AccessShareLock on the subscription held to commit (pg_subscription.c:405); ALTER SUBSCRIPTION takes AccessExclusiveLock (subscriptioncmds.c:1714). So the claimed lock cycles can occur — but both parties are ordinary backends on heavyweight locks, so the local deadlock detector resolves the cycle after deadlock_timeout with a clean, retryable "deadlock detected" error. That disqualifies it as a defect on four grounds: (1) The trigger requires the user's own transaction to acquire conflicting locks on multiple objects in inconsistent order — the canonical application-responsibility scenario per the docs (mvcc.sgml "Deadlocks"); two plain user sessions doing the same two ALTER SEQUENCEs in opposite order deadlock identically with no replication involved. (2) The finding's novelty claim ("unlike tablesync ... this wait-while-holding pattern is new") is wrong: the apply worker has accumulated RowExclusiveLocks on many relations per applied transaction, in remote-determined order the local user cannot predict, since PG10 (worker.c:2673/2730, 2834/2918, 3057/3117 open with RowExclusiveLock, close with NoLock), and applyparallelworker.c:62-67 explicitly states such deadlocks "can happen even without parallel mode when there are concurrent operations on the subscriber" — the project's design bar is detectability, not prevention. (3) sequencesync.c deliberately implements that doctrine: the header comment (lines 42-44) documents batch-duration lock retention, and the comment at 485-509 orders the remote fetch (walrcv_exec, line 529) before any local sequence locks so the worker never waits on the network while holding local locks, eliminating undetectable cross-node deadlocks and knowingly accepting locally detectable ones. (4) The outcome is transient, atomic, and self-healing: a victim worker's batch aborts atomically (SetSequence and READY-state updates roll back together, sequences stay INIT, no inconsistent catalog state), start_sequence_sync reports stats and rethrows, and the apply worker relaunches a sequencesync worker after wal_retrieve_retry_interval (syncutils.c:117-148, 175), which then succeeds; disable_on_error disabling the subscription on any error, including deadlocks, is that option's documented contract and applies equally to apply-worker deadlocks today. There is also no actionable fix consistent with the design: the lock protecting the WAL-logged SetSequence update cannot be released before commit, batching is a documented performance choice, and no ordering discipline can be consistent with arbitrary user DDL order. Not covered by any already-fixed commit (git log on sequencesync.c shows no locking changes). A report of this would be answered "working as intended; retry on deadlock", so confirming it would waste committer time.
- **A failed or interrupted sync batch leaves subscriber sequence values silently overwritten (non-transactionally) while pg_subscription_rel still says INIT**
- Refutation: Every code citation in the finding is accurate — I traced them all in master: copy_sequences() batches up to 100 sequences per transaction (src/backend/replication/logical/sequencesync.c:441, StartTransactionCommand :462, CommitTransactionCommand :647), accumulates RowExclusiveLocks to commit (:336, table_close(NoLock) :625), copy_sequence() calls SetSequence at :407 followed by transactional UpdateSubscriptionRelState(SUBREL_STATE_READY) at :416-417, and SetSequence (src/backend/commands/sequence.c:946-1043) updates the sequence page in place in a critical section with XLOG_SEQ_LOG (:1011-1038) — no relfilenode swap, so the change is immediately visible (frozen-xmin tuple) and survives transaction abort. A pg_terminate_backend hitting CHECK_FOR_INTERRUPTS (:544) mid-batch does leave earlier sequences' values applied while their catalog rows revert to 'i' with NULL srsublsn (REFRESH SEQUENCES writes InvalidXLogRecPtr, subscriptioncmds.c:1392-1393). Nothing in git history changed this. However, the behavior is not a defect: (1) SetSequence IS setval()'s implementation, and setval's non-transactionality is explicitly documented (doc/src/sgml/func/func-sequence.sgml:199-201: changes "are immediately visible to other transactions, and are not undone if the calling transaction rolls back") — the feature inherits documented sequence semantics; (2) "failed sync => no persisted mutation" was never this feature's contract even absent aborts: report_sequence_errors() raises the overall failure ERROR only AFTER all batches have committed (sequencesync.c:670-673), so a sync failing on one mismatched/missing/no-permission sequence has already permanently committed publisher values AND READY state for every other sequence — the finding's within-batch-abort case (values applied, state 'i') is a strictly milder variant of designed behavior (values applied, state 'r', command reports failure); (3) the leftover 'i' state is the conservative, correct-by-contract state: the apply worker respawns the sequencesync worker (ProcessSequencesForSync, sequencesync.c:96-140; syncutils.c wal_retrieve_retry_interval throttling) and re-copy via SetSequence is idempotent, a retry loop the docs explicitly describe (logical-replication.sgml:1824-1829); the ordering (non-rollbackable value first, transactional catalog second) is the only crash-safe ordering for a non-MVCC object — the reverse would produce READY-without-copy, a real data-loss bug; (4) the claimed harm ("regressing a locally-advanced sequence" to the publisher value) is exactly what the user commanded — a successful REFRESH SEQUENCES installs the same value — and no wrong final value can persist while the subscription exists; (5) no documentation promises atomicity or rollback of sequence values on sync failure, so there is no documented-vs-actual divergence, and pg_subscription_rel showing 'i' is accurate per its meaning ("needs synchronization"; resync is pending and harmless). The tablesync comparison establishes no contract: heap data is MVCC-rollbackable, sequence data fundamentally is not. A report of this to pgsql-hackers would be closed as works-as-intended/inherent setval semantics. The mechanism is real but it is intended, documented-primitive, self-healing behavior — not a user-visible defect.
## Appendix C: provenance
Produced by a 45-agent workflow (20 finder lenses over master @ a8c2547 -> dedup/rank of 63 raw findings -> 23 adversarial verifiers -> synthesis). All verification is static code tracing; no repro was executed. Per-agent transcripts: ~/.claude/projects/-home-nm-src-pg-postgresql/3a1250f2-41e8-46fd-afe1-08ff8e4dace0/subagents/workflows/wf_e3d25a7b-189/journal.jsonl
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: sequencesync worker race with REFRESH SEQUENCES
@ 2026-07-10 07:00 vignesh C <[email protected]>
parent: Noah Misch <[email protected]>
0 siblings, 2 replies; 10+ messages in thread
From: vignesh C @ 2026-07-10 07:00 UTC (permalink / raw)
To: Noah Misch <[email protected]>; +Cc: [email protected]; pgsql-hackers
On Fri, 10 Jul 2026 at 10:22, Noah Misch <[email protected]> wrote:
>
> A Fable 5 review of logical replication of sequences found a way to get
> subscribed sequences into READY state despite the subscriber side having data
> older than the last REFRESH SEQUENCES. I'm attaching the test case it wrote.
> I reviewed the test, and I think it identifies a genuine defect.
Thanks for reporting this. The test case reproduces this issue. This
issue is not limited to lock contention. It can also occur if the
sequence synchronization worker is simply slow. For example, the
worker may fetch the sequence value from the publisher, after which
the publisher sequence advances and ALTER SUBSCRIPTION ... REFRESH
SEQUENCES is executed. When the worker eventually updates the
subscriber, it uses the stale value that it fetched earlier,
overwriting the sequence with an out-of-date value.
How about raising a warning for the second REFRESH SEQUENCES
indicating that the sequence is already being synchronized and
skipping it? The warning could also include a hint to rerun ALTER
SUBSCRIPTION ... REFRESH SEQUENCES after the current synchronization
completes. This approach avoids making ALTER SUBSCRIPTION ... REFRESH
SEQUENCES wait for the sequence synchronization worker, which could
itself be blocked waiting for a lock held by a long-running
transaction on the sequence.
Regards,
Vignesh
^ permalink raw reply [nested|flat] 10+ messages in thread
* RE: sequencesync worker race with REFRESH SEQUENCES
@ 2026-07-10 08:18 Hayato Kuroda (Fujitsu) <[email protected]>
parent: vignesh C <[email protected]>
1 sibling, 0 replies; 10+ messages in thread
From: Hayato Kuroda (Fujitsu) @ 2026-07-10 08:18 UTC (permalink / raw)
To: 'vignesh C' <[email protected]>; Noah Misch <[email protected]>; +Cc: [email protected] <[email protected]>; pgsql-hackers
Dear Vignesh, Noah,
Agree the basic approach that we avoid the re-initializing the catalog.
My primitive idea is to follow what slotsync does. SlotSyncCtxStruct::syncing is
checked from the SQL function and the slotsync worker, and either of them can
continue. So we may be able to have LogicalRepCtxStruct::synching_seqs or
something to indicate the status.
But there might be other ways, needs to be investigated.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: sequencesync worker race with REFRESH SEQUENCES
@ 2026-07-10 08:40 shveta malik <[email protected]>
parent: vignesh C <[email protected]>
1 sibling, 1 reply; 10+ messages in thread
From: shveta malik @ 2026-07-10 08:40 UTC (permalink / raw)
To: vignesh C <[email protected]>; +Cc: Noah Misch <[email protected]>; [email protected]; pgsql-hackers; shveta malik <[email protected]>
On Fri, Jul 10, 2026 at 12:30 PM vignesh C <[email protected]> wrote:
>
> On Fri, 10 Jul 2026 at 10:22, Noah Misch <[email protected]> wrote:
> >
> > A Fable 5 review of logical replication of sequences found a way to get
> > subscribed sequences into READY state despite the subscriber side having data
> > older than the last REFRESH SEQUENCES. I'm attaching the test case it wrote.
> > I reviewed the test, and I think it identifies a genuine defect.
>
> Thanks for reporting this. The test case reproduces this issue. This
> issue is not limited to lock contention. It can also occur if the
> sequence synchronization worker is simply slow. For example, the
> worker may fetch the sequence value from the publisher, after which
> the publisher sequence advances and ALTER SUBSCRIPTION ... REFRESH
> SEQUENCES is executed. When the worker eventually updates the
> subscriber, it uses the stale value that it fetched earlier,
> overwriting the sequence with an out-of-date value.
I have the same opinion here.
> How about raising a warning for the second REFRESH SEQUENCES
> indicating that the sequence is already being synchronized and
> skipping it? The warning could also include a hint to rerun ALTER
> SUBSCRIPTION ... REFRESH SEQUENCES after the current synchronization
> completes.
I agree. This is the simplest solution here.
~~
> Agree the basic approach that we avoid the re-initializing the catalog.
> My primitive idea is to follow what slotsync does. SlotSyncCtxStruct::syncing is
> checked from the SQL function and the slotsync worker, and either of them can
> continue. So we may be able to have LogicalRepCtxStruct::synching_seqs or
> something to indicate the status.
> But there might be other ways, needs to be investigated.
If this approach is accepted, I would prefer implementing it similarly
to how ALTER SUBSCRIPTION ... SET (two_phase) is handled. The command
simply fails (with an appropriate hint) if the apply worker for the
subscription is still running, by checking for the presence of the
apply worker.
IMO, a mechanism similar to SlotSync is not needed here because there
is currently no scenario in which continuous incremental
synchronization and one-time manual synchronization can run
concurrently. The worker itself is started only by the manual
synchronization operation and is not a continuously running process.
thanks
Shveta
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: sequencesync worker race with REFRESH SEQUENCES
@ 2026-07-10 12:36 vignesh C <[email protected]>
parent: shveta malik <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: vignesh C @ 2026-07-10 12:36 UTC (permalink / raw)
To: shveta malik <[email protected]>; +Cc: Noah Misch <[email protected]>; [email protected]; pgsql-hackers
On Fri, 10 Jul 2026 at 14:10, shveta malik <[email protected]> wrote:
>
> On Fri, Jul 10, 2026 at 12:30 PM vignesh C <[email protected]> wrote:
> >
> > On Fri, 10 Jul 2026 at 10:22, Noah Misch <[email protected]> wrote:
> > >
> > > A Fable 5 review of logical replication of sequences found a way to get
> > > subscribed sequences into READY state despite the subscriber side having data
> > > older than the last REFRESH SEQUENCES. I'm attaching the test case it wrote.
> > > I reviewed the test, and I think it identifies a genuine defect.
> >
> > Thanks for reporting this. The test case reproduces this issue. This
> > issue is not limited to lock contention. It can also occur if the
> > sequence synchronization worker is simply slow. For example, the
> > worker may fetch the sequence value from the publisher, after which
> > the publisher sequence advances and ALTER SUBSCRIPTION ... REFRESH
> > SEQUENCES is executed. When the worker eventually updates the
> > subscriber, it uses the stale value that it fetched earlier,
> > overwriting the sequence with an out-of-date value.
>
> I have the same opinion here.
>
> > How about raising a warning for the second REFRESH SEQUENCES
> > indicating that the sequence is already being synchronized and
> > skipping it? The warning could also include a hint to rerun ALTER
> > SUBSCRIPTION ... REFRESH SEQUENCES after the current synchronization
> > completes.
>
> I agree. This is the simplest solution here.
The attached v1-0001 patch avoids resetting the synchronization state
of sequences that are already being synchronized. Instead, it emits a
warning informing the user that the sequence is already being
synchronized and suggests rerunning ALTER SUBSCRIPTION ... REFRESH
SEQUENCES after the current synchronization completes. I'm not sure
whether a test is necessary for this change. Nevertheless, the
attached v1-0002 patch adds a regression test that uses an injection
point to deterministically reproduce the race condition and verifies
that the expected warning is emitted when ALTER SUBSCRIPTION ...
REFRESH SEQUENCES is invoked while a sequence synchronization is
already in progress.
Regards,
Vignesh
Attachments:
[application/octet-stream] v1-0001-Skip-refreshing-sequences-that-are-already-being-.patch (2.6K, ../../CALDaNm0VQVE8Ju+TtQwH0o3_2j19nByoFdrBUwCLtDjV4wieJg@mail.gmail.com/2-v1-0001-Skip-refreshing-sequences-that-are-already-being-.patch)
download | inline diff:
From 2d274c48ecd8b262df260b91f111bbdbfd9f9eb7 Mon Sep 17 00:00:00 2001
From: Vignesh C <[email protected]>
Date: Fri, 10 Jul 2026 18:02:05 +0530
Subject: [PATCH v1 1/2] Skip refreshing sequences that are already being
synchronized
'ALTER SUBSCRIPTION ... REFRESH SEQUENCES' resets the synchronization
state of each subscribed sequence to 'INIT', allowing the sequence sync
worker to synchronize its value from the publisher.
However, if a sequence is already in the 'INIT' state, it indicates that
a previous 'REFRESH SEQUENCES' has not yet been processed by the sequence
sync worker. Resetting it again can race with the worker if it has
already fetched the publisher's sequence value but has not yet applied it
to the subscriber. In that case, the worker may later overwrite the
subscriber sequence with the stale value it previously fetched and mark
the sequence as 'READY', causing the subsequent refresh request to be
lost.
Avoid this race by skipping sequences that are already in the 'INIT'
state. Emit a warning informing the user that the sequence is already
being synchronized and suggesting that 'ALTER SUBSCRIPTION ... REFRESH
SEQUENCES' be rerun after the current synchronization completes.
---
src/backend/commands/subscriptioncmds.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 4292e7fb8f4..6569cebb142 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1389,6 +1389,26 @@ AlterSubscription_refresh_seq(Subscription *sub)
{
Oid relid = subrel->relid;
+ /*
+ * If the sequence is already marked INIT, a previous REFRESH
+ * SEQUENCES has not yet been picked up by the sequencesync
+ * worker. Resetting it again here would race with that worker
+ * applying the values it already fetched from the publisher,
+ * possibly leaving the sequence marked READY with stale data.
+ * Skip it and let the user know instead.
+ */
+ if (subrel->state == SUBREL_STATE_INIT)
+ {
+ ereport(WARNING,
+ errmsg("sequence \"%s.%s\" of subscription \"%s\" is already being synchronized, skipping",
+ get_namespace_name(get_rel_namespace(relid)),
+ get_rel_name(relid),
+ sub->name),
+ errhint("Rerun %s after the current synchronization completes.",
+ "ALTER SUBSCRIPTION ... REFRESH SEQUENCES"));
+ continue;
+ }
+
UpdateSubscriptionRelState(sub->oid, relid, SUBREL_STATE_INIT,
InvalidXLogRecPtr, false);
ereport(DEBUG1,
--
2.50.1 (Apple Git-155)
[application/octet-stream] v1-0002-Add-a-test-for-concurrent-REFRESH-SEQUENCES.patch (7.9K, ../../CALDaNm0VQVE8Ju+TtQwH0o3_2j19nByoFdrBUwCLtDjV4wieJg@mail.gmail.com/3-v1-0002-Add-a-test-for-concurrent-REFRESH-SEQUENCES.patch)
download | inline diff:
From fc4a0d32f6bf41db8132555278b507d8a1e35fa2 Mon Sep 17 00:00:00 2001
From: Vignesh C <[email protected]>
Date: Fri, 10 Jul 2026 18:03:10 +0530
Subject: [PATCH v1 2/2] Add a test for concurrent REFRESH SEQUENCES
Add a TAP test covering the case where 'ALTER SUBSCRIPTION ...
REFRESH SEQUENCES' is executed while a sequence synchronization is
already in progress.
The test uses an injection point to pause the sequence synchronization
worker immediately before it applies the fetched sequence value to the
subscriber. While the worker is paused, a second 'ALTER SUBSCRIPTION ...
REFRESH SEQUENCES' command is issued and verified to emit the expected
warning indicating that the sequence is already being synchronized.
This test exercises the race condition and verifies that concurrent
refresh requests are handled safely without resetting the synchronization
state of sequences that are already being processed.
---
.../replication/logical/sequencesync.c | 5 +
src/test/subscription/meson.build | 1 +
.../t/039_sequences_refresh_warning.pl | 127 ++++++++++++++++++
3 files changed, 133 insertions(+)
create mode 100644 src/test/subscription/t/039_sequences_refresh_warning.pl
diff --git a/src/backend/replication/logical/sequencesync.c b/src/backend/replication/logical/sequencesync.c
index 770fa5de10b..631b9c4bf88 100644
--- a/src/backend/replication/logical/sequencesync.c
+++ b/src/backend/replication/logical/sequencesync.c
@@ -68,6 +68,7 @@
#include "utils/inval.h"
#include "utils/lsyscache.h"
#include "utils/memutils.h"
+#include "utils/injection_point.h"
#include "utils/pg_lsn.h"
#include "utils/syscache.h"
#include "utils/usercontext.h"
@@ -552,8 +553,12 @@ copy_sequences(WalReceiverConn *conn)
sync_status = get_and_validate_seq_info(slot, &sequence_rel,
&seqinfo, &seqidx);
if (sync_status == COPYSEQ_SUCCESS)
+ {
+ INJECTION_POINT("sequencesync-before-copy-sequence", NULL);
+
sync_status = copy_sequence(seqinfo,
sequence_rel->rd_rel->relowner);
+ }
switch (sync_status)
{
diff --git a/src/test/subscription/meson.build b/src/test/subscription/meson.build
index e71e95c6297..79f7e5a6405 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_sequences_refresh_warning.pl',
't/100_bugs.pl',
],
},
diff --git a/src/test/subscription/t/039_sequences_refresh_warning.pl b/src/test/subscription/t/039_sequences_refresh_warning.pl
new file mode 100644
index 00000000000..23508ef3780
--- /dev/null
+++ b/src/test/subscription/t/039_sequences_refresh_warning.pl
@@ -0,0 +1,127 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+# Test that ALTER SUBSCRIPTION ... REFRESH SEQUENCES emits a WARNING and
+# skips resetting a sequence that is already in INIT state, i.e. one whose
+# previous REFRESH SEQUENCES has not yet been picked up by the
+# sequencesync worker.
+#
+# This is exercised deterministically using an injection point placed just
+# before copy_sequence() is called in the sequencesync worker's main loop:
+#
+# 1. Run a first REFRESH SEQUENCES, which puts the sequence into INIT state
+# and lets the sequencesync worker start.
+# 2. The worker blocks at the injection point after fetching the sequence's
+# remote values but before applying them / marking the row READY.
+# 3. Run a second REFRESH SEQUENCES while the worker is still blocked. Since
+# the sequence is still in INIT state, the command must emit a WARNING
+# (with a HINT to retry later) and must not disturb the row.
+# 4. Release the injection point and let the worker finish; the sequence
+# should reach READY normally.
+
+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;
+
+# This test depends on an injection point to block the sequencesync worker
+# right before it applies a sequence's fetched values.
+my $injection_points_supported =
+ $node_subscriber->check_extension('injection_points');
+
+if (!$injection_points_supported)
+{
+ plan skip_all => 'Server does not support injection points.';
+}
+
+$node_subscriber->append_conf('postgresql.conf',
+ "shared_preload_libraries = 'injection_points'");
+$node_subscriber->restart;
+
+$node_subscriber->safe_psql('postgres', "CREATE EXTENSION injection_points;");
+
+# Identical sequence definitions on both nodes.
+my $ddl = qq(CREATE SEQUENCE regress_seq1;);
+$node_publisher->safe_psql('postgres', $ddl);
+$node_subscriber->safe_psql('postgres', $ddl);
+
+$node_publisher->safe_psql('postgres',
+ "CREATE PUBLICATION regress_seq_pub FOR ALL SEQUENCES");
+
+my $publisher_connstr = $node_publisher->connstr . ' dbname=postgres';
+$node_subscriber->safe_psql('postgres',
+ "CREATE SUBSCRIPTION regress_seq_sub CONNECTION '$publisher_connstr' PUBLICATION regress_seq_pub"
+);
+
+# Wait for the initial sync to complete before testing REFRESH SEQUENCES.
+$node_subscriber->poll_query_until('postgres',
+ "SELECT count(*) = 0 FROM pg_subscription_rel WHERE srsubstate <> 'r'")
+ or die "timed out waiting for initial sequence sync";
+
+# Attach the injection point that will block the sequencesync worker just
+# before it applies a sequence's fetched values.
+$node_subscriber->safe_psql('postgres',
+ "SELECT injection_points_attach('sequencesync-before-copy-sequence', 'wait');"
+);
+
+# First REFRESH SEQUENCES: puts the sequence into INIT and starts the
+# sequencesync worker.
+$node_subscriber->safe_psql('postgres',
+ "ALTER SUBSCRIPTION regress_seq_sub REFRESH SEQUENCES");
+
+# Wait until the worker is blocked at the injection point, i.e. after
+# fetching the remote values but before applying them.
+$node_subscriber->wait_for_event('logical replication sequencesync worker',
+ 'sequencesync-before-copy-sequence');
+
+my $result = $node_subscriber->safe_psql('postgres',
+ "SELECT srsubstate FROM pg_subscription_rel WHERE srrelid = 'regress_seq1'::regclass"
+);
+is($result, 'i',
+ 'sequence is still in INIT state while the worker is blocked');
+
+# Second REFRESH SEQUENCES while the sequence is still in INIT: it must
+# not disturb the row, and must emit a WARNING with a HINT instead.
+my ($cmdret, $stdout, $stderr) = $node_subscriber->psql('postgres',
+ "ALTER SUBSCRIPTION regress_seq_sub REFRESH SEQUENCES");
+
+like(
+ $stderr,
+ qr/WARNING:.*sequence "public.regress_seq1" of subscription "regress_seq_sub" is already being synchronized, skipping/,
+ 'second REFRESH SEQUENCES warns that the sequence is already being synchronized'
+);
+like(
+ $stderr,
+ qr/HINT:.*Rerun ALTER SUBSCRIPTION \.\.\. REFRESH SEQUENCES after the current synchronization completes\./,
+ 'second REFRESH SEQUENCES hints to retry later'
+);
+
+$result = $node_subscriber->safe_psql('postgres',
+ "SELECT srsubstate FROM pg_subscription_rel WHERE srrelid = 'regress_seq1'::regclass"
+);
+is($result, 'i',
+ 'sequence remains in INIT state after the second REFRESH SEQUENCES');
+
+# Release the worker and let it finish normally.
+$node_subscriber->safe_psql('postgres',
+ "SELECT injection_points_wakeup('sequencesync-before-copy-sequence');
+ SELECT injection_points_detach('sequencesync-before-copy-sequence');"
+);
+
+$node_subscriber->poll_query_until('postgres',
+ "SELECT count(*) = 0 FROM pg_subscription_rel WHERE srsubstate <> 'r'")
+ or die "timed out waiting for sequence to reach READY state";
+
+$result = $node_subscriber->safe_psql('postgres',
+ "SELECT last_value, is_called FROM regress_seq1");
+is($result, '1|f', 'sequence reaches READY with expected values');
+
+done_testing();
--
2.50.1 (Apple Git-155)
^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2026-07-10 12:36 UTC | newest]
Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-10-09 16:16 Re: Avoid possible overflow (src/port/bsearch_arg.c) Ranier Vilela <[email protected]>
2024-10-28 12:13 ` Heikki Linnakangas <[email protected]>
2024-10-28 12:33 ` Ranier Vilela <[email protected]>
2024-10-28 15:30 ` Tomas Vondra <[email protected]>
2024-10-28 16:52 ` Heikki Linnakangas <[email protected]>
2026-07-10 04:52 sequencesync worker race with REFRESH SEQUENCES Noah Misch <[email protected]>
2026-07-10 07:00 ` Re: sequencesync worker race with REFRESH SEQUENCES vignesh C <[email protected]>
2026-07-10 08:18 ` RE: sequencesync worker race with REFRESH SEQUENCES Hayato Kuroda (Fujitsu) <[email protected]>
2026-07-10 08:40 ` Re: sequencesync worker race with REFRESH SEQUENCES shveta malik <[email protected]>
2026-07-10 12:36 ` Re: sequencesync worker race with REFRESH SEQUENCES vignesh C <[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