From: Andres Freund Date: Tue, 10 Mar 2026 14:06:09 +0000 (-0400) Subject: heapam: Don't mimic MarkBufferDirtyHint() in inplace updates X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f4a4ce52c0d1565c13c436ea17960d22787ea752;p=thirdparty%2Fpostgresql.git heapam: Don't mimic MarkBufferDirtyHint() in inplace updates Previously heap_inplace_update_and_unlock() used an operation order similar to MarkBufferDirty(), to reduce the number of different approaches used for updating buffers. However, in an upcoming patch, MarkBufferDirtyHint() will switch to using the update protocol used by most other places (enabled by hint bits only being set while holding a share-exclusive lock). Luckily it's pretty easy to adjust heap_inplace_update_and_unlock(). As a comment already foresaw, we can use the normal order, with the slight change of updating the buffer contents after WAL logging. Reviewed-by: Heikki Linnakangas Reviewed-by: Noah Misch Discussion: https://postgr.es/m/5ubipyssiju5twkb7zgqwdr7q2vhpkpmuelxfpanetlk6ofnop@hvxb4g2amb2d --- diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index a231563f0df..1ecc8330851 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -6613,11 +6613,11 @@ heap_inplace_update_and_unlock(Relation relation, /*---------- * NO EREPORT(ERROR) from here till changes are complete * - * Our buffer lock won't stop a reader having already pinned and checked - * visibility for this tuple. Hence, we write WAL first, then mutate the - * buffer. Like in MarkBufferDirtyHint() or RecordTransactionCommit(), - * checkpoint delay makes that acceptable. With the usual order of - * changes, a crash after memcpy() and before XLogInsert() could allow + * Our exclusive buffer lock won't stop a reader having already pinned and + * checked visibility for this tuple. With the usual order of changes + * (i.e. updating the buffer contents before WAL logging), a reader could + * observe our not-yet-persistent update to relfrozenxid and update + * datfrozenxid based on that. A crash in that moment could allow * datfrozenxid to overtake relfrozenxid: * * ["D" is a VACUUM (ONLY_DATABASE_STATS)] @@ -6629,21 +6629,15 @@ heap_inplace_update_and_unlock(Relation relation, * [crash] * [recovery restores datfrozenxid w/o relfrozenxid] * - * Mimic MarkBufferDirtyHint() subroutine XLogSaveBufferForHint(). - * Specifically, use DELAY_CHKPT_START, and copy the buffer to the stack. - * The stack copy facilitates a FPI of the post-mutation block before we - * accept other sessions seeing it. DELAY_CHKPT_START allows us to - * XLogInsert() before MarkBufferDirty(). Since XLogSaveBufferForHint() - * can operate under BUFFER_LOCK_SHARED, it can't avoid DELAY_CHKPT_START. - * This function, however, likely could avoid it with the following order - * of operations: MarkBufferDirty(), XLogInsert(), memcpy(). Opt to use - * DELAY_CHKPT_START here, too, as a way to have fewer distinct code - * patterns to analyze. Inplace update isn't so frequent that it should - * pursue the small optimization of skipping DELAY_CHKPT_START. - */ - Assert((MyProc->delayChkptFlags & DELAY_CHKPT_START) == 0); + * We avoid that by using a temporary copy of the buffer to hide our + * change from other backends until the change has 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. + */ START_CRIT_SECTION(); - MyProc->delayChkptFlags |= DELAY_CHKPT_START; + + MarkBufferDirty(buffer); /* XLOG stuff */ if (RelationNeedsWAL(relation)) @@ -6692,8 +6686,6 @@ heap_inplace_update_and_unlock(Relation relation, memcpy(dst, src, newlen); - MarkBufferDirty(buffer); - LockBuffer(buffer, BUFFER_LOCK_UNLOCK); /* @@ -6702,7 +6694,6 @@ heap_inplace_update_and_unlock(Relation relation, */ AtInplace_Inval(); - MyProc->delayChkptFlags &= ~DELAY_CHKPT_START; END_CRIT_SECTION(); UnlockTuple(relation, &tuple->t_self, InplaceUpdateTupleLock);