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

Andres Freund <andres@anarazel.de>

From: Andres Freund <andres@anarazel.de>
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Melanie Plageman <melanieplageman@gmail.com>, Matthias van de Meent <boekewurm+postgres@gmail.com>, pgsql-hackers@postgresql.org, Thomas Munro <thomas.munro@gmail.com>, Noah Misch <noah@leadboat.com>, Robert Haas <robertmhaas@gmail.com>, Michael Paquier <michael.paquier@gmail.com>
Date: 2025-12-02T13:20:03Z
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 10:01:06 +0200, Heikki Linnakangas wrote:
> On 25/11/2025 22:46, Andres Freund wrote:
> > > > > diff --git a/src/backend/storage/freespace/fsmpage.c
> > > > > b/src/backend/storage/freesp>
> > > > > /*
> > > > > * Update the next-target pointer. Note that we do this even if we're only
> > > > > * holding a shared lock, on the grounds that it's better to use a shared
> > > > > * lock and get a garbled next pointer every now and then, than take the
> > > > > * concurrency hit of an exclusive lock.
> > > > > 
> > > > > We appear to avoid the garbling now?
> > > > 
> > > > I don't think so. Two backends concurrently can do fsm_search_avail() and
> > > > one backend might set a hint to a page that is already used up by the other
> > > > one. At least I think so?
> > > 
> > > Maybe I don't know what it meant by garbled, but I thought it was
> > > talking about two backends each trying to set fp_next_slot. If they
> > > now have to have a share-exclusive lock and they can't both have a
> > > share-exclusive lock at the same time, then it seems like that
> > > wouldn't be a problem. It sounds like you may be talking about a
> > > backend taking up the freespace of a page that is referred to by the
> > > fp_next_slot?
> > 
> > Yes, a version of the latter. The value that fp_next_slot will be set to can
> > be outdated by the time we actually set it, unless we do all of
> > fsm_search_avail() under some form of exclusive lock - clearly not something
> > desirable.
> 
> I'm pretty sure the "garbled" in the comment means the former, not the
> latter. I.e. it means that the pointer itself might become garbage. Would be
> good to update the comment if that's no longer possible.

Hm. I thought we had always assumed that 4byte values can be read/written
tear-free. Hence thinking that garbled couldn't refer to reading entire
garbage due to a concurrent write.


> But speaking of that: why do we not allow two processes to concurrently set
> hint bits on a page anymore?

It'd make the locking a lot more complicated without much of a benefit.

The new share-exclusive lock mode only requires one additional bit of lock
state, for the single allowed holder. If we wanted a new lockmode that
prevented the page from being written out concurrently, but could be held
multiple times, we'd need at least MAX_BACKENDS bits for the lock level
allowing hint bits to be set and another lock level to acquire while writing
out the buffer.

At the same time, there seems to be little benefit in setting hint bits on a
page concurrently. A very common case is that the same hint bit(s) would be
set by multiple backends, we don't gain anything from that. And in the cases
where hint bits were intended to be set for different tuples, the window in
which that is not allowed is very narrow, and the cost of not setting right in
that moment is pretty small and the cost of not setting the hint bit right
then and there isn't high.

Makes sense?

Greetings,

Andres Freund