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
