Message-ID: <68e89de8-5f6c-4eaf-a800-e16a5e487667@iki.fi>
Date: Sat, 7 Feb 2026 12:44:25 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Buffer locking is special (hints, checksums, AIO writes)
To: Andres Freund <andres@anarazel.de>,
 Melanie Plageman <melanieplageman@gmail.com>, Noah Misch <noah@leadboat.com>
Cc: 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>
References: <q3ybyaiownp4ivt6xfy55s5llvtq6vqsd4zbt5kn6hacxdcpp4@2ngd4h4bev5d>
 <dsbacri4xldlobl3jbqqlksefsyrwj7lzljbohxsbqjus3z4o7@otb6rge4w7az>
 <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>
Content-Language: en-US
From: Heikki Linnakangas <hlinnaka@iki.fi>
In-Reply-To: <5ubipyssiju5twkb7zgqwdr7q2vhpkpmuelxfpanetlk6ofnop@hvxb4g2amb2d>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/68e89de8-5f6c-4eaf-a800-e16a5e487667%40iki.fi>
Precedence: bulk

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?

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.

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.

- Heikki