From c10bb9d382925366cf6b2734870da594c429ada3 Mon Sep 17 00:00:00 2001 From: Zsolt Parragi Date: Thu, 16 Jul 2026 06:37:40 +0000 Subject: [PATCH vnofcbot] Add deterministic test for concurrent sequence drop during sync Reproduce the sequencesync assert crash deterministically with an injection point in has_sequence_privilege(): pause the publisher's sequence fetch right before the privilege lookup, drop the sequence from another session, then resume. The privilege column comes back NULL, exercising the concurrent-drop path in the sequencesync worker. Add a "wait-accept-invalidations" action to the injection_points module so the parked backend processes the concurrent drop's invalidations on resume; a plain "wait" would keep the pre-drop catalog snapshot and the sequence would still look present. Without the sequencesync fix the subscriber aborts on Assert(!isnull); with it the sequence is reported as missing on the publisher. --- src/backend/utils/adt/acl.c | 8 +++ .../injection_points/injection_points.c | 19 +++++ src/test/subscription/meson.build | 1 + .../t/039_sequence_concurrent_drop.pl | 69 +++++++++++++++++++ 4 files changed, 97 insertions(+) create mode 100644 src/test/subscription/t/039_sequence_concurrent_drop.pl diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c index e2547d719ed..bc7050acec0 100644 --- a/src/backend/utils/adt/acl.c +++ b/src/backend/utils/adt/acl.c @@ -46,6 +46,7 @@ #include "utils/array.h" #include "utils/builtins.h" #include "utils/catcache.h" +#include "utils/injection_point.h" #include "utils/inval.h" #include "utils/lsyscache.h" #include "utils/memutils.h" @@ -2243,6 +2244,13 @@ has_sequence_privilege_id(PG_FUNCTION_ARGS) roleid = GetUserId(); mode = convert_sequence_priv_string(priv_type_text); + + /* + * Injection point to test a sequence being dropped concurrently right + * before its relkind is looked up, which makes this function return NULL. + */ + INJECTION_POINT("has-sequence-privilege-before-relkind", NULL); + relkind = get_rel_relkind(sequenceoid); if (relkind == '\0') PG_RETURN_NULL(); diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c index 2d26ecedd5d..097dbb01ebb 100644 --- a/src/test/modules/injection_points/injection_points.c +++ b/src/test/modules/injection_points/injection_points.c @@ -31,6 +31,7 @@ #include "utils/builtins.h" #include "utils/guc.h" #include "utils/injection_point.h" +#include "utils/inval.h" #include "utils/memutils.h" #include "utils/tuplestore.h" #include "utils/wait_event.h" @@ -81,6 +82,9 @@ extern PGDLLEXPORT void injection_notice(const char *name, extern PGDLLEXPORT void injection_wait(const char *name, const void *private_data, void *arg); +extern PGDLLEXPORT void injection_wait_accept_invalidations(const char *name, + const void *private_data, + void *arg); /* track if injection points attached in this process are linked to it */ static bool injection_point_local = false; @@ -290,6 +294,19 @@ injection_wait(const char *name, const void *private_data, void *arg) SpinLockRelease(&inj_state->lock); } +/* + * Like injection_wait(), but process pending cache invalidations after being + * woken up. Useful to test code that must observe catalog changes made + * concurrently while it was parked at the injection point. + */ +void +injection_wait_accept_invalidations(const char *name, const void *private_data, + void *arg) +{ + injection_wait(name, private_data, arg); + AcceptInvalidationMessages(); +} + /* * SQL function for creating an injection point. */ @@ -308,6 +325,8 @@ injection_points_attach(PG_FUNCTION_ARGS) function = "injection_notice"; else if (strcmp(action, "wait") == 0) function = "injection_wait"; + else if (strcmp(action, "wait-accept-invalidations") == 0) + function = "injection_wait_accept_invalidations"; else elog(ERROR, "incorrect action \"%s\" for injection point creation", action); diff --git a/src/test/subscription/meson.build b/src/test/subscription/meson.build index e71e95c6297..1c233fbd08a 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_sequence_concurrent_drop.pl', 't/100_bugs.pl', ], }, diff --git a/src/test/subscription/t/039_sequence_concurrent_drop.pl b/src/test/subscription/t/039_sequence_concurrent_drop.pl new file mode 100644 index 00000000000..f95565f1ed7 --- /dev/null +++ b/src/test/subscription/t/039_sequence_concurrent_drop.pl @@ -0,0 +1,69 @@ + +# Copyright (c) 2025-2026, PostgreSQL Global Development Group + +# Drop a sequence on the publisher while the sequencesync worker fetches it. +# The subscriber must treat it as a concurrent drop, not crash. +use strict; +use warnings FATAL => 'all'; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +if ($ENV{enable_injection_points} ne 'yes') +{ + plan skip_all => 'Injection points not supported by this build'; +} + +my $node_publisher = PostgreSQL::Test::Cluster->new('publisher'); +$node_publisher->init(allows_streaming => 'logical'); +$node_publisher->start; + +if (!$node_publisher->check_extension('injection_points')) +{ + plan skip_all => 'Extension injection_points not installed'; +} + +my $node_subscriber = PostgreSQL::Test::Cluster->new('subscriber'); +$node_subscriber->init; +$node_subscriber->start; + +$node_publisher->safe_psql('postgres', "CREATE EXTENSION injection_points"); +$node_publisher->safe_psql('postgres', "CREATE SEQUENCE regress_s1"); +$node_subscriber->safe_psql('postgres', "CREATE SEQUENCE regress_s1"); + +# Park the publisher's sequence fetch right before the privilege lookup, so we +# can drop the sequence while the fetch is in flight. The action processes the +# drop's invalidations on resume, so the fetch observes the missing sequence. +$node_publisher->safe_psql('postgres', + "SELECT injection_points_attach('has-sequence-privilege-before-relkind', 'wait-accept-invalidations')" +); + +my $publisher_connstr = $node_publisher->connstr . ' dbname=postgres'; +$node_publisher->safe_psql('postgres', + "CREATE PUBLICATION regress_seq_pub FOR ALL SEQUENCES"); +$node_subscriber->safe_psql('postgres', + "CREATE SUBSCRIPTION regress_seq_sub CONNECTION '$publisher_connstr' PUBLICATION regress_seq_pub" +); + +# Wait until the fetch is parked at the injection point. +$node_publisher->poll_query_until('postgres', + "SELECT count(*) > 0 FROM pg_stat_activity WHERE wait_event = 'has-sequence-privilege-before-relkind'" +) or die "Timed out waiting for the sequence fetch to reach the injection point"; + +# Drop the sequence, then release the fetch. has_sequence_privilege() now +# returns NULL for the vanished sequence. +$node_publisher->safe_psql('postgres', "DROP SEQUENCE regress_s1"); + +my $log_offset = -s $node_subscriber->logfile; +$node_publisher->safe_psql('postgres', + "SELECT injection_points_wakeup('has-sequence-privilege-before-relkind')"); + +# Subscriber must stay up and report the sequence as missing, not crash. +$node_subscriber->wait_for_log( + qr/WARNING: ( [A-Z0-9]+:)? missing sequence on publisher \("public.regress_s1"\)/, + $log_offset); + +is( $node_subscriber->safe_psql('postgres', "SELECT 1"), + '1', 'subscriber survived the concurrent sequence drop'); + +done_testing(); -- 2.54.0