Horton
Menu

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

Kirill Reshke <reshkekirill@gmail.com>

View thread
From: Kirill Reshke <reshkekirill@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: Melanie Plageman <melanieplageman@gmail.com>, Heikki Linnakangas <hlinnaka@iki.fi>, Noah Misch <noah@leadboat.com>, Matthias van de Meent <boekewurm+postgres@gmail.com>, pgsql-hackers@postgresql.org, Thomas Munro <thomas.munro@gmail.com>, Robert Haas <robertmhaas@gmail.com>, Michael Paquier <michael.paquier@gmail.com>
Date: 2026-02-06T21:18:21Z
Lists: pgsql-hackers 
On Tue, 3 Feb 2026 at 03:33, Andres Freund <andres@anarazel.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

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. bufmgr: Fix ordering of checks in PinBuffer()

  2. Use UnlockReleaseBuffer() in more places

  3. bufmgr: Make UnlockReleaseBuffer() more efficient

  4. bufmgr: Don't copy pages while writing out

  5. Fix use of wrong variable in _hash_kill_items()

  6. Fix bug due to confusion about what IsMVCCSnapshot means

  7. bufmgr: Switch to standard order in MarkBufferDirtyHint()

  8. bufmgr: Remove the, now obsolete, BM_JUST_DIRTIED

  9. Require share-exclusive lock to set hint bits and to flush

  10. heapam: Don't mimic MarkBufferDirtyHint() in inplace updates

  11. bufmgr: Allow conditionally locking of already locked buffer

  12. bufmgr: Avoid spurious compiler warning after fcb9c977aa5

  13. lwlock: Remove ForEachLWLockHeldByMe

  14. lwlock: Remove support for disowned lwlwocks

  15. bufmgr: Implement buffer content locks independently of lwlocks

  16. bufmgr: Change BufferDesc.state to be a 64-bit atomic

  17. lwlock: Improve local variable name

  18. lwlock: Invert meaning of LW_FLAG_RELEASE_OK

  19. bufmgr: Make definitions related to buffer descriptor easier to modify

  20. heapam: Add batch mode mvcc check and use it in page mode

  21. freespace: Don't modify page without any lock

  22. heapam: Move logic to handle HEAP_MOVED into a helper function

  23. bufmgr: Optimize & harmonize LockBufHdr(), LWLockWaitListLock()

  24. bufmgr: Add one-entry cache for private refcount

  25. bufmgr: Separate keys for private refcount infrastructure

  26. Add pg_atomic_unlocked_write_u64

  27. Rename BUFFERPIN wait event class to BUFFER

  28. bufmgr: Turn BUFFER_LOCK_* into an enum

  29. lwlock: Fix, currently harmless, bug in LWLockWakeup()

  30. bufmgr: Use atomic sub for unpinning buffers

  31. bufmgr: Allow some buffer state modifications while holding header lock

  32. bufmgr: Fix valgrind checking for buffers pinned in StrategyGetBuffer()

  33. bufmgr: Don't lock buffer header in StrategyGetBuffer()

  34. bufmgr: fewer calls to BufferDescriptorGetContentLock

  35. bufmgr: Fix signedness of mask variable in BufferSync()

  36. bufmgr: Introduce FlushUnlockedBuffer

  37. Improve ReadRecentBuffer() scalability