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: Michael Paquier <michael@paquier.xyz>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, 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-04-05T05:47:03Z
Lists: pgsql-hackers 
On Fri, Apr 5, 2024 at 9:56 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Wed, Feb 07, 2024 at 06:42:21PM +0900, Michael Paquier wrote:
> > On Wed, Feb 07, 2024 at 02:08:42PM +0530, Amit Kapila wrote:
> > > Thanks for the report and looking into it. Pushed!
> >
> > Thanks Amit for the commit and Kuroda-san for the patch!
>
> I have been pinged about this thread and I should have looked a bit
> closer, but wait a minute here.
>
> There is still some divergence between the code path of
> _hash_freeovflpage() and the replay in hash_xlog_squeeze_page() when
> squeezing a page: we do not set the LSN of the write buffer if
> (xlrec.ntups <= 0 && xlrec.is_prim_bucket_same_wrt &&
> !xlrec.is_prev_bucket_same_wrt) when generating the squeeze record,
> but at replay we call PageSetLSN() on the write buffer and mark it
> dirty in this case.  Isn't that incorrect?
>

Agreed. We will try to reproduce this.

>  It seems to me that we
> should be able to make the conditional depending on the state of the
> xl_hash_squeeze_page record, no?
>

I think we can have a flag like mod_buf and set it in both the
conditions if (xldata->ntups > 0) and if
(xldata->is_prev_bucket_same_wrt). If the flag is set then we can set
the LSN and mark buffer dirty.

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