Horton

Re: Is this a problem in GenericXLogFinish()?

Amit Kapila <amit.kapila16@gmail.com>

View thread View on postgr.es
From: Amit Kapila <amit.kapila16@gmail.com>
To: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
Cc: Alexander Lakhin <exclusion@gmail.com>, 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-30T10:11:19Z
Lists: pgsql-hackers 
On Thu, Nov 30, 2023 at 12:58 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:
>
> >
> > Good catch, thank you for reporting! I will investigate more about it and post my
> > analysis.
> >
>
> Again, good catch. Here is my analysis and fix patch.
> I think it is sufficient to add an initialization for writebuf.
>
> In the reported case, neither is_prim_bucket_same_wrt nor is_prev_bucket_same_wrt
> is set in the WAL record, and ntups is also zero. This means that the wbuf is not
> written in the WAL record on primary side (see [1]).
> Also, there are no reasons to read (and lock) other buffer page because we do
> not modify it. Based on them, I think that just initializing as InvalidBuffer is sufficient.
>

Agreed.

>
> 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).
>

In my environment, it increased from 375ms to 385ms (median of five
runs). I think it is acceptable especially if it increases code
coverage. Can you once check that?

-- 
With Regards,
Amit Kapila.

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.