On Wed, May 10, 2023 at 7:33 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Wed, May 10, 2023 at 3:41 PM Zhijie Hou (Fujitsu) > <houzj.f...@fujitsu.com> wrote: > > > > On Wednesday, May 10, 2023 2:36 PM Bharath Rupireddy > > <bharath.rupireddyforpostg...@gmail.com> wrote: > > > > > > > > > > My current guess is that mis-using a condition variable is the best > > > > bet. I > > > > think it should work to use ConditionVariablePrepareToSleep() before a > > > > WalSndWait(), and then ConditionVariableCancelSleep(). I.e. to never use > > > > ConditionVariableSleep(). The latch set from > > > > ConditionVariableBroadcast() > > > > would still cause the necessary wakeup. > > > > > > How about something like the attached? Recovery and subscription tests > > > don't complain with the patch. > > > > Thanks for the patch. I noticed one place where the logic is different from > > before and > > just to confirm: > > > > if (AllowCascadeReplication()) > > - WalSndWakeup(switchedTLI, true); > > + ConditionVariableBroadcast(&WalSndCtl->cv); > > > > After the change, we wakeup physical walsender regardless of switchedTLI > > flag. > > Is this intentional ? if so, I think It would be better to update the > > comments above this. > > > > This raises the question of whether we need this condition variable > logic for physical walsenders?
It sounds like a good idea. We can have two condition variables for logical and physical walsenders, and selectively wake up walsenders sleeping on the condition variables. It should work, it seems like much of a hack, though. Regards, [1] https://www.postgresql.org/message-id/2d314c22b9e03415aa1c7d8fd1f698dae60effa7.camel%40j-davis.com -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
