Feedback-ID: id4a34324:Fastmail
Date: Tue, 2 Dec 2025 19:47:35 -0500
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>
Subject: Re: Buffer locking is special (hints, checksums, AIO writes)
Message-ID: <lneuyxqxamqoayd2ntau3lqjblzdckw6tjgeu4574ezwh4tzlg@noioxkquezdw>
References: <yivb2evcrj7fna5ymuunw3g5u5xxttwjbjxaa4ofkfkviystjv@4dfylftqxyxh>
 <6kmid26do57ykqfpvq6iieniy4djsymhrypkjccazq5g4bbe6a@2y6owwv7qpex>
 <CAEze2WgGe8vjj3jiWqUugWuwLJ9cLryaGrnASjm-yJ=tEALX2A@mail.gmail.com>
 <pmto7djq64mei53p7r5smfync2waittilhbuzc7j7lpflf2b3y@laz7r76y5pux>
 <CAEze2WjeK9CY003S4dmCugv_H4tz9AaXgnqW+wTc=BaPDg+2xg@mail.gmail.com>
 <3je3ahgf7rrmmurxo6hnlhg5d3ffwfrtjwjxd6jm5srlv5iebp@vxqk5qtgmowr>
 <3w7v3w6a57jnssokap4k7thoekig72flnyhd4wp3yftzdd7lm7@f6lpcfen6hr7>
 <6rgb2nvhyvnszz4ul3wfzlf5rheb2kkwrglthnna7qhe24onwr@vw27225tkyar>
 <CAAKRu_ZcZCXNUWfhEg4qOYuRO8T1x3_gvaUFFrwgCfY0RnVhkg@mail.gmail.com>
 <3nce7i72ayzkunai6mkz24ckbxk74jodz4ua2chcdrwppxlxcd@w6x5kfkjrkru>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="kad4mm4awdo7v2ki"
Content-Disposition: inline
In-Reply-To: <3nce7i72ayzkunai6mkz24ckbxk74jodz4ua2chcdrwppxlxcd@w6x5kfkjrkru>
Archived-At: 
 <https://www.postgresql.org/message-id/lneuyxqxamqoayd2ntau3lqjblzdckw6tjgeu4574ezwh4tzlg%40noioxkquezdw>
Precedence: bulk


--kad4mm4awdo7v2ki
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Hi,

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?


Greetings,

Andres Freund

--kad4mm4awdo7v2ki
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v7-0001-bufmgr-Turn-BUFFER_LOCK_-into-an-enum.patch"