Horton

Re: Is this a problem in GenericXLogFinish()?

Alexander Law <exclusion@gmail.com>

View thread View on postgr.es
From: Alexander Lakhin <exclusion@gmail.com>
To: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, 'Amit Kapila' <amit.kapila16@gmail.com>
Cc: 'Michael Paquier' <michael@paquier.xyz>, '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: 2023-11-30T11:00:01Z
Lists: pgsql-hackers 
Hello Kuroda-san,

30.11.2023 10:28, Hayato Kuroda (Fujitsu) wrote:
> Again, good catch. Here is my analysis and fix patch.
> I think it is sufficient to add an initialization for writebuf.

I agree with the change. It aligns hash_xlog_squeeze_page() with
hash_xlog_move_page_contents() in regard to initialization of writebuf.
Interestingly enough, the discrepancy exists since these functions
appeared with c11453ce0, and I can't see what justified adding the
initialization inside hash_xlog_move_page_contents() only.
So I think that if this doesn't affect performance, having aligned coding
(writebuf initialized in both functions) is better.

> I want to add a test case for it as well. I've tested on my env and found that proposed
> tuples seems sufficient.
> (We must fill the primary bucket, so initial 500 is needed. Also, we must add
> many dead pages to lead squeeze operation. Final page seems OK to smaller value.)
>
> I compared the execution time before/after patching, and they are not so different
> (1077 ms -> 1125 ms).
>
> How do you think?

I can confirm that the test case proposed demonstrates what is expected
and the duration increase is tolerable, as to me.

Best regards,
Alexander

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.