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

On Wed, May 10, 2023 at 7:33 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> 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?

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