public inbox for [email protected]  

help / color / mirror / Atom feed

From: Greg Nancarrow <[email protected]>
To: [email protected] <[email protected]>
Cc: vignesh C <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: Mark Dilger <[email protected]>
Cc: Smith, Peter <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Optionally automatically disable logical replication subscriptions on error
Date: Thu, 16 Dec 2021 16:32:08 +1100
Message-ID: <CAJcOf-fY7VspwzN2PPDQq1c1DpvhxHuDv4DWq+=zDWb_60u-rQ@mail.gmail.com> (raw)
In-Reply-To: <TYCPR01MB8373DEBEB6EC28CB82F50D47ED759@TYCPR01MB8373.jpnprd01.prod.outlook.com>
References: <TYCPR01MB83732011D83A027C32A2802CED6A9@TYCPR01MB8373.jpnprd01.prod.outlook.com>
	<CAJcOf-dkEVtRstWMchEAjb02xp_5cg_5g_0GUCthUx+o-bYdXg@mail.gmail.com>
	<TYCPR01MB83737F77BBE63432611CE7C8ED6D9@TYCPR01MB8373.jpnprd01.prod.outlook.com>
	<CALDaNm1O8iBHSjfkv2-Ld6B7C4ay8F3-O1Q2QHVrM8H7Zu40iA@mail.gmail.com>
	<TYCPR01MB8373DEBEB6EC28CB82F50D47ED759@TYCPR01MB8373.jpnprd01.prod.outlook.com>

On Tue, Dec 14, 2021 at 4:34 PM [email protected]
<[email protected]> wrote:
>
> Besides all of those changes, I've removed the obsolete
> comment of DisableSubscriptionOnError in v12.
>

I have a few minor comments, otherwise the patch LGTM at this point:

doc/src/sgml/catalogs.sgml
(1)
Current comment says:

+       If true, the subscription will be disabled when subscription's
+       worker detects any errors

However, in create_subscription.sgml, it says "disabled if any errors
are detected by subscription workers ..."

For consistency, I think it should be:

+       If true, the subscription will be disabled when subscription
+       workers detect any errors

src/bin/psql/describe.c
(2)
I think that:

+  gettext_noop("Disable On Error"));

should be:

+  gettext_noop("Disable on error"));

for consistency with the uppercase/lowercase usage on other similar entries?
(e.g. "Two phase commit")


src/include/catalog/pg_subscription.h
(3)

+  bool subdisableonerr; /* True if apply errors should disable the
+                                       * subscription upon error */

The comment should just say "True if occurrence of apply errors should
disable the subscription"


Regards,
Greg Nancarrow
Fujitsu Australia

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: <CAJcOf-fY7VspwzN2PPDQq1c1DpvhxHuDv4DWq+=zDWb_60u-rQ@mail.gmail.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