Re: Is this a problem in GenericXLogFinish()?

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Alexander Lakhin <exclusion(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Jeff Davis <pgsql(at)j-davis(dot)com>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Is this a problem in GenericXLogFinish()?
Date: 2024-02-05 06:40:52
Message-ID: CAA4eK1+Sf6eZ7+WwCv0HaKgpVQmoVtp3dYAoPYJFMxG8f+tbWA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Feb 5, 2024 at 10:00 AM Hayato Kuroda (Fujitsu)
<kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
>
> >
> > Amit, this has been applied as of 861f86beea1c, and I got pinged about
> > the fact this triggers inconsistencies because we always set the LSN
> > of the write buffer (wbuf in _hash_freeovflpage) but
> > XLogRegisterBuffer() would *not* be called when the two following
> > conditions happen:
> > - When xlrec.ntups <= 0.
> > - When !xlrec.is_prim_bucket_same_wrt && !xlrec.is_prev_bucket_same_wrt
> >
> > And it seems to me that there is still a bug here: there should be no
> > point in setting the LSN on the write buffer if we don't register it
> > in WAL at all, no?
>
> Thanks for pointing out, I agreed your saying. PSA the patch for diagnosing the
> issue.
>

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

--
With Regards,
Amit Kapila.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2024-02-05 06:55:36 Re: meson: catalog/syscache_ids.h isn't installed
Previous Message jian he 2024-02-05 06:26:46 Re: Change COPY ... ON_ERROR ignore to ON_ERROR ignore_row