From: | shveta malik <shveta(dot)malik(at)gmail(dot)com> |
---|---|
To: | Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> |
Cc: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Ajin Cherian <itsajin(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, shveta malik <shveta(dot)malik(at)gmail(dot)com> |
Subject: | Re: Synchronizing slots from primary to standby |
Date: | 2024-01-17 10:30:17 |
Message-ID: | CAJpy0uBc6H22RbNVU213AZ_BPw+uDptcTVHakKegpTCv74yN=w@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Jan 17, 2024 at 3:08 PM Bertrand Drouvot
<bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
>
> 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
PFA v63.
--It addresses comments by Peter given in [1], [2], comment by Nisha
given in [3], comments by Bertrand given in [4]
--It also moves race-condition fix from patch003 to patch002 as
suggested by Swada-san offlist. Race-condition is mentioned in [5]
All the changes are in patch02, patch003 and patch006.
[1]: https://www.postgresql.org/message-id/CAHut%2BPuECB8fNBfXMdTHSMKF9kL%3D0XqPw1Am4NVahfJSSHzoYg%40mail.gmail.com
[2]: https://www.postgresql.org/message-id/CAHut%2BPt0uum%2B6Hg5UDofWMEJWhVEyArM1b0_B94UJmRcQmz7DA%40mail.gmail.com
[3]: https://www.postgresql.org/message-id/CABdArM73qdHyA0nteDLAQrfKNHRP%2B5Qq6p8uobg5bkE3EWiC%2Bg%40mail.gmail.com
[4]: https://www.postgresql.org/message-id/ZaegJe9JpUiQeV%2BD%40ip-10-97-1-34.eu-west-3.compute.internal
[5]: https://www.postgresql.org/message-id/CAD21AoA5izeKpp9Ei4Cd745pKX3wn-TRvhhmPFEW9UY1nx%2B_aw%40mail.gmail.com
thanks
Shveta
Attachment | Content-Type | Size |
---|---|---|
v63-0004-Allow-logical-walsenders-to-wait-for-the-physica.patch | application/octet-stream | 40.3 KB |
v63-0003-Slot-sync-worker-as-a-special-process.patch | application/octet-stream | 35.8 KB |
v63-0001-Enable-setting-failover-property-for-a-slot-thro.patch | application/octet-stream | 100.3 KB |
v63-0005-Non-replication-connection-and-app_name-change.patch | application/octet-stream | 9.7 KB |
v63-0002-Add-logical-slot-sync-capability-to-the-physical.patch | application/octet-stream | 83.3 KB |
v63-0006-Document-the-steps-to-check-if-the-standby-is-re.patch | application/octet-stream | 9.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | shveta malik | 2024-01-17 10:36:19 | Re: Synchronizing slots from primary to standby |
Previous Message | Alvaro Herrera | 2024-01-17 09:50:59 | Re: Add pgindent test to check if codebase is correctly formatted |