Message-ID: <6d23aff2-7151-65b1-528b-2189ffa6e2df@iki.fi>
Date: Wed, 27 Sep 2023 18:57:42 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Subject: Re: Is this a problem in GenericXLogFinish()?
Content-Language: en-US
To: Robert Haas <robertmhaas@gmail.com>, Jeff Davis <pgsql@j-davis.com>
Cc: pgsql-hackers@postgresql.org
References: <d2c31606e6bb9b83a02ed4835d65191b38d4ba12.camel@j-davis.com>
 <25104133-7df8-cae3-b9a2-1c0aaa1c094a@iki.fi>
 <399492009193fa2c37a61b630e4449546a550039.camel@j-davis.com>
 <c84114f8-c7f1-5b57-f85a-3adc31e1a904@iki.fi>
 <146426fc662f88604c700a72a3833b1919922f9e.camel@j-davis.com>
 <CA+TgmobvdZMWj5=fextXP_Q81eS9g0sbVbVE4qiKAtJs8dMjyA@mail.gmail.com>
 <08b4f1f7dcf321d38373f06acc65a8d62cc2f017.camel@j-davis.com>
 <CA+TgmoZ24f5eNNJFV6chv+xjCOOy9-uZqwPwrWVvQMZfHUsMxQ@mail.gmail.com>
From: Heikki Linnakangas <hlinnaka@iki.fi>
In-Reply-To: 
 <CA+TgmoZ24f5eNNJFV6chv+xjCOOy9-uZqwPwrWVvQMZfHUsMxQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/6d23aff2-7151-65b1-528b-2189ffa6e2df%40iki.fi>
Precedence: bulk

On 27/09/2023 18:47, Robert Haas wrote:
> On Wed, Sep 27, 2023 at 11:03 AM Jeff Davis <pgsql@j-davis.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)