Re: datfrozenxid > relfrozenxid w/ crash before XLOG_HEAP_INPLACE

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.