Re: Is this a problem in GenericXLogFinish()?

On 27/09/2023 18:47, Robert Haas wrote:
> On Wed, Sep 27, 2023 at 11:03 AM Jeff Davis <pgsql(at)j-davis(dot)com> wrote:
>> So it looks like it's intentionally registering a clean buffer so that
>> it can take a cleanup lock for reasons other than cleaning (or even
>> modiying) the page. I would think that there's a better way of
>> accomplishing that goal, so perhaps we can fix that?
>
> I had forgotten some of the details of how this works until you
> reminded me, but now that you've jarred my memory, I remember some of
> it.
>
> When Amit Kaplla and I were working on the project to add WAL-logging
> to hash indexes, we ran into some problems with concurrency control
> for individual buckets within the hash index. Before that project,
> this was handled using heavyweight locks, one per bucket. That got
> changed in 6d46f4783efe457f74816a75173eb23ed8930020 for the reasons
> explained in that commit message. Basically, instead of taking
> heavyweight locks, we started taking cleanup locks on the primary
> bucket pages. I always thought that was a little awkward, but I didn't
> quite see how to do better. I don't think that I gave much thought at
> the time to the consequence you've uncovered here, namely that it
> means we're sometimes locking one page (the primary bucket page)
> because we want to do something to another bucket page (some page in
> the linked list of pages that are part of that bucket) and for that to
> be safe, we need to lock out concurrent scans of that bucket.
>
> I guess I don't know of any easy fix here. :-(

That seems OK. Maybe there would be a better way to do that, but there's
nothing wrong with that approach per se.

We could define a new REGBUF_NO_CHANGE flag to signal that the buffer is
registered just for locking purposes, and not actually modified by the
WAL record.

--
Heikki Linnakangas
Neon (https://neon.tech)