commit fe331b9 (HEAD -> seq-refresh-sequences-race)
Author:     Noah Misch <noah@leadboat.com>
AuthorDate: Thu Jul 9 19:39:45 2026 +0000
Commit:     Noah Misch <noah@leadboat.com>
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 <noreply@anthropic.com>
    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();
