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