Date: Sun, 15 Feb 2026 11:52:39 -0800
From: Noah Misch <noah@leadboat.com>
To: Andres Freund <andres@anarazel.de>,
	Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Melanie Plageman <melanieplageman@gmail.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: <20260215195239.ce.noahmisch@microsoft.com>
References: <1108f18d-cf7c-4f17-b29c-a119fe42f7e5@iki.fi>
 <5dwlfu2jyzkyf3nrlzxxblxctb6xio5es73ptgsahjnmfu5miu@772rc764hfhi>
 <ossv2eistssmubfsir6xjll76tynvxv5lup4zkrfzjkud7fycw@rf5vii6l6cha>
 <4csodkvvfbfloxxjlkgsnl2lgfv2mtzdl7phqzd4jxjadxm4o5@usw7feyb5bzf>
 <CALdSSPgyc7VuMLUZ8J7v2G-PebBa_vEi+mp-cLkcOwNycB56Hw@mail.gmail.com>
 <kjdgvws34gpnz4y6xu5aaeul5mspgy3ahvyiw4lndb3ecsacdb@b2ccyowotpqh>
 <jtg5cu4n6h5lib3kzx66ju4yhh6kmviaud7oq6dtut6c4q4rdi@xwsfoagt3c2b>
 <k5j77f3q6ztihnjnx2nqxzyor6fbj2qxcbhzuxhkh2yy63jyfg@p72phigar3n4>
 <5ubipyssiju5twkb7zgqwdr7q2vhpkpmuelxfpanetlk6ofnop@hvxb4g2amb2d>
 <68e89de8-5f6c-4eaf-a800-e16a5e487667@iki.fi>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <68e89de8-5f6c-4eaf-a800-e16a5e487667@iki.fi>
User-Agent: Mutt/2.2.12 (2023-09-09)
Archived-At: <https://www.postgresql.org/message-id/20260215195239.ce.noahmisch%40microsoft.com>
Precedence: bulk

On Sat, Feb 07, 2026 at 12:44:25PM +0200, Heikki Linnakangas wrote:
> On 03/02/2026 00:33, Andres Freund wrote:
> > - The way MarkBufferDirtyHint() operates was copied into
> >    heap_inplace_update_and_unlock(). Now that MarkBufferDirtyHint() won't work
> >    that way anymore, it seems better to go with the alternative approach the
> >    comments already outlined, namely to only delay updating of the buffer
> >    contents.
> > 
> >    I've done this in a prequisite commit, as it doesn't actually depend on any
> >    of the other changes.  Noah, any chance you could take a look at this?

v12-0001-heapam-Don-t-mimic-MarkBufferDirtyHint-in-inplac.patch looks good.

> Patch 0001 Looks correct to me. However:
> 
> > 	 * ["D" is a VACUUM (ONLY_DATABASE_STATS)]
> > 	 * ["R" is a VACUUM tbl]
> > 	 * D: vac_update_datfrozenxid() -> systable_beginscan(pg_class)
> > 	 * D: systable_getnext() returns pg_class tuple of tbl
> > 	 * R: memcpy() into pg_class tuple of tbl
> > 	 * D: raise pg_database.datfrozenxid, XLogInsert(), finish
> > 	 * [crash]
> > 	 * [recovery restores datfrozenxid w/o relfrozenxid]
> > 	 *
> > 	 * As we hold an exclusive lock - preventing the buffer from being written
> > 	 * out once dirty - we can work around this as follows: MarkBufferDirty(),
> > 	 * XLogInsert(), memcpy().
> 
> That last reference to 'memcpy' is a little orphaned now. The comment used
> to talk about the stack copy of the page, but now there's no mention of that
> except for this reference to memcpy(). To make things worse, the steps have
> "memcpy() into pg_class tuple of tbl", so one could think that the "memcpy"
> refers to that.

"memcpy" does refer to "memcpy() into pg_class tuple of tbl", so I don't see
that as orphaned.  Nonetheless:

> How about this:
> 
> 	 * We avoid that by using a temporary copy of the buffer to hide our
> 	 * change from other backends until it's been WAL-logged. We apply our
> 	 * change to the temporary copy and WAL-log it before modifying the real
> 	 * page. That way any action a reader of the in-place-updated value takes
> 	 * will be WAL logged after this change.

Either v12 or v12 w/ this edit is fine with me.  I find this proposed text
redundant with nearby comment "register block matching what buffer will look
like after changes", so I mildly prefer v12.