Horton

RE: Is this a problem in GenericXLogFinish()?

Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>

View thread View on postgr.es
From: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
To: 'Amit Kapila' <amit.kapila16@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>, Alexander Lakhin <exclusion@gmail.com>, Robert Haas <robertmhaas@gmail.com>, Jeff Davis <pgsql@j-davis.com>, Heikki Linnakangas <hlinnaka@iki.fi>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2024-02-05T07:57:09Z
Lists: pgsql-hackers

Attachments

Dear Amit,

> 
> @@ -692,6 +697,9 @@ _hash_freeovflpage(Relation rel, Buffer bucketbuf,
> Buffer ovflbuf,
>   if (!xlrec.is_prev_bucket_same_wrt)
>   wbuf_flags |= REGBUF_NO_CHANGE;
>   XLogRegisterBuffer(1, wbuf, wbuf_flags);
> +
> + /* Track the registration status for later use */
> + wbuf_registered = true;
>   }
> 
>   XLogRegisterBuffer(2, ovflbuf, REGBUF_STANDARD);
> @@ -719,7 +727,12 @@ _hash_freeovflpage(Relation rel, Buffer
> bucketbuf, Buffer ovflbuf,
> 
>   recptr = XLogInsert(RM_HASH_ID, XLOG_HASH_SQUEEZE_PAGE);
> 
> - PageSetLSN(BufferGetPage(wbuf), recptr);
> + /* Set LSN to wbuf page buffer only when it is being registered */
> + if (wbuf_registered)
> + PageSetLSN(BufferGetPage(wbuf), recptr);
> 
> Why set LSN when the page is not modified (say when we use the flag
> REGBUF_NO_CHANGE)?  I think you need to use a flag mod_wbuf and set it
> in appropriate places during register buffer calls.

You are right. Based on the previous discussions, PageSetLSN() must be called
after the MakeBufferDirty(). REGBUF_NO_CHANGE has been introduced for skipping
these requirements. Definitevely, no_change buffers must not be PageSetLSN()'d.
Other pages, e.g., metabuf, has already been followed the rule.

I updated the patch based on the requirement.

Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/

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.