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)