Re: walsender performance regression due to logical decoding on standby changes

On Wed, May 10, 2023 at 3:41 PM Zhijie Hou (Fujitsu)
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On Wednesday, May 10, 2023 2:36 PM Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)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?

--
With Regards,
Amit Kapila.