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

From: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
To: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Cc: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, "Drouvot, Bertrand" <bertranddrouvot(dot)pg(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers(at)postgresql(dot)org, Jeff Davis <pgsql(at)j-davis(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>
Subject: Re: walsender performance regression due to logical decoding on standby changes
Date: 2023-05-15 14:39:00
Message-ID: CALj2ACWeo64RSqf8tDbnSSUm_vbpK5GYdDiiFQk8E3Fg38mBdw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, May 15, 2023 at 6:14 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> On Mon, May 15, 2023 at 1:49 PM Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
> >
> > On Fri, May 12, 2023 at 11:58 PM Bharath Rupireddy
> > <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> > > Andres, rightly put it - 'mis-using' CV infrastructure. It is simple,
> > > works, and makes the WalSndWakeup() easy solving the performance
> > > regression.
> >
> > Yeah, this seems OK, and better than the complicated alternatives. If
> > one day we want to implement CVs some other way so that this
> > I-know-that-CVs-are-really-made-out-of-latches abstraction leak
> > becomes a problem, and we still need this, well then we can make a
> > separate latch-wait-list thing.
>
> +1

Thanks for acknowledging the approach.

On Mon, May 15, 2023 at 2:11 PM Drouvot, Bertrand
<bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
>
> Thanks for V2! It does look good to me and I like the fact that
> WalSndWakeup() does not need to loop on all the Walsenders slot
> anymore (for both the physical and logical cases).

Indeed, it doesn't have to.

On Mon, May 15, 2023 at 7:50 AM Zhijie Hou (Fujitsu)
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> Thanks for updating the patch. I did some simple primary->standby replication test for the
> patch and can see the degradation doesn't happen in the replication after applying it[1].
>
> One nitpick in the comment:
>
> + * walsenders. It makes WalSndWakeup() callers life easy.
>
> callers life easy => callers' life easy.

Changed.

> [1]
> max_wal_senders = 100
> before regression(ms) after regression(ms) v2 patch(ms)
> 13394.4013 14141.2615 13455.2543
> Compared with before regression 5.58% 0.45%
>
> max_wal_senders = 200
> before regression(ms) after regression(ms) v2 patch(ms)
> 13280.8507 14597.1173 13632.0606
> Compared with before regression 9.91% 1.64%
>
> max_wal_senders = 300
> before regression(ms) after regression(ms) v2 patch(ms)
> 13535.0232 16735.7379 13705.7135
> Compared with before regression 23.65% 1.26%

Yes, the numbers with v2 patch look close to where we were before.
Thanks for confirming. Just wondering, where does this extra
0.45%/1.64%/1.26% coming from?

Please find the attached v3 with the review comment addressed.

Do we need to add an open item for this issue in
https://wiki.postgresql.org/wiki/PostgreSQL_16_Open_Items? If yes, can
anyone in this loop add one?

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Attachment Content-Type Size
v3-0001-Optimize-walsender-wake-up-logic-with-Conditional.patch application/octet-stream 6.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Marina Polyakova 2023-05-15 15:27:29 Conflict between regression tests namespace & transactions due to recent changes
Previous Message Tom Lane 2023-05-15 14:28:52 Re: psql: Could we get "-- " prefixing on the **** QUERY **** outputs? (ECHO_HIDDEN)