Horton

Re: datfrozenxid > relfrozenxid w/ crash before XLOG_HEAP_INPLACE

Noah Misch <noah@leadboat.com>

View thread View on postgr.es
From: Noah Misch <noah@leadboat.com>
To: pgsql-hackers@postgresql.org
Date: 2025-04-13T20:37:58Z
Lists: pgsql-hackers

Attachments

On Sun, Apr 06, 2025 at 11:00:54AM -0700, Noah Misch wrote:
> I pushed that as commit 8e7e672 (2024-10-25).  I now think DELAY_CHKPT_START
> is superfluous here, per this proc.h comment:
> 
>  * (In the
>  * extremely common case where the data being modified is in shared buffers
>  * and we acquire an exclusive content lock on the relevant buffers before
>  * writing WAL, this mechanism is not needed, because phase 2 will block
>  * until we release the content lock and then flush the modified data to
>  * disk.)
> 
> heap_inplace_update_and_unlock() meets those conditions.  Its closest
> precedent is XLogSaveBufferForHint(), which does need DELAY_CHKPT_START due to
> having only BUFFER_LOCK_SHARED.

True so far, but ...

> heap_inplace_update_and_unlock() has
> BUFFER_LOCK_EXCLUSIVE, so it doesn't need DELAY_CHKPT_START.

... not so, because we've not yet done MarkBufferDirty().  transam/README
cites SyncOneBuffer(), which says:

	 * Check whether buffer needs writing.
	 *
	 * We can make this check without taking the buffer content lock so long
	 * as we mark pages dirty in access methods *before* logging changes with
	 * XLogInsert(): if someone marks the buffer dirty just after our check we
	 * don't worry because our checkpoint.redo points before log record for
	 * upcoming changes and so we are not required to write such dirty buffer.

The attached patch updates the aforementioned proc.h comment and the
heap_inplace_update_and_unlock() comment that my last message proposed.

Commits

  1. WAL-log inplace update before revealing it to other sessions.

  2. Comment on need to MarkBufferDirty() if omitting DELAY_CHKPT_START.