RE: Optionally automatically disable logical replication subscriptions on error

From: "osumi(dot)takamichi(at)fujitsu(dot)com" <osumi(dot)takamichi(at)fujitsu(dot)com>
To: 'vignesh C' <vignesh21(at)gmail(dot)com>
Cc: Greg Nancarrow <gregn4422(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, "Smith, Peter" <peters(at)fast(dot)au(dot)fujitsu(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: RE: Optionally automatically disable logical replication subscriptions on error
Date: 2021-11-26 14:36:35
Message-ID: TYCPR01MB837398F781D4B21A94B4EDE0ED639@TYCPR01MB8373.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Monday, November 22, 2021 3:53 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> Few comments:
Thank you so much for your review !

> 1) Changes to handle pg_dump are missing. It should be done in
> dumpSubscription and getSubscriptions
Fixed.

> 2) "And" is missing
> --- a/doc/src/sgml/ref/alter_subscription.sgml
> +++ b/doc/src/sgml/ref/alter_subscription.sgml
> @@ -201,8 +201,8 @@ ALTER SUBSCRIPTION <replaceable
> class="parameter">name</replaceable> RENAME TO <
> information. The parameters that can be altered
> are <literal>slot_name</literal>,
> <literal>synchronous_commit</literal>,
> - <literal>binary</literal>, and
> - <literal>streaming</literal>.
> + <literal>binary</literal>,<literal>streaming</literal>
> + <literal>disable_on_error</literal>.
> Should be:
> - <literal>binary</literal>, and
> - <literal>streaming</literal>.
> + <literal>binary</literal>,<literal>streaming</literal>, and
> + <literal>disable_on_error</literal>.
Fixed.

> 3) Should we change this :
> + 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>.
> to:
> + Specifies whether the subscription should be automatically
> disabled
> + while replicating data from the publisher triggers
> non-transient errors
> + such as referential integrity, permissions errors, etc. The
> default is
> + <literal>false</literal>.
I preferred the previous description. The option
"disable_on_error" works with even one error.
If we use "while", the nuance would be like
we keep disabling a subscription more than once.
This situation happens only when user makes
the subscription enable without resolving the non-transient error,
which sounds a bit unnatural. So, I wanna keep the previous description.
If you are not satisfied with this, kindly let me know.

This v7 uses v26 of skip xid patch [1]

[1] - https://www.postgresql.org/message-id/CAD21AoDNe_O%2BCPucd_jQPu3gGGaCLNP%2BJ_kSPNecTdAM8HFPww%40mail.gmail.com

Best Regards,
Takamichi Osumi

Attachment Content-Type Size
v7-0001-Optionally-disable-subscriptions-on-error.patch application/octet-stream 50.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2021-11-26 15:24:15 Re: pgsql: xlog.c: Remove global variables ReadRecPtr and EndRecPtr.
Previous Message Alvaro Herrera 2021-11-26 13:49:50 Re: prevent immature WAL streaming