public inbox for [email protected]  
help / color / mirror / Atom feed
From: Peter Smith <[email protected]>
To: shveta malik <[email protected]>
Cc: Bertrand Drouvot <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: Zhijie Hou (Fujitsu) <[email protected]>
Cc: Dilip Kumar <[email protected]>
Cc: Nisha Moond <[email protected]>
Cc: Hayato Kuroda (Fujitsu) <[email protected]>
Cc: Bharath Rupireddy <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Bruce Momjian <[email protected]>
Cc: Ashutosh Sharma <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: pgsql-hackers <[email protected]>
Cc: Ajin Cherian <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Subject: Re: Synchronizing slots from primary to standby
Date: Thu, 25 Jan 2024 12:37:09 +1100
Message-ID: <CAHut+Pt2oLfxv_=GN23dOOduKHBHdAkCvwSZiwSbtTJFFbQm-w@mail.gmail.com> (raw)
In-Reply-To: <CAJpy0uALV+-dHZohApDPpcEXCdNhF9PFbT5BNr4WLGXZ-qHaOg@mail.gmail.com>
References: <CAJpy0uCJ7MKgZnKLAACd-AXN0VbWM5gVJ+GRJ1Za_A2UmF3R0A@mail.gmail.com>
	<[email protected]>
	<CAJpy0uBc6H22RbNVU213AZ_BPw+uDptcTVHakKegpTCv74yN=w@mail.gmail.com>
	<CAD21AoBgzONdt3o5mzbQ4MtqAE=WseiXUOq0LMqne-nWGjZBsA@mail.gmail.com>
	<CAJpy0uDfgZcJxTcmnRoiL8zNfRht-iMGb51_CyGJ6ybxjT4H2w@mail.gmail.com>
	<CAA4eK1Jhy1-bsu6vc0=Nja7aw5-EK_=101pnnuM3ATqTA8+=Sg@mail.gmail.com>
	<CAD21AoApGoTZu7D_7=bVYQqKnj+PZ2Rz+nc8Ky1HPQMS_XL6+A@mail.gmail.com>
	<CAA4eK1LMfoDbo4CfoW9tMDWYRZwQ+paF_wrD9=WiXJR3mkm7Pg@mail.gmail.com>
	<CAD21AoBy+6ygad-hBZ=vB86infK4RQhBwPHByNrtB0GCSM7T+Q@mail.gmail.com>
	<CAA4eK1L8Oh2grbT=MyS+mow2LUEF_7Z-idjsyK50Rh24yRujyA@mail.gmail.com>
	<[email protected]>
	<CAJpy0uD2B+S5+r_G7TZ-Z=Dh==8RF3X1_T8ofYf0n=et0ZXtXA@mail.gmail.com>
	<CAJpy0uALV+-dHZohApDPpcEXCdNhF9PFbT5BNr4WLGXZ-qHaOg@mail.gmail.com>

Here are some review comments for the patch v67-0001.

======
1.
There are a couple of places checking for failover usage on a standby.

+ if (RecoveryInProgress() && failover)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot enable failover for a replication slot"
+    " created on the standby"));

and

+ if (RecoveryInProgress() && failover)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot enable failover for a replication slot"
+    " on the standby"));

IMO the conditions should be written the other way around (failover &&
RecoveryInProgress()) to avoid the unnecessary function calls when
'failover' flag is probably mostly default false anyway.

======
Kind Regards,
Peter Smith.
Fujitsu Australia





view thread (119+ 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], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Synchronizing slots from primary to standby
  In-Reply-To: <CAHut+Pt2oLfxv_=GN23dOOduKHBHdAkCvwSZiwSbtTJFFbQm-w@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