On Thu, Feb 17, 2022 at 4:12 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Mon, Jan 31, 2022 at 6:18 PM Ajin Cherian <itsa...@gmail.com> wrote: > > > > Few comments: > ============= >
One more comment: @@ -1546,10 +1557,11 @@ WalSndWaitForWal(XLogRecPtr loc) * otherwise idle, this keepalive will trigger a reply. Processing the * reply will update these MyWalSnd locations. */ - if (MyWalSnd->flush < sentPtr && + if (force_keepalive_syncrep || + (MyWalSnd->flush < sentPtr && MyWalSnd->write < sentPtr && - !waiting_for_ping_response) - WalSndKeepalive(false); + !waiting_for_ping_response)) + WalSndKeepalive(false); Will this allow syncrep to proceed in case we are skipping the transaction? Won't we need to send a feedback message with 'requestReply' true in this case as we release syncrep waiters while processing standby message, see ProcessStandbyReplyMessage->SyncRepReleaseWaiters. Without 'requestReply', the subscriber might not send any message and the syncrep won't proceed. Why do you decide to delay sending this message till WalSndWaitForWal()? It may not be called for each transaction. I feel we should try to device a test case to test this sync replication mechanism such that without this particular change the sync rep transaction waits momentarily but with this change it doesn't wait. I am not entirely sure whether we can devise an automated test as this is timing related issue but I guess we can at least manually try to produce a case. -- With Regards, Amit Kapila.
