Feedback-ID: id4a34324:Fastmail
Date: Fri, 10 Jul 2026 10:49:19 -0400
From: Andres Freund <andres@anarazel.de>
To: Melanie Plageman <melanieplageman@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Missing FSM Update when Updating VM On-access
Message-ID: <gmuwqrfyjdzmm7fu2uugvgmt32fz5itclkv7vftmbottkaj3i6@25u2clp2wwod>
References: 
 <CAAKRu_b2StZrEC=HmW8LePuQbczyFRnfs8qTAJwn_=W76-y24w@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CAAKRu_b2StZrEC=HmW8LePuQbczyFRnfs8qTAJwn_=W76-y24w@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/gmuwqrfyjdzmm7fu2uugvgmt32fz5itclkv7vftmbottkaj3i6%4025u2clp2wwod>
Precedence: bulk

Hi,

On 2026-06-29 18:32:47 -0400, Melanie Plageman wrote:
> I tried to think of some heuristics so that we could limit when we did
> the FSM pinning and locking, but none seemed very good. We could check
> if the new amount of free space is bigger than an FSM category step
> (32), but that doesn't help us if we are correcting an FSM
> overestimation. This could happen because inserts don't update the FSM
> until the inserting tuple doesn't fit on the target page.

One potential heuristic would be to skip trying to update the FSM if the page
is considered full and we didn't remove any tuples (i.e. just marked it as
all-visible). Skipping the FSM update in that case would, at worst, lead to
unnecessarily visiting the page during a future insert (finding it to be full
when doing so).  We could additionally make it depend on whether PD_PAGE_FULL
is set (in which case we presumably already updated the FSM).

The case where that would potentially help is when doing the first scan
through a freshly bulk loaded table.

However, compared to the other costs in that case, I have a hard time
believing the FSM access is that bad. And given the point in the release
cycle, I think going for simpler is the way to go.


> I also thought of caching the pinned FSM page in the scan descriptor
> like we do with the VM page. This doesn't work as nicely because each
> FSM page covers fewer heap pages.

I don't think that'd be an issue, an FSM page still covers thousands of heap
pages. I'd guess we'd need maybe a dozen pages or so to make the pin cost
basically invisible.


> Also, the pinning and unpinning all happens inside of the FSM API functions.

However that definitely is an issue. I think it'd be good to renovate the FSM
functions, which also would allow us to do things like pinning the FSM page
before a critical section and then updating it as part of the CS, like we
already do for the VM.

The biggest benefit we could get is to avoid a lot of repeated work during
relation extension. We do separate RecordPageWithFreeSpace() calls for each
newly created page, and then a FreeSpaceMapVacuumRange() for all of
them. That's a lot of repeated pinning. And I've seen those accesses be rather
contended in concurrent COPY, so making them more efficient would be rather
nice.

But that's all clearly >19 material.


> Therefore, I think the best option is the simplest -- if we set the
> page all-visible, also see if we should update the FSM.

Agreed.

> @@ -376,16 +380,32 @@ heap_page_prune_opt(Relation relation, Buffer buffer, Buffer *vmbuffer,
>  			if (presult.ndeleted > presult.nnewlpdead)
>  				pgstat_update_heap_dead_tuples(relation,
>  											   presult.ndeleted - presult.nnewlpdead);
> +
> +			/*
> +			 * If this prune newly set the page all-visible, VACUUM may later
> +			 * skip the page and thus not update its free space map (FSM)
> +			 * entry. Keep the FSM from going stale by recording it now. We do
> +			 * not want to update the freespace map otherwise (to reserve
> +			 * freespace on this page for future updates).
> +			 */
> +			if (presult.newly_all_visible)
> +			{
> +				record_free_space = true;
> +				freespace = PageGetHeapFreeSpace(page);
> +			}
>  		}
>  
>  		/* And release buffer lock */
>  		LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
>  
>  		/*
> -		 * We avoid reuse of any free space created on the page by unrelated
> -		 * UPDATEs/INSERTs by opting to not update the FSM at this point.  The
> -		 * free space should be reused by UPDATEs to *this* page.

Does the logic in this removed comment actually make sense to anyone? Because
it sure doesn't to me.  I don't even know what the hell "unrelated"
UPDATEs/INSERTs means here. We don't have the slightest idea what's related
and unrelated, assuming that means it's for different tuples than what this
backend is reading the page for.

Sure, in-page updates are good (hot, general efficiency). But we do update the
FSM in other places without this consideration, so using that argument to skip
an FSM update here seems ... bogus.


I could see an argument for not inserting almost full pages into the FSM, but
if we went for that, it'd not be just this place we'd want to do so.

Greetings,

Andres Freund