From: Yura Sokolov <y.sokolov@postgrespro.ru>
To: David Rowley <dgrowleyml@gmail.com>
To: Stepan Neretin <slpmcf@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Optimize truncation logic to reduce AccessExclusive lock impact
Date: Wed, 19 Mar 2025 19:05:13 +0300
Message-ID: <0be607a5-20f6-4894-bb22-412e3c784bc2@postgrespro.ru> (raw)
In-Reply-To: <CAApHDvrfngqCpQ09LKdr-BnJEVHW0=wAhxfRBnw=HJ2645cJPg@mail.gmail.com>
References: <CA+Yyo5Trs9x03-sc4PpmEXPY9K_B_jbEbNd01RbF+vOxs3zVUA@mail.gmail.com>
<CAApHDvrfngqCpQ09LKdr-BnJEVHW0=wAhxfRBnw=HJ2645cJPg@mail.gmail.com>
19.03.2025 05:09, David Rowley wrote:
> On Tue, 18 Mar 2025 at 19:04, Stepan Neretin <slpmcf@gmail.com> wrote:
>> We propose modifying the truncation condition in should_attempt_truncation to avoid unnecessary full buffer scans. The new formula ensures we only attempt truncation when we can free at least 3% of the relation size + 2 pages. This change prevents excessive truncation attempts on small tables while reducing the impact on larger relations. Previously, small tables could theoretically be truncated to a single page, but with this change, they may remain around 3 pages instead. We don't see this as a significant issue.
>
> I'm just following this code through. Looking at
> DropRelationBuffers(), d6ad34f34 added an optimisation so that we can
> skip scanning all of shared buffers while in recovery (where
> smgrnblocks_cached() can return a non-InvalidBlockNumber value), which
> the following code might decide to lookup the buffers one-by-one
> rather than scanning the entire buffer pool:
>
> if (BlockNumberIsValid(nBlocksToInvalidate) &&
> nBlocksToInvalidate < BUF_DROP_FULL_SCAN_THRESHOLD)
> {
> for (j = 0; j < nforks; j++)
> FindAndDropRelationBuffers(rlocator.locator, forkNum[j],
> nForkBlock[j], firstDelBlock[j]);
> return;
> }
>
> While I don't know this code too well, from a quick look, it seem to
> me that since DropRelationBuffers() is being called from
> smgrtruncate() in this case, smgrtruncate() has access to old_nblocks
> so it knows the old size of the relation. Couldn't we do something
> like have another version of DropRelationBuffers() which accepts a
> parameter for the old number of blocks and uses that instead of
> calling smgrnblocks_cached()? That would allow the
> FindAndDropRelationBuffers() optimisation to be used in cases where
> the buffers to truncate is small or shared buffers is large.
Yes, it will work for smaller relations.
But it doesn't fix issue with large relations:
- imho, there is no big need to truncate 2TB table to release every 8MB of
free space, but it happens now.
So, probably we should do both:
- improve truncate of smaller relations
- reduce frequency of truncation larger relations.
I believe, suggested patch is acceptable in its form and will not harm
after optimization for small relations will be implemented.
(Disclaimer: I'm the author of the patch.)
> As for the patch being proposed, it looks like you're maybe just
> showing the answer to the question left in the comment below is "Yes".
>
> /*
> * Space/time tradeoff parameters: do these need to be user-tunable?
Well, the patch changes formula as well. It is harder to implement
user-tunable formula.
-------
regards
Yura Sokolov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: y.sokolov@postgrespro.ru, dgrowleyml@gmail.com, slpmcf@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Optimize truncation logic to reduce AccessExclusive lock impact
In-Reply-To: <0be607a5-20f6-4894-bb22-412e3c784bc2@postgrespro.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox