Re: Speedup truncations of temporary relation forks

On 2025/05/31 15:26, Daniil Davydov wrote:
> Hi,
>
> On Sat, May 31, 2025 at 7:49 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>>
>> On Fri, May 30, 2025 at 06:01:16PM +0700, Daniil Davydov wrote:
>>> For now we fully scan local buffers for each fork of the temporary
>>> relation that we want to truncate (in order to drop its buffers). It
>>> happens in the function "DropRelationBuffers".
>>> There used to be the same problem for regular tables (i.e. shared
>>> buffers) and it was fixed in commit [1] and now shared buffers are
>>> scanned only one time for those three relation forks.
>>> I suggest making the same fix for temporary relations. See attached patch.
>>
>> Applying the same kind of optimization for local buffers makes sense
>> here, even if the impact is more limited than regular relations.

+1

>> Please make sure to add this patch to the next commit fest.
>
> OK, already created.

Here are a few review comments on the patch:

+ for (j = 0; j < nforks; j++)
{
- InvalidateLocalBuffer(bufHdr, true);
+ if ((buf_state & BM_TAG_VALID) &&
+ BufTagGetForkNum(&bufHdr->tag) == forkNum[j] &&
+ bufHdr->tag.blockNum >= firstDelBlock[j])
+ {
+ InvalidateLocalBuffer(bufHdr, true);
+ }

It looks like the "buf_state & BM_TAG_VALID" check can be moved
outside the loop, along with the BufTagMatchesRelFileLocator() check.
That would avoid unnecessary looping.

Also, should we add a "break" right after calling InvalidateLocalBuffer()?
Since the buffer has already been invalidated, continuing the loop
may not be necessary.

Regards,

--
Fujii Masao
NTT DATA Japan Corporation