On Friday, February 9, 2024 6:45 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Fri, Feb 9, 2024 at 10:00 AM Zhijie Hou (Fujitsu) <houzj.f...@fujitsu.com> > wrote: > > > > Few comments on 0001 > =================== > 1. Shouldn't pg_sync_replication_slots() check whether the user has > replication > privilege?
Yes, added. > > 2. The function declarations in slotsync.h don't seem to be in the same order > as > they are defined in slotsync.c. For example, see ValidateSlotSyncParams(). The > same is true for new functions exposed via walreceiver.h and walsender.h. > Please > check the patch for other such inconsistencies. I reordered the function declarations. > > 3. > +# Wait for the standby to finish sync > +$standby1->wait_for_log( > + qr/LOG: ( [A-Z0-9]+:)? newly created slot \"lsub1_slot\" is sync-ready > +now/, $offset); > + > +$standby1->wait_for_log( > + qr/LOG: ( [A-Z0-9]+:)? newly created slot \"lsub2_slot\" is sync-ready > +now/, $offset); > + > +# Confirm that the logical failover slots are created on the standby > +and are # flagged as 'synced' > +is($standby1->safe_psql('postgres', > + q{SELECT count(*) = 2 FROM pg_replication_slots WHERE slot_name IN > ('lsub1_slot', 'lsub2_slot') AND synced;}), > + "t", > + 'logical slots have synced as true on standby'); > > Isn't the last test that queried pg_replication_slots sufficient? I think > wait_for_log() would be required for slotsync worker or am I missing > something? I think it's not needed in 0001, so removed. > Apart from the above, I have modified a few comments in the attached. Thanks, it looks good to me, so applied. Attach the V83 patch which addressed Peter[1][2], Amit and Sawada-san's[3] comments. Only 0001 is sent in this version, we will send other patches after rebasing. [1] https://www.postgresql.org/message-id/CAHut%2BPvW8s6AYD2UD0xadM%2B3VqBkXP2LjD30LEGRkHUa-Szm%2BQ%40mail.gmail.com [2] https://www.postgresql.org/message-id/CAHut%2BPv88vp9mNxX37c_Bc5FDBsTS%2BdhV02Vgip9Wqwh7GBYSg%40mail.gmail.com [3] https://www.postgresql.org/message-id/CAD21AoDvyLu%3D2-mqfGn_T_3jUamR34w%2BsxKvYnVzKqTCpyq_FQ%40mail.gmail.com Best Regards, Hou zj
v83-0001-Add-a-slot-synchronization-function.patch
Description: v83-0001-Add-a-slot-synchronization-function.patch