public inbox for [email protected]  
help / color / mirror / Atom feed
From: Amit Kapila <[email protected]>
To: Peter Smith <[email protected]>
Cc: Zhijie Hou (Fujitsu) <[email protected]>
Cc: shveta malik <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: Ajin Cherian <[email protected]>
Cc: Dilip Kumar <[email protected]>
Cc: Nisha Moond <[email protected]>
Cc: Hayato Kuroda (Fujitsu) <[email protected]>
Cc: Bertrand Drouvot <[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: Alvaro Herrera <[email protected]>
Subject: Re: Synchronizing slots from primary to standby
Date: Tue, 27 Feb 2024 18:05:14 +0530
Message-ID: <CAA4eK1LDF8jyJS8yUkyySjP2BdQj_3-C5_5AqdyxoKF_5W1P0g@mail.gmail.com> (raw)
In-Reply-To: <CAHut+PsGTQb0ijYOfd=x=qX5Aubj=F-pqnfjQ47Jt3MeyBipqA@mail.gmail.com>
References: <CAJpy0uBSbBWAWzb2j3OSSGDwhUBZNDAuMWH9B=A9xtVXdOLnCw@mail.gmail.com>
	<Zdcqb3Ok+B/[email protected]>
	<CAA4eK1KcQSk7wzC7Zfrth9OhrjW2HvxL4tKgU42qqH7p6jn+FA@mail.gmail.com>
	<CAA4eK1Lzt6nT6Vrjq+4FEEVE1toK=9sN6vJrGWCziwW-7k8Xpg@mail.gmail.com>
	<OS0PR01MB5716E563BFFD591D1F414F4294562@OS0PR01MB5716.jpnprd01.prod.outlook.com>
	<OS0PR01MB5716383F68248912EF8D7CE594552@OS0PR01MB5716.jpnprd01.prod.outlook.com>
	<OS0PR01MB57161DE4BDB3194C496546A694552@OS0PR01MB5716.jpnprd01.prod.outlook.com>
	<OS0PR01MB5716E2CCA142FA07A294D98C94552@OS0PR01MB5716.jpnprd01.prod.outlook.com>
	<CAJpy0uAW3GDeCUuOUqdYQymX3TLyjEKQYLUwG9tUgjYJtZYq2w@mail.gmail.com>
	<OS3PR01MB5718FDA975F9694D8BA7B9A794552@OS3PR01MB5718.jpnprd01.prod.outlook.com>
	<[email protected]>
	<CAA4eK1LnLvM2+-2YfQBk6UHYzdEdNtTuhEunxOO3_n-TqDR0ew@mail.gmail.com>
	<OS0PR01MB571654A0288C53A676051AE794592@OS0PR01MB5716.jpnprd01.prod.outlook.com>
	<CAHut+PsGTQb0ijYOfd=x=qX5Aubj=F-pqnfjQ47Jt3MeyBipqA@mail.gmail.com>

On Tue, Feb 27, 2024 at 12:48 PM Peter Smith <[email protected]> wrote:
>
> Here are some review comments for v99-0001
>
> ==========
> 0. GENERAL.
>
> +#standby_slot_names = '' # streaming replication standby server slot names that
> + # logical walsender processes will wait for
>
> IMO the GUC name is too generic. There is nothing in this name to
> suggest it has anything to do with logical slot synchronization; that
> meaning is only found in the accompanying comment -- it would be
> better if the GUC name itself were more self-explanatory.
>
> e.g. Maybe like 'wal_sender_sync_standby_slot_names' or
> 'wal_sender_standby_slot_names', 'wal_sender_wait_for_standby_slots',
> or ...
>

It would be wrong and or misleading to append wal_sender to this GUC
name as this is used during SQL APIs as well. Also, adding wait sounds
more like a boolean. So, I don't see the proposed names any better
than the current one.

> ~~~
>
> 9.
> + /* Now verify if the specified slots really exist and have correct type */
> + if (!validate_standby_slots(newval))
> + return false;
>
> As in a prior comment, if ReplicationSlotCtl is NULL then it is not
> always going to do exactly what that comment says it is doing...
>

It will do what the comment says when invoked as part of the SIGHUP
signal. I think the current comment is okay.

-- 
With Regards,
Amit Kapila.






view thread (68+ 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: <CAA4eK1LDF8jyJS8yUkyySjP2BdQj_3-C5_5AqdyxoKF_5W1P0g@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