Feedback-ID: i675e48f3:Fastmail
Date: Thu, 20 Nov 2025 14:08:57 -0500
From: Greg Burd <greg@burd.me>
To: Andres Freund <andres@anarazel.de>
Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>,
 "=?utf-8?Q?pgsql-hackers=40postgresql.org?="
 <pgsql-hackers@postgresql.org>, Melanie Plageman
 <melanieplageman@gmail.com>, Thomas Munro <thomas.munro@gmail.com>,
 Heikki Linnakangas <hlinnaka@iki.fi>, Noah Misch <noah@leadboat.com>,
 Robert Haas <robertmhaas@gmail.com>, Michael Paquier
 <michael.paquier@gmail.com>
Message-ID: <1FAF2680-51A6-4ACF-8446-C6FA3234D72A@greg.burd.me>
In-Reply-To: <6rgb2nvhyvnszz4ul3wfzlf5rheb2kkwrglthnna7qhe24onwr@vw27225tkyar>
References: <6rgb2nvhyvnszz4ul3wfzlf5rheb2kkwrglthnna7qhe24onwr@vw27225tkyar>
Subject: Re: Buffer locking is special (hints, checksums, AIO writes)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
Archived-At: 
 <https://www.postgresql.org/message-id/1FAF2680-51A6-4ACF-8446-C6FA3234D72A%40greg.burd.me>
Precedence: bulk


On Nov 19 2025, at 9:47 pm, Andres Freund <andres@anarazel.de> wrote:

> Hi,
> 
> On 2025-10-09 17:16:49 -0400, Andres Freund wrote:
>> On 2025-10-09 16:35:44 -0400, Andres Freund wrote:
>> > I pushed a few commits from this patchset after Matthias' review
>> > (thanks!). Unfortunately in 5e899859287 I missed that the valgrind annotations
>> > would not be done anymore for the buffers returned by
>> > StrategyGetBuffer(). Which turned skink red.
>> > 
>> > The attached 0001 patch centralizes the valgrind initialization in
>> > TrackNewBufferPin(), which 5e899859287 had added. The nice side
>> effect of that
>> > is that there are fewer VALGRIND_MAKE_MEM_DEFINED() calls than
>> before. The
>> > naming isn't the perfect match, but it seems fine to me.
>> 
>> Forgot to say: I'll push this patch soon, to get skink back to green. Unless
>> somebody says something.  We can adjust this later, if the comment and/or
>> placement of VALGRIND_MAKE_MEM_DEFINED() isn't to everyones liking.
> 
> I have pushed that fix as well as the subsequent buffer header locking changes
> a while ago.

Hello Andres,

After talking to you about these ideas at PGConf in NYC I've been
anxiously awaiting this patch set. Thanks for dedicating the energy and
time to get it to this stage.

High level feedback after reading the patches/email/commit messages is
that it looks to get you to where you wanted to be, unblocking AIO
writes. I think they'll end up being faster than what's in place now,
even before you get to the AIO piece.  Certainly removing the copy of
each page to do a checksum will help.  Opening the door to a future
where we can have super-pinned/locked pages is also a net win.

Everything before/after 0008 was rather easy to digest and understand
and I found nothing really to call out at this stage

0008 is understandable too, it's just sizable. While it is large, I find
it well laid out and more readable than before.  I gave the locking code
a good look, it seems correct AFAICT.

Keep going, I'll be happy to dedicate time to testing and digging into
the commits as you get this into a final state.  I look forward to
extending/enhancing this code once integrated.

> I want to again emphasize that the important commits (i.e. 0008, 0012, 0014)
> aren't close to being mergeable. But I think they're in a stage that they
> could benefit from "lenient" high-level review.
> 
> Greetings,
> 
> Andres Freund

cheers.

-greg