Message-ID: <9aa385d8-fe75-c35b-cf91-31bb3647d977@gmail.com>
Date: Thu, 30 Nov 2023 14:00:01 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: Is this a problem in GenericXLogFinish()?
Content-Language: en-US
To: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>,
 'Amit Kapila' <amit.kapila16@gmail.com>
Cc: '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>
References: 
 <CA+TgmobjyWDQfiMLj1_Lf0GheiY0SaOEPbFEepfa3G-XOLmHOA@mail.gmail.com>
 <ZTzpsFEV3DhlmxX5@paquier.xyz>
 <CAA4eK1+use8DaXfgEwRiM7CnEk83d_FZCMf4qK7DZ8Hbt2vhzg@mail.gmail.com>
 <ZUH9F6i-TtlJ6gw7@paquier.xyz>
 <CAA4eK1KH1LKhgA_i3akFtTV0NqSAG-N=ttm3ravBcjPWGkbZ+w@mail.gmail.com>
 <TYAPR01MB58667885EE61EFB2844820DDF5AAA@TYAPR01MB5866.jpnprd01.prod.outlook.com>
 <TYAPR01MB58668D6E367C6C57B515E965F5AAA@TYAPR01MB5866.jpnprd01.prod.outlook.com>
 <TYAPR01MB58663DB52ED18495A5AD1338F5AEA@TYAPR01MB5866.jpnprd01.prod.outlook.com>
 <CAA4eK1J1vxqwMfETXN43_0xiHd51=ryM=aVXgq+PWa7RoUnwdw@mail.gmail.com>
 <TYAPR01MB5866CE98DC1339C42E45C2ACF5B3A@TYAPR01MB5866.jpnprd01.prod.outlook.com>
 <CA+TgmobSARYNQHvOHrehH-+YHkLGWdqJkuxrnDHCcviOYZ+Qgw@mail.gmail.com>
 <62ed1a9f-746a-8e86-904b-51b9b806a1d9@gmail.com>
 <TY3PR01MB9889F5D88EB8BD4C61675E36F582A@TY3PR01MB9889.jpnprd01.prod.outlook.com>
 <TY3PR01MB98898183BBB540967625A342F582A@TY3PR01MB9889.jpnprd01.prod.outlook.com>
From: Alexander Lakhin <exclusion@gmail.com>
In-Reply-To: 
 <TY3PR01MB98898183BBB540967625A342F582A@TY3PR01MB9889.jpnprd01.prod.outlook.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/9aa385d8-fe75-c35b-cf91-31bb3647d977%40gmail.com>
Precedence: bulk

Hello Kuroda-san,

30.11.2023 10:28, Hayato Kuroda (Fujitsu) wrote:
> Again, good catch. Here is my analysis and fix patch.
> I think it is sufficient to add an initialization for writebuf.

I agree with the change. It aligns hash_xlog_squeeze_page() with
hash_xlog_move_page_contents() in regard to initialization of writebuf.
Interestingly enough, the discrepancy exists since these functions
appeared with c11453ce0, and I can't see what justified adding the
initialization inside hash_xlog_move_page_contents() only.
So I think that if this doesn't affect performance, having aligned coding
(writebuf initialized in both functions) is better.

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

I can confirm that the test case proposed demonstrates what is expected
and the duration increase is tolerable, as to me.

Best regards,
Alexander