Re: inefficient loop in StandbyReleaseLockList()

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: "Bossart, Nathan" <bossartn(at)amazon(dot)com>
Cc: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, "michael(at)paquier(dot)xyz" <michael(at)paquier(dot)xyz>, "andres(at)anarazel(dot)de" <andres(at)anarazel(dot)de>, "sulamul(at)gmail(dot)com" <sulamul(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: inefficient loop in StandbyReleaseLockList()
Date: 2021-11-01 22:01:18
Message-ID: 1311319.1635804078@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

"Bossart, Nathan" <bossartn(at)amazon(dot)com> writes:
> On 10/31/21, 1:55 PM, "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> 2. I think we almost certainly have a problem in SyncPostCheckpoint.

> This one doesn't look as straightforward. It looks like we might need
> a list_delete_first_n() to delete the first N entries all at once to
> improve this one.

Yeah. We don't absolutely need a new list primitive: we could use
list_copy_tail() and then free the old list. But the extra palloc
traffic involved makes this sound like a bad idea. It does have
the advantage that we could shorten the List's storage even when
it doesn't go to empty, but I'm not sure that's worth anything.
If the List isn't going to empty, that implies that we're getting
a steady stream of unlink requests, meaning we'd probably just
fill it up again.

The minimum-change patch would have us truncating the list before
each AbsorbSyncRequests call, so that the list state meets that
function's expectations. However, as long as UNLINKS_PER_ABSORB
is only 10, I don't think that gets us out of the O(N^2) woods.
So what I did in the attached is add a "canceled" flag to
PendingUnlinkEntry, which lets us deal with canceled or finished
entries without having to delete them from the list right away.
Then we only need to physically clean up the list once per
SyncPostCheckpoint call.

regards, tom lane

Attachment Content-Type Size
improve-SyncPostCheckpoint-performance.patch text/x-diff 5.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2021-11-01 22:15:32 Re: Missing include <openssl/x509.h> in be-secure-openssl.c?
Previous Message Daniel Gustafsson 2021-11-01 21:57:57 Re: Fix C4819 warning in MSVC