Feedback-ID: id4a34324:Fastmail
Date: Mon, 9 Feb 2026 17:19:06 -0500
From: Andres Freund <andres@anarazel.de>
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Melanie Plageman <melanieplageman@gmail.com>,
	Noah Misch <noah@leadboat.com>, Kirill Reshke <reshkekirill@gmail.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>
Subject: Re: Buffer locking is special (hints, checksums, AIO writes)
Message-ID: <aYpc0K0H2jYvBrLX@alap3.anarazel.de>
References: <ossv2eistssmubfsir6xjll76tynvxv5lup4zkrfzjkud7fycw@rf5vii6l6cha>
 <4csodkvvfbfloxxjlkgsnl2lgfv2mtzdl7phqzd4jxjadxm4o5@usw7feyb5bzf>
 <CALdSSPgyc7VuMLUZ8J7v2G-PebBa_vEi+mp-cLkcOwNycB56Hw@mail.gmail.com>
 <kjdgvws34gpnz4y6xu5aaeul5mspgy3ahvyiw4lndb3ecsacdb@b2ccyowotpqh>
 <jtg5cu4n6h5lib3kzx66ju4yhh6kmviaud7oq6dtut6c4q4rdi@xwsfoagt3c2b>
 <k5j77f3q6ztihnjnx2nqxzyor6fbj2qxcbhzuxhkh2yy63jyfg@p72phigar3n4>
 <5ubipyssiju5twkb7zgqwdr7q2vhpkpmuelxfpanetlk6ofnop@hvxb4g2amb2d>
 <b6ca8cfa-e93a-4755-ad46-55e053d0605f@iki.fi>
 <u644ma4erj75z46wckuq3szrlnci3wzlevq7brauk2p3v6h2l7@jkd7siijr7hx>
 <9f7853f1-415e-475f-b8a7-3f32cafff68b@iki.fi>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <9f7853f1-415e-475f-b8a7-3f32cafff68b@iki.fi>
Archived-At: 
 <https://www.postgresql.org/message-id/aYpc0K0H2jYvBrLX%40alap3.anarazel.de>
Precedence: bulk

Hi,

On 2026-02-09 12:14:47 +0200, Heikki Linnakangas wrote:
> On 09/02/2026 03:52, Andres Freund wrote:
> > On 2026-02-07 14:59:34 +0200, Heikki Linnakangas wrote:
> > > > +/*
> > > > + * Try to set a single hint bit in a buffer.
> > > > + *
> > > > + * This is a bit faster than BufferBeginSetHintBits() /
> > > > + * BufferFinishSetHintBits() when setting a single hint bit, but slower than
> > > > + * the former when setting several hint bits.
> > > > + */
> > > > +bool
> > > > +BufferSetHintBits16(uint16 *ptr, uint16 val, Buffer buffer)
> > > 
> > > This could use some more explanation. The point is that this does "*ptr =
> > > val", if it's allowed to set hint bits. That's not obvious. And "single hint
> > > bit" isn't really accurate, as you could update multiple bits in *ptr with
> > > one call.
> > 
> > Agreed.  I updated it to
> > 
> >   * Try to set hint bits on a single 16bit value in a buffer.
> >   *
> >   * If hint bits are allowed to be set, set *ptr = val, try mark the buffer
> >   * dirty and return true. Otherwise false is returned.
> >   *
> >   * *ptr needs to be a pointer to memory within the buffer.
> >   *
> >   * This is a bit faster than BufferBeginSetHintBits() /
> >   * BufferFinishSetHintBits() when setting hints once in a buffer, but slower
> >   * than the former when setting hint bits multiple times in the same buffer.
> 
> +1. Instead of "try mark the buffer dirty", I'd say just "mark the buffer
> dirty". The only reason it might not to mark the buffer dirty is that it was
> already marked dirty, right? I wouldn't call that a failure.

It's not quite the only reason:

		/*
		 * If we need to protect hint bit updates from torn writes, WAL-log a
		 * full page image of the page. This full page image is only necessary
		 * if the hint bit update is the first change to the page since the
		 * last checkpoint.
		 *
		 * We don't check full_page_writes here because that logic is included
		 * when we call XLogInsert() since the value changes dynamically.
		 */
		if (XLogHintBitIsNeeded() && (lockstate & BM_PERMANENT))
		{
			/*
			 * If we must not write WAL, due to a relfilelocator-specific
			 * condition or being in recovery, don't dirty the page.  We can
			 * set the hint, just not dirty the page as a result so the hint
			 * is lost when we evict the page or shutdown.
			 *
			 * See src/backend/storage/page/README for longer discussion.
			 */
			if (RecoveryInProgress() ||
				RelFileLocatorSkippingWAL(BufTagGetRelFileLocator(&bufHdr->tag)))
				return;

			wal_log = true;
		}

Greetings,

Andres Freund