From: Tom Lane <tgl@sss.pgh.pa.us>
To: Bossart, Nathan <bossartn@amazon.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Andres Freund <andres@anarazel.de>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: sulamul@gmail.com <sulamul@gmail.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: Re: inefficient loop in StandbyReleaseLockList()
Date: Sun, 31 Oct 2021 15:38:15 -0400
Message-ID: <1013797.1635709095@sss.pgh.pa.us> (raw)
In-Reply-To: <0108A03D-FF2B-4CBC-933B-E047CD975158@amazon.com>
References: <CD2F0E7F-9822-45EC-A411-AE56F14DEA9F@amazon.com>
<CAAJ_b94_awY-VfbLbzxm4zNni3NXxG3wrSkSgnHyCDqHnT+eJg@mail.gmail.com>
<20211028.155751.1898631306946946681.horikyota.ntt@gmail.com>
<20211028220748.6ekrl74minwq75f4@alap3.anarazel.de>
<20211028221424.6wtcse3qkbf2cmqo@alap3.anarazel.de>
<560364.1635459891@sss.pgh.pa.us>
<B4C04240-1AAC-4200-9636-BCC7984688D8@amazon.com>
<565036.1635463448@sss.pgh.pa.us>
<20211028235248.4kbhtcqn2hvjukbd@alap3.anarazel.de>
<YXuaLlYJxkmwnTDl@paquier.xyz>
<0108A03D-FF2B-4CBC-933B-E047CD975158@amazon.com>
"Bossart, Nathan" <bossartn@amazon.com> writes:
> On 10/28/21, 11:53 PM, "Michael Paquier" <michael@paquier.xyz> wrote:
>> Actually, as the list of recovery locks is saved in TopMemoryContext,
>> wouldn't it be better to keep a per-cell deletion of the list, which
>> would mean that we'd better do the operation in the reverse order to
>> make things faster with the new list implementation? But that's what
>> Andres points at with CFIs in the middle of one list of the hash table
>> processed?
> Hm. IIUC anything bad enough to cause the startup process to break
> out of the StandbyReleaseLockList() loop will also cause the entire
> process to be restarted. I'm not seeing any risk of reusing a half-
> released lock list. I might be misunderstanding the concern, though.
Yeah, there's no expectation that this data structure needs to be kept
consistent after an error; and I'm not real sure that the existing
code could claim to satisfy such a requirement if we did need it.
(There's at least a short window where the caller's hash table entry
will point at an already-freed List.)
Pushed the patch as given. I've not yet reviewed other list_delete_first
callers, but I'll take a look. (I seem to remember that I did survey
them while writing 1cff1b95a, but I evidently missed that this code
could be dealing with a list long enough to be problematic.)
regards, tom lane
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, bossartn@amazon.com, michael@paquier.xyz, andres@anarazel.de, horikyota.ntt@gmail.com, sulamul@gmail.com
Subject: Re: inefficient loop in StandbyReleaseLockList()
In-Reply-To: <1013797.1635709095@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox