Re: Synchronizing slots from primary to standby

Hi,

On 9/19/23 6:50 AM, shveta malik wrote:
> On Wed, Sep 13, 2023 at 5:19 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>>
>> On Wed, Sep 13, 2023 at 4:54 PM shveta malik <shveta.malik@gmail.com> wrote:
>>>
>>> PFA  v17. It has below changes:
>>>
>>
>> @@ -2498,6 +2500,13 @@ ReorderBufferProcessTXN(ReorderBuffer *rb,
>> ReorderBufferTXN *txn,
>>    }
>>    else
>>    {
>> + /*
>> + * Before we send out the last set of changes to logical decoding
>> + * output plugin, wait for specified streaming replication standby
>> + * servers (if any) to confirm receipt of WAL upto commit_lsn.
>> + */
>> + WaitForStandbyLSN(commit_lsn);
>>
>> It seems the first patch has a wait logic for every commit. I think it
>> is better to integrate this wait with WalSndWaitForWal() as suggested
>> by Andres in his email[1].
>>
>> [1] - https://www.postgresql.org/message-id/20220207204557.74mgbhowydjco4mh%40alap3.anarazel.de
>>
>> --
> 
> Sure Amit. PFA  v18. It addresses below:
> 
> 1) patch001: wait for physical-standby confirmation logic is now
> integrated with WalSndWaitForWal(). Now walsender waits for physical
> standby's confirmation to take changes upto RecentFlushPtr in
> WalSndWaitForWal(). This allows walsender to send the changes to
> logical subscribers one by one which are already covered in
> RecentFlushPtr without needing to wait on every commit for physical
> standby confirmation.

+       /* XXX: Is waiting for 1 second before retrying enough or more or less? */
+       (void) WaitLatch(MyLatch,
+                                        WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
+                                        1000L,
+                                        WAIT_EVENT_WAL_SENDER_WAIT_FOR_STANDBY_CONFIRMATION);

I think it would be better to let the physical walsender(s) wake up those logical
walsender(s) (instead of waiting for 1 sec or such). Maybe we could introduce a new CV that would
broadcast in PhysicalConfirmReceivedLocation() when restart_lsn is changed, what do you think?

Still regarding preventing the logical replication to go ahead of
physical replication standbys specified in standby_slot_names: we currently don't impose this
limitation to pg_logical_slot_get_changes and friends (that don't start a dedicated walsender).

Shouldn't we also prevent them to go ahead of physical replication standbys specified in standby_slot_names?

Regards,

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