MIME-Version: 1.0
References: <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>
 <CAAKRu_bsQ5KmtAjFsOEHGZCiirp_wgTPyfAQfsyJsGU=g7rkCQ@mail.gmail.com>
 <uuukiok2b5gbbae6blokwim2ohxxvp5235epd5zrbc5d3fojb6@vsqee77athye>
In-Reply-To: <uuukiok2b5gbbae6blokwim2ohxxvp5235epd5zrbc5d3fojb6@vsqee77athye>
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Tue, 25 Nov 2025 16:23:04 -0500
Message-ID: 
 <CAAKRu_apB8+6dDaDBE+tPv8H8rmta3JHomTKFh7VEZK-LYga1g@mail.gmail.com>
Subject: Re: Buffer locking is special (hints, checksums, AIO writes)
To: Andres Freund <andres@anarazel.de>
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>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAAKRu_apB8%2B6dDaDBE%2BtPv8H8rmta3JHomTKFh7VEZK-LYga1g%40mail.gmail.com>
Precedence: bulk

On Tue, Nov 25, 2025 at 3:46=E2=80=AFPM Andres Freund <andres@anarazel.de> =
wrote:
>
>
> > > > I presume you didn't do this because HeapTupleSatisifiesMVCC() for
> > > > SNAPSHOT_MVCC calls the non-batch version (and, of course,
> > > > HeapTupleSatisifiesVisibility() is the much more common case).
> > > >
> > > > if (TransactionIdIsValid(xid))
> > > > {
> > > >         if (BufferIsPermanent(buffer))
> > > >         {
> > > >                 /* NB: xid must be known committed here! */
> > > >                 XLogRecPtr    commitLSN =3D TransactionIdGetCommitL=
SN(xid);
> > > >
> > > >                 if (XLogNeedsFlush(commitLSN) &&
> > > >                         BufferGetLSNAtomic(buffer) < commitLSN)
> > > >                 {
> > > >                         /* not flushed and no LSN interlock, so don=
't
> > > > set hint */
> > > >                         return; false;
> > > >                 }
> > > >         }
> > > > }
> > > >
> > > > Separately, I was thinking, should we assert here about having the
> > > > right lock type?
> > >
> > > Not sure I get what assert of what locktype where?
> >
> > In SetHintBitsExt() that we have share-exclusive or above.
>
> We *don't* necessarily hold that though, it'll just be acquired by
> BufferBeginSetHintBits().  Or do you mean in the SHB_ENABLED case?

Yea, in the enabled case. Also, can't we skip the whole

    if (TransactionIdIsValid(xid))
    {
        if (BufferIsPermanent(buffer))
        {
            /* NB: xid must be known committed here! */
            XLogRecPtr    commitLSN =3D TransactionIdGetCommitLSN(xid);

            if (XLogNeedsFlush(commitLSN) &&
                BufferGetLSNAtomic(buffer) < commitLSN)
            {
                /* not flushed and no LSN interlock, so don't set hint */
                return;
            }
        }
    }

part if state is SHB_DISABLED?

- Melanie