Feedback-ID: id4a34324:Fastmail
Date: Wed, 8 Apr 2026 14:44:30 -0400
From: Andres Freund <andres@anarazel.de>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: Sami Imseih <samimseih@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>,
	Alexander Lakhin <exclusion@gmail.com>,
 Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>,
	Robert Treat <rob@xzilla.net>, satyanarlapuram@gmail.com,
	pgsql-hackers <pgsql-hackers@postgresql.org>, tndrwang@gmail.com
Subject: Re: Add pg_stat_autovacuum_priority
Message-ID: <c4ftgtosizlzwo65kurhsbjhvtvq4qilfsfdbwnwohn6fsqpg6@5t5aspbgkifc>
References: 
 <CAA5RZ0tYzPDeXbqz9uKacRV2xjD055qrg=s3w2PEQPK8EruuSg@mail.gmail.com>
 <adFiCgN22xJ7Z-oR@nathan>
 <CAA5RZ0vRP-W2wJD2OxEb-=VGj2sp5pMCqHQg9YJiuDVPhaY5jQ@mail.gmail.com>
 <adQsdvPPNviWMCXb@nathan>
 <5684f479-858e-4c5d-b8f5-bcf05de1f909@gmail.com>
 <3077290.1775668665@sss.pgh.pa.us>
 <adaWuTR7oCKodH7k@nathan>
 <adacEjlRZco26f0h@nathan>
 <CAA5RZ0tUq7SEHvAPkYpCw7Y4XOoWLPpsfxO2ZQ5AzQRcQMMAgg@mail.gmail.com>
 <adaga-DMFE2KnhXO@nathan>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <adaga-DMFE2KnhXO@nathan>
Archived-At: 
 <https://www.postgresql.org/message-id/c4ftgtosizlzwo65kurhsbjhvtvq4qilfsfdbwnwohn6fsqpg6%405t5aspbgkifc>
Precedence: bulk

Hi,

On 2026-04-08 13:37:31 -0500, Nathan Bossart wrote:
> On Wed, Apr 08, 2026 at 01:28:06PM -0500, Sami Imseih wrote:
> >> +    if (AmAutoVacuumWorkerProcess())
> >> +        pfree(tabentry);
> >>  }
> > 
> > This works too, but v1-0001 is more generalized and we don't have to
> > care about who the caller is when deciding to free or not.
> 
> Well, I think if we were building this view from scratch without any
> knowledge of autovacuum, we probably wouldnt bother manually freeing the
> stats entries based on the value of the GUC.  We'd probably just let the
> commit/abort code take care of it, which AFAICT is what we do elsewhere.
> Manually freeing the entries is also risky, which commit 02502c1bca touches
> on:
> 
>     Note: pfree'ing the PgStat_StatTabEntry structs here seems a bit
>     risky, because pgstat_fetch_stat_tabentry_ext does not guarantee
>     anything about whether its result is long-lived.  It appears okay
>     so long as autovacuum forces PGSTAT_FETCH_CONSISTENCY_NONE, but
>     I think that API could use a re-think.
> 
> This seems very much to be an autovacuum-specific hack that we shouldn't be
> propagating elsewhere.

Agreed.  There aren't that many callers of pgstat_fetch_entry (even if you
count indirect ones), I think the best path might be to add a bool *may_free
argument where necessary and just deal with the size of change that brings
with it.

Greetings,

Andres Freund