Horton

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

Andres Freund <andres@anarazel.de>

View thread
From: Andres Freund <andres@anarazel.de>
To: Melanie Plageman <melanieplageman@gmail.com>
Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>, pgsql-hackers@postgresql.org, 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>
Date: 2025-12-03T16:03:40Z
Lists: pgsql-hackers

Commits

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

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

  2. lwlock: Remove ForEachLWLockHeldByMe

  3. bufmgr: Implement buffer content locks independently of lwlocks

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

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

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

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

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

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

  10. bufmgr: Separate keys for private refcount infrastructure

  11. Add pg_atomic_unlocked_write_u64

  12. Rename BUFFERPIN wait event class to BUFFER

  13. bufmgr: Turn BUFFER_LOCK_* into an enum

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

  15. bufmgr: Use atomic sub for unpinning buffers

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

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

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

  19. bufmgr: fewer calls to BufferDescriptorGetContentLock

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

  21. bufmgr: Introduce FlushUnlockedBuffer

  22. Improve ReadRecentBuffer() scalability

Hi,

On 2025-12-02 19:47:35 -0500, Andres Freund wrote:
> On 2025-11-25 11:54:00 -0500, Andres Freund wrote:
> > Thanks a lot for that detailed review!  A few questions and comments, before I
> > try to address the comments in the next version.
> 
> Here's that new new version, with the following changes
> 
> - Some more micro-optimizations, most importantly adding a commit that doesn't
>   initialize the delay in LockBufHdr() unless needed. With those I don't see a
>   consistent slowdown anymore (slight speedup on one workstation, slight
>   slowdown on another, in an absurdly adverse workload)
> 
> - Tried to address Melanie's feedback, with some exceptions (some noted below,
>   but I also need to make another pass through the reviews)
> 
> - re-implemented AssertNotCatalogBufferLock() in the new world
> 
> - Substantially expanded comments around setting hint bits (in buffer/README,
>   heapam_visibility.c and bufmgr.c)
> 
> - split out the change to fsm_vacuum_page() to start to lock the page into is
>   own commit
> 
> - reordered patch series so that smaller changes are before the 64bit-state
>   and "Implement buffer content locks independently of" commits, so they can
>   be committed while we finish cleaning the later changes
> 
> - I didn't invest much in cleaning up the later patches ("Don't copy pages
>   while writing out" and "Make UnlockReleaseBuffer() more efficient") yet,
>   wanted to focus on the earlier patches first
> 
> 
> Todo:
> 
> - still need to rename ResOwnerReleaseBufferPin(). Wondering about what to
>   rename ResourceOwnerDesc.name to. "buffer ownership" maybe? Not great...
> 
> - gistkillitems() complaint by Melanie
> 
> - amortize vs batch vs SetHintBits comment + SHB_* names
> 
> - for the next version I'll remove the BATCHMVCC_FEWER_ARGS conditionals from
>   0010. I don't love needing BatchMVCCState but I don't really see an
>   alternative, the performance difference is pretty persistent.
> 
> 
> Questions:
> - ForEachLWLockHeldByMe() and LWLockDisown() aren't used anymore, should we
>   remove them?

I'm planning to work on committing 0001, 0002, 0003, 0008 soon-ish, unless
somebody sees a reason to hold off on that.  After that I think 0005, 0006
would be next.  I think 0004 is a clear improvement, but nobody has looked at
it yet...

For 0007, I wished ConditionalLockBuffer() accepted the lock level, there's no
point in waiting for the lock in the use case. I'm on the fence about whether
it's worth changing the ~12 users of ConditionalLockBuffer()...

Greetings,

Andres Freund