public inbox for [email protected]
help / color / mirror / Atom feedFrom: Mark Dilger <[email protected]>
To: Andrew Dunstan <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: Non-superuser subscription owners
Date: Mon, 1 Nov 2021 10:58:25 -0700
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
> On Nov 1, 2021, at 7:18 AM, Andrew Dunstan <[email protected]> wrote:
>
> w.r.t. this:
>
> + On the subscriber, the subscription owner's privileges are
> re-checked for
> + each change record when applied, but beware that a change of
> ownership for a
> + subscription may not be noticed immediately by the replication workers.
> + Changes made on the publisher may be applied on the subscriber as
> + the old owner. In such cases, the old owner's privileges will be
> the ones
> + that matter. Worse still, it may be hard to predict when replication
> + workers will notice the new ownership. Subscriptions created
> disabled and
> + only enabled after ownership has been changed will not be subject to
> this
> + race condition.
>
>
> maybe we should disable the subscription before making such a change and
> then re-enable it?
Right. I commented the code that way because there is a clear concern, but I was uncertain which way around the problem was best.
ALTER SUBSCRIPTION..[ENABLE | DISABLE] do not synchronously start or stop subscription workers. The ALTER command updates the catalog's subenabled field, but workers only lazily respond to that. Disabling and enabling the subscription as part of the OWNER TO would not reliably accomplish anything.
The attached patch demonstrates the race condition. It sets up a publisher and subscriber, and toggles the subscription on and off on the subscriber node, interleaved with inserts and deletes on the publisher node. If the ALTER SUBSCRIPTION commands were synchronous, the test results would be deterministic, with only the inserts performed while the subscription is enabled being replicated, but because the ALTER commands are asynchronous, the results are nondeterministic.
It is unclear that I can make ALTER SUBSCRIPTION..OWNER TO synchronous without redesigning the way workers respond to pg_subscription catalog updates generally. That may be a good project to eventually tackle, but I don't see that it is more important to close the race condition in an OWNER TO than for a DISABLE.
Thoughts?
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Attachments:
[application/octet-stream] alter_subscription_race.patch (3.3K, ../[email protected]/2-alter_subscription_race.patch)
download | inline diff:
diff --git a/src/test/subscription/t/026_race.pl b/src/test/subscription/t/026_race.pl
new file mode 100644
index 0000000000..cbbe6c34e2
--- /dev/null
+++ b/src/test/subscription/t/026_race.pl
@@ -0,0 +1,86 @@
+
+# Copyright (c) 2021, PostgreSQL Global Development Group
+
+# Test subscriptions owned by non-superusers
+use strict;
+use warnings;
+use PostgresNode;
+use TestLib;
+use Test::More;
+
+our $LOOPCOUNT = 100;
+plan tests => 2 * $LOOPCOUNT;
+my $result;
+
+# Setup
+
+my $node_publisher = PostgresNode->new('publisher');
+$node_publisher->init(allows_streaming => 'logical');
+$node_publisher->start;
+
+my $node_subscriber = PostgresNode->new('subscriber');
+$node_subscriber->init(allows_streaming => 'logical');
+$node_subscriber->start;
+
+my $publisher_connstr = $node_publisher->connstr . ' dbname=postgres';
+
+$node_publisher->safe_psql('postgres', qq(
+CREATE TABLE t (i INTEGER);
+ALTER TABLE t REPLICA IDENTITY FULL;
+CREATE PUBLICATION pub FOR TABLE t;
+));
+
+$node_subscriber->safe_psql('postgres', qq(
+CREATE TABLE t (i INTEGER);
+CREATE SUBSCRIPTION sub CONNECTION '$publisher_connstr' PUBLICATION pub;
+CREATE TABLE b (LIKE t);
+));
+
+# Wait for initial sync of all subscriptions.
+my $synced_query =
+ "SELECT count(1) = 0 FROM pg_subscription_rel WHERE srsubstate NOT IN ('r', 's');";
+$node_subscriber->poll_query_until('postgres', $synced_query)
+ or die "Timed out while waiting for subscriber to synchronize data";
+
+# Upon first entry to this loop, the subscription is enabled
+foreach my $loop (1..$LOOPCOUNT)
+{
+ my $even = $loop * 2;
+ my $odd = ($loop * 2) + 1;
+
+ # The subscription is enabled. This even number is fair game for
+ # replication.
+ $node_publisher->safe_psql('postgres', "INSERT INTO t (i) VALUES ($even)");
+
+ # We may replicate the even number before the subscription is disabled, but
+ # maybe not.
+ $node_subscriber->safe_psql('postgres', "ALTER SUBSCRIPTION sub DISABLE");
+
+ # The subscription is now disabled, but workers on the subscriber side may
+ # not have gotten the message yet. As such, this odd number may slip
+ # through.
+ $node_publisher->safe_psql('postgres', "INSERT INTO t (i) VALUES ($odd)");
+
+ # See if we can catch any odd numbers in the subscriber's copy of table t
+ $result = $node_subscriber->safe_psql('postgres',
+ "SELECT COUNT(*) FROM t WHERE i % 2 = 1");
+ is ($result, 0, "No odd numbers in subscriber copy of table t before delete on publisher");
+
+ # Clear out all values on the publisher side, so that the odd number cannot
+ # be replicated after we re-enable the subscription. Of course, it may have
+ # already slipped through, and it is also possible that the even number has
+ # not replicated yet.
+ #
+ # Since the publication only publishes INSERTs, this delete should only have
+ # effect on the publisher side.
+ $node_publisher->safe_psql('postgres', "DELETE FROM t WHERE i = $odd");
+
+ # Enable the subscription for the next loop. Note that at this point, the
+ # table is empty on the publisher side.
+ $node_subscriber->safe_psql('postgres', "ALTER SUBSCRIPTION sub ENABLE");
+
+ # See if we can catch an odd number before the delete is propogated
+ $result = $node_subscriber->safe_psql('postgres',
+ "SELECT COUNT(*) FROM t WHERE i % 2 = 1");
+ is ($result, 0, "No odd numbers in subscriber copy of table t after delete on publisher");
+}
view thread (68+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: Non-superuser subscription owners
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox