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: Robert Haas <robertmhaas@gmail.com>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, Michael Paquier <michael@paquier.xyz>, Jeff Davis <pgsql@j-davis.com>, Alexander Lakhin <exclusion@gmail.com>, Heikki Linnakangas <hlinnaka@iki.fi>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2023-11-14T03:42:29Z
Lists: pgsql-hackers 
On Mon, Nov 13, 2023 at 10:51 PM Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Mon, Nov 13, 2023 at 12:47 AM Hayato Kuroda (Fujitsu)
> <kuroda.hayato@fujitsu.com> wrote:
> > Moved.
>
> I see that this patch was committed, but I'm not very convinced that
> the approach is correct. The comment says this:
>
> +           /*
> +            * A write buffer needs to be registered even if no tuples are
> +            * added to it to ensure that we can acquire a cleanup lock on it
> +            * if it is the same as primary bucket buffer or update the
> +            * nextblkno if it is same as the previous bucket buffer.
> +            */
>
> But surely if the buffer is the same as one of those others, then it's
> registered anyway,

I don't think for others it's registered. For example, consider the
case when prevpage and the writepage are the same (aka
xlrec.is_prev_bucket_same_wrt is true), it won't be registered in
another code path (see comment [1]).

>
> and if it isn't, then it doesn't need to be.
>

In the previous example, we need it to update the nextblockno during replay.

I am not sure if I understand the scenario you are worried about, so
if my response doesn't address your concern, can you please explain a
bit more about the scenario you have in mind?

[1] -
/*
 * If prevpage and the writepage (block in which we are moving tuples
 * from overflow) are same, then no need to separately register
 * prevpage.  During replay, we can directly update the nextblock in
 * writepage.
 */

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