Horton

Re: Is this a problem in GenericXLogFinish()?

Heikki Linnakangas <hlinnaka@iki.fi>

View thread View on postgr.es
From: Heikki Linnakangas <hlinnaka@iki.fi>
To: Jeff Davis <pgsql@j-davis.com>, pgsql-hackers@postgresql.org
Date: 2023-09-25T10:04:41Z
Lists: pgsql-hackers 
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)

Commits

  1. Fix inconsistency with replay of hash squeeze record for clean buffers

  2. Set LSN for wbuf in _hash_freeovflpage() iff wbuf is modified.

  3. Fix an uninitialized access in hash_xlog_squeeze_page().

  4. Use REGBUF_NO_CHANGE at one more place in the hash index.

  5. Assert that buffers are marked dirty before XLogRegisterBuffer().

  6. Fix bug in GenericXLogFinish().

  7. Improve hash index bucket split behavior.