MIME-Version: 1.0
References: <20230509190247.3rrplhdgem6su6cg@awork3.anarazel.de>
 <CALj2ACVZyh++dgq5REgmO5-wNtZt=tN4zv=kMqPEARJ4zOpfVg@mail.gmail.com> <OS0PR01MB5716CADAD77D0D720E27607A94779@OS0PR01MB5716.jpnprd01.prod.outlook.com>
In-Reply-To: <OS0PR01MB5716CADAD77D0D720E27607A94779@OS0PR01MB5716.jpnprd01.prod.outlook.com>
From: Amit Kapila <amit.kapila16@gmail.com>
Date: Wed, 10 May 2023 16:03:07 +0530
Message-ID: <CAA4eK1LTcqyaSBdACRM+QyZtDO6zd-40NoqMoxRbW4TQ5yAtUA@mail.gmail.com>
Subject: Re: walsender performance regression due to logical decoding on
 standby changes
To: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>
Cc: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, 
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>, 
	"Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>, Jeff Davis <pgsql@j-davis.com>, 
	Robert Haas <robertmhaas@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, 
	Andres Freund <andres@anarazel.de>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CAA4eK1LTcqyaSBdACRM%2BQyZtDO6zd-40NoqMoxRbW4TQ5yAtUA%40mail.gmail.com>
Precedence: bulk

On Wed, May 10, 2023 at 3:41=E2=80=AFPM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:
>
> On Wednesday, May 10, 2023 2:36 PM Bharath Rupireddy <bharath.rupireddyfo=
rpostgres@gmail.com> wrote:
> >
> >
> > > My current guess is that mis-using a condition variable is the best b=
et. I
> > > think it should work to use ConditionVariablePrepareToSleep() before =
a
> > > WalSndWait(), and then ConditionVariableCancelSleep(). I.e. to never =
use
> > > ConditionVariableSleep(). The latch set from ConditionVariableBroadca=
st()
> > > 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 fr=
om 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 com=
ments above this.
>

This raises the question of whether we need this condition variable
logic for physical walsenders?

--=20
With Regards,
Amit Kapila.