RE: Is this a problem in GenericXLogFinish()?

From: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>
To: 'Alexander Lakhin' <exclusion(at)gmail(dot)com>, 'Amit Kapila' <amit(dot)kapila16(at)gmail(dot)com>
Cc: 'Michael Paquier' <michael(at)paquier(dot)xyz>, '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: 2023-11-30 07:28:05
Message-ID: TY3PR01MB98898183BBB540967625A342F582A@TY3PR01MB9889.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Dear Alexander,

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

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?

[1]:
```
else if (xlrec.is_prim_bucket_same_wrt || xlrec.is_prev_bucket_same_wrt)
{
uint8 wbuf_flags;

/*
* 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.
*/
Assert(xlrec.ntups == 0);

wbuf_flags = REGBUF_STANDARD;
if (!xlrec.is_prev_bucket_same_wrt)
wbuf_flags |= REGBUF_NO_CHANGE;
XLogRegisterBuffer(1, wbuf, wbuf_flags);
}
```

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

Attachment Content-Type Size
initialize_writebuf.patch application/octet-stream 2.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2023-11-30 07:35:26 Re: Testing autovacuum wraparound (including failsafe)
Previous Message Hao Zhang 2023-11-30 07:15:55 [PATCH] plpython function causes server panic