RE: Optionally automatically disable logical replication subscriptions on error - Mailing list pgsql-hackers

From osumi.takamichi@fujitsu.com
Subject RE: Optionally automatically disable logical replication subscriptions on error
Date
Msg-id TYCPR01MB8373771371B31E1E6CC74B0AED999@TYCPR01MB8373.jpnprd01.prod.outlook.com
Whole thread Raw
In response to Re: Optionally automatically disable logical replication subscriptions on error  (vignesh C <vignesh21@gmail.com>)
Responses Re: Optionally automatically disable logical replication subscriptions on error
List pgsql-hackers
On Friday, November 12, 2021 1:09 PM vignesh C <vignesh21@gmail.com> wrote:
> On Thu, Nov 11, 2021 at 2:50 PM osumi.takamichi@fujitsu.com
> <osumi.takamichi@fujitsu.com> wrote:
> Thanks for the updated patch, Few comments:
> 1)  tab completion should be added for disable_on_error:
> /* Complete "CREATE SUBSCRIPTION <name> ...  WITH ( <opt>" */ else if
> (HeadMatches("CREATE", "SUBSCRIPTION") && TailMatches("WITH", "("))
> COMPLETE_WITH("binary", "connect", "copy_data", "create_slot",
>   "enabled", "slot_name", "streaming",
>   "synchronous_commit", "two_phase");
Fixed.

> 2) disable_on_error is supported by alter subscription, the same should be
> documented:
> @ -871,11 +886,19 @@ AlterSubscription(ParseState *pstate,
> AlterSubscriptionStmt *stmt,
>                         {
>                                 supported_opts = (SUBOPT_SLOT_NAME |
> 
> SUBOPT_SYNCHRONOUS_COMMIT | SUBOPT_BINARY |
> -
> SUBOPT_STREAMING);
> +
> SUBOPT_STREAMING | SUBOPT_DISABLE_ON_ERR);
> 
>                                 parse_subscription_options(pstate,
> stmt->options,
> 
>             supported_opts, &opts);
> 
> +                               if (IsSet(opts.specified_opts,
> SUBOPT_DISABLE_ON_ERR))
> +                               {
> +
> values[Anum_pg_subscription_subdisableonerr - 1]
> +                                               =
> BoolGetDatum(opts.disableonerr);
> +
> replaces[Anum_pg_subscription_subdisableonerr - 1]
> +                                               = true;
> +                               }
> +
Fixed the documentation. Also, add one test for alter subscription.

 
> 3) Describe subscriptions (dRs+) should include displaying of disableonerr:
> \dRs+ sub1
>                                                       List of subscriptions
> Name |  Owner  | Enabled | Publication | Binary | Streaming | Two
> phase commit | Synchronous commit |         Conninfo
> ------+---------+---------+-------------+--------+-----------+----------
> --------+--------------------+---------------------------
>  sub1 | vignesh | t       | {pub1}      | f      | f         | d
>          | off                | dbname=postgres port=5432
> (1 row)
Fixed.


> 4) I felt transicent should be transient, might be a typo:
> +          Specifies whether the subscription should be automatically
> disabled
> +          if replicating data from the publisher triggers non-transicent errors
> +          such as referential integrity or permissions errors. The default is
> +          <literal>false</literal>.
Fixed.

> 5) The commented use PostgresNode and use TestLib can be removed:
> +# Test of logical replication subscription self-disabling feature use
> +strict; use warnings; # use PostgresNode; # use TestLib; use
> +PostgreSQL::Test::Cluster; use PostgreSQL::Test::Utils; use Test::More
> +tests => 10;
Removed.


Also, my colleague Greg provided an offlist patch to me and
I've incorporated his suggested modifications into this version.
So, I noted his name as a coauthor.

C codes are checked by pgindent again.

This v5 depends on v23 skip xid in [1].

[1] - https://www.postgresql.org/message-id/CAD21AoA5jupM6O%3DpYsyfaxQ1aMX-en8%3DQNgpW6KfXsg7_CS0CQ%40mail.gmail.com


Best Regards,
    Takamichi Osumi


Attachment

pgsql-hackers by date:

Previous
From: Masahiko Sawada
Date:
Subject: Re: Showing I/O timings spent reading/writing temp buffers in EXPLAIN
Next
From: "osumi.takamichi@fujitsu.com"
Date:
Subject: RE: Optionally automatically disable logical replication subscriptions on error