public inbox for [email protected]
help / color / mirror / Atom feedFrom: [email protected] <[email protected]>
To: 'vignesh C' <[email protected]>
Cc: Greg Nancarrow <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: Mark Dilger <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: Smith, Peter <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: RE: Optionally automatically disable logical replication subscriptions on error
Date: Tue, 30 Nov 2021 12:13:29 +0000
Message-ID: <TYCPR01MB83730337233D33175E66FB77ED679@TYCPR01MB8373.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <CALDaNm1CxXF0c8a3jZWC8W3C9bhN7xjcY_3_dtGT25-forCEdg@mail.gmail.com>
References: <[email protected]>
<CAA4eK1KkhsNAW3=XxOdVRD9+RcQG5PRRFBFzAMibMG0YFFmAmg@mail.gmail.com>
<[email protected]>
<CAA4eK1KM23D06_rUEsgnTFUpABoeCCoTun2TMrQzWgxWOYY7fA@mail.gmail.com>
<[email protected]>
<CAD21AoB+QH4or-QDiakALZp6pkXiAaGxfGcsvNYJoDHk9kknZw@mail.gmail.com>
<[email protected]>
<CAD21AoCPSU-MBHkortBnELO7Hu0tLsX5GoOEbSYzyxKBPkKvBg@mail.gmail.com>
<TYCPR01MB837384DA6C879A709EBF30D1ED8B9@TYCPR01MB8373.jpnprd01.prod.outlook.com>
<CALDaNm0bXsyrXg3dQGbAhkUDGKnFXtM6mKk8d-8VFLo6wz3ZJA@mail.gmail.com>
<TYCPR01MB8373934498C93D1A1466A2E3ED939@TYCPR01MB8373.jpnprd01.prod.outlook.com>
<CAJcOf-ci4NeNwowE=BmtSWip2NRgZdGe-VFjvSDY2JP0NNLTMQ@mail.gmail.com>
<TYCPR01MB8373D8A622ED51CDEF3DAF5DED949@TYCPR01MB8373.jpnprd01.prod.outlook.com>
<CALDaNm0PdzqVSy0uUZ+UKd9rQeQONUjkALj3Ge+fAmdw0cSLgA@mail.gmail.com>
<TYCPR01MB8373771371B31E1E6CC74B0AED999@TYCPR01MB8373.jpnprd01.prod.outlook.com>
<CAJcOf-f6oqtKt2CD-KcUH00=3+Bybym7magDenegOao2YE-SZQ@mail.gmail.com>
<TYCPR01MB837308EA9546476CFA0C53F0ED9B9@TYCPR01MB8373.jpnprd01.prod.outlook.com>
<CALDaNm2g8E-cF+6DzzMe8zdeCt6Ktd454_1UFBRA+2jE3si3Zg@mail.gmail.com>
<TYCPR01MB837398F781D4B21A94B4EDE0ED639@TYCPR01MB8373.jpnprd01.prod.outlook.com>
<CALDaNm1CxXF0c8a3jZWC8W3C9bhN7xjcY_3_dtGT25-forCEdg@mail.gmail.com>
On Monday, November 29, 2021 2:38 PM vignesh C <[email protected]>
> Thanks for the updated patch, Few comments:
Thank you for your review !
> 1) Since this function is used only from 027_disable_on_error and not used by
> others, this can be moved to 027_disable_on_error:
> +sub wait_for_subscriptions
> +{
> + my ($self, $dbname, @subscriptions) = @_;
> +
> + # Unique-ify the subscriptions passed by the caller
> + my %unique = map { $_ => 1 } @subscriptions;
> + my @unique = sort keys %unique;
> + my $unique_count = scalar(@unique);
> +
> + # Construct a SQL list from the unique subscription names
> + my @escaped = map { s/'/''/g; s/\\/\\\\/g; $_ } @unique;
> + my $sublist = join(', ', map { "'$_'" } @escaped);
> +
> + my $polling_sql = qq(
> + SELECT COUNT(1) = $unique_count FROM
> + (SELECT s.oid
> + FROM pg_catalog.pg_subscription s
> + LEFT JOIN pg_catalog.pg_subscription_rel
> sr
> + ON sr.srsubid = s.oid
> + WHERE (sr IS NULL OR sr.srsubstate IN
> ('s', 'r'))
> + AND s.subname IN ($sublist)
> + AND s.subenabled IS TRUE
> + UNION
> + SELECT s.oid
> + FROM pg_catalog.pg_subscription s
> + WHERE s.subname IN ($sublist)
> + AND s.subenabled IS FALSE
> + ) AS synced_or_disabled
> + );
> + return $self->poll_query_until($dbname, $polling_sql); }
Fixed.
> 2) The empty line after comment is not required, it can be removed
> +# Create non-unique data in both schemas on the publisher.
> +#
> +for $schema (@schemas)
> +{
Fixed.
> 3) Similarly it can be changed across the file
> +# Wait for the initial subscription synchronizations to finish or fail.
> +#
> +$node_subscriber->wait_for_subscriptions('postgres', @schemas)
> + or die "Timed out while waiting for subscriber to synchronize
> +data";
>
> +# Enter unique data for both schemas on the publisher. This should
> +succeed on # the publisher node, and not cause any additional problems
> +on the subscriber # side either, though disabled subscription "s1" should not
> replicate anything.
> +#
> +for $schema (@schemas)
Fixed.
> 4) Since subid is used only at one place, no need of subid variable, you could
> replace subid with subform->oid in LockSharedObject
> + Datum values[Natts_pg_subscription];
> + HeapTuple tup;
> + Oid subid;
> + Form_pg_subscription subform;
>
> + subid = subform->oid;
> + LockSharedObject(SubscriptionRelationId, subid, 0,
> + AccessExclusiveLock);
Fixed.
> 5) "permissions errors" should be "permission errors"
> + Specifies whether the subscription should be automatically
> disabled
> + if replicating data from the publisher triggers non-transient errors
> + such as referential integrity or permissions errors. The default is
> + <literal>false</literal>.
> + </para>
Fixed.
The new patch v8 is shared in [1].
[1] - https://www.postgresql.org/message-id/TYCPR01MB83735AA021E0F614A3AB3221ED679%40TYCPR01MB8373.jpnprd0...
Best Regards,
Takamichi Osumi
view thread (74+ 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], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: RE: Optionally automatically disable logical replication subscriptions on error
In-Reply-To: <TYCPR01MB83730337233D33175E66FB77ED679@TYCPR01MB8373.jpnprd01.prod.outlook.com>
* 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