Re: Is this a problem in GenericXLogFinish()?

On 22/09/2023 23:52, Jeff Davis wrote:
> 
> src/backend/transam/README says:
> 
>    ...
>    4. Mark the shared buffer(s) as dirty with MarkBufferDirty().  (This
>    must happen before the WAL record is inserted; see notes in
>    SyncOneBuffer().)
>    ...
> 
> But GenericXLogFinish() does this:
> 
>    ...
>    /* Insert xlog record */
>    lsn = XLogInsert(RM_GENERIC_ID, 0);
> 
>    /* Set LSN and mark buffers dirty */
>    for (i = 0; i < MAX_GENERIC_XLOG_PAGES; i++)
>    {
>        PageData   *pageData = &state->pages[i];
> 
>        if (BufferIsInvalid(pageData->buffer))
>            continue;
>        PageSetLSN(BufferGetPage(pageData->buffer), lsn);
>        MarkBufferDirty(pageData->buffer);
>    }
>    END_CRIT_SECTION();
> 
> Am I missing something or is that a problem?

Yes, that's a problem.

I wish we had an assertion for that. XLogInsert() could assert that the 
page is already marked dirty, for example.

-- 
Heikki Linnakangas
Neon (https://neon.tech)