public inbox for [email protected]  

help / color / mirror / Atom feed
From: Bertrand Drouvot <[email protected]>
To: shveta malik <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: Zhijie Hou (Fujitsu) <[email protected]>
Cc: Dilip Kumar <[email protected]>
Cc: Peter Smith <[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: Wed, 17 Jan 2024 09:38:45 +0000
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAJpy0uCJ7MKgZnKLAACd-AXN0VbWM5gVJ+GRJ1Za_A2UmF3R0A@mail.gmail.com>
References: <CAA4eK1L74EUU7+Dr11joY+Xact_MsjZu9G12sX+vRUWJsptP8Q@mail.gmail.com>
	<CAA4eK1+ORhnb2zUUpO_FcgWUUbwvQX5+=tdCehx7x2nB2x8VMg@mail.gmail.com>
	<CAD21AoCoX+jhy_i3v+T2s78NG_0HH1oXOUiTOWhDdxVPBtDHKA@mail.gmail.com>
	<CAJpy0uAwz9iNw3=5QC8S3z8Bf-w0z27rE32SFxjDMer7aTQMPg@mail.gmail.com>
	<CAA4eK1+q8R=yEpTMiOZ_Xksdd_Jg3SXvBMxRutHQtkCbdeK_yQ@mail.gmail.com>
	<CAJpy0uAU+nzufQwSzyB2YmhPHHn71_=1zkauP4Ra32a_eqbcig@mail.gmail.com>
	<CAA4eK1KtwFg6Mr1MRnHUUfPotzc2=5UCTq_D2t8x0PM1Lyhk8g@mail.gmail.com>
	<CAD21AoA03f0QKYjiSymGcnROvh1X-3YSqyB-19mkRXt5dfaxFw@mail.gmail.com>
	<CAJpy0uCneiKeNuz3B91Wf2LRxAenD3fpaxYfdL09y9qHmvVp+w@mail.gmail.com>
	<CAJpy0uCJ7MKgZnKLAACd-AXN0VbWM5gVJ+GRJ1Za_A2UmF3R0A@mail.gmail.com>

Hi,

On Tue, Jan 16, 2024 at 05:27:05PM +0530, shveta malik wrote:
> PFA v62. Details:

Thanks! 

> v62-003:
> It is a new patch which attempts to implement slot-sync worker as a
> special process which is neither a bgworker nor an Auxiliary process.
> Here we get the benefit of converting enable_syncslot to a PGC_SIGHUP
> Guc rather than PGC_POSTMASTER. We launch the slot-sync worker only if
> it is hot-standby and 'enable_syncslot' is ON.

The implementation looks reasonable to me (from what I can see some parts is
copy/paste from an already existing "special" process and some parts are
"sync slot" specific) which makes fully sense.

A few remarks:

1 ===
+                * Was it the slot sycn worker?

Typo: sycn

2 ===
+                * ones), and no walwriter, autovac launcher or bgwriter or slot sync

Instead? "* ones), and no walwriter, autovac launcher, bgwriter or slot sync"

3 ===
+ * restarting slot slyc worker. If stopSignaled is set, the worker will

Typo: slyc

4 ===
+/* Flag to tell if we are in an slot sync worker process */

s/an/a/ ?

5 === (coming from v62-0002)
+       Assert(tuplestore_tuple_count(res->tuplestore) == 1);

Is it even possible for the related query to not return only one row? (I think the 
"count" ensures it).

6 ===
        if (conninfo_changed ||
                primary_slotname_changed ||
+               old_enable_syncslot != enable_syncslot ||
                (old_hot_standby_feedback != hot_standby_feedback))
        {
                ereport(LOG,
                                errmsg("slot sync worker will restart because of"
                                           " a parameter change"));

I don't think "slot sync worker will restart" is true if one change enable_syncslot
from on to off.

IMHO, v62-003 is in good shape and could be merged in v62-002 (that would ease
the review). But let's wait to see if others think differently.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com





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: <[email protected]>

* 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