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

On Tue, 3 Feb 2026 at 03:33, Andres Freund <andres(at)anarazel(dot)de> 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