Subject: Re: Buffer locking is special (hints, checksums, AIO writes)

From: Kirill Reshke <[email-masked]>
Date: Sat, 7 Feb 2026 02:18:21 +0500

On Tue, 3 Feb 2026 at 03:33, Andres Freund <[email-masked]> wrote:
>
> Hi,
>
> On 2026-01-14 16:20:58 -0500, Andres Freund wrote:
> > I'm now working on cleaning up the last two commits. The most crucial bit is
> > to simplify what happens in MarkSharedBufferDirtyHint(), we afaict can delete
> > the use of DELAY_CHKPT_START etc and just go to marking the buffer dirty first
> > and then do the WAL logging, just like normal WAL logging. The previous order
> > was only required because we were dirtying the page while holding only a
> > shared lock, which did not conflict with the lock held by SyncBuffers() etc.
>
> I've been working on that.
>
> - A lot of what was special about MarkBufferDirtyHint() isn't needed anymore:
>
>   - The "abnormal" order of WAL logging before marking the buffer dirty was
>     only needed because we marked buffers dirty. Which in turn was only needed
>     because setting hint bits didn't conflict with flushing the page. With
>     share-exclusive they do conflict, and we can switch to the normal order of
>     operations, where marking a buffer dirty makes checkpoint wait when the
>     buffer is encountered (due to wanting to flush the buffer but not getting
>     the lock)
>
>
>   - Now that we use the normal order of WAL logging, we don't need to delay
>     checkpoint starts anymore.
>
>     I think the explanation for why that is ok is correct [1], but it needs to
>     be looked at by somebody with experience around this. Maybe Heikki?
>
>
>   - Thanks to holding share-exclusive lock, nothing can concurrently dirty or
>     undirty the buffer. Therefore the comments about spurious failures to mark
>     the buffer dirty can be removed.
>
>
> - I realized that, now that buffers cannot be dirtied while IO is ongoing, we
>   don't need BM_JUST_DIRTIED anymore.
>
>
> - The way MarkBufferDirtyHint() operates was copied into
>   heap_inplace_update_and_unlock(). Now that MarkBufferDirtyHint() won't work
>   that way anymore, it seems better to go with the alternative approach the
>   comments already outlined, namely to only delay updating of the buffer
>   contents.
>
>   I've done this in a prequisite commit, as it doesn't actually depend on any
>   of the other changes.  Noah, any chance you could take a look at this?
>
>
> - Lots of minor polish
>
>
> Greetings,
>
> Andres Freund
>
> [1]
>         /*
>          * Update RedoRecPtr so that we can make the right decision. It's possible
>          * that a new checkpoint will start just after GetRedoRecPtr(), but that
>          * is ok, as the buffer is already dirty, ensuring that any BufferSync()
>          * started after the buffer was marked dirty cannot complete without
>          * flushing this buffer.  If a checkpoint started between marking the
>          * buffer dirty and this check, we will emit an unnecessary WAL record (as
>          * the buffer will be written out as part of the checkpoint), but the
>          * window for that is small.
>          */

Hi!
I was reviewing new patches in this thread, and noticed your changes
in v12-0002, in gistkillitems. This makes me think you can be
interested in reviewing [0].

Anyway, v12-0003, on other patches I don't have an opinion (yet).

[0] https://commitfest.postgresql.org/patch/6399/
-- 
Best regards,
Kirill Reshke