public inbox for [email protected]  

help / color / mirror / Atom feed

From: Tom Lane <[email protected]>
To: Nathan Bossart <[email protected]>
Cc: Alexander Lakhin <[email protected]>
Cc: Sami Imseih <[email protected]>
Cc: Bharath Rupireddy <[email protected]>
Cc: Robert Treat <[email protected]>
Cc: [email protected]
Cc: pgsql-hackers <[email protected]>
Cc: [email protected]
Subject: Re: Add pg_stat_autovacuum_priority
Date: Wed, 08 Apr 2026 14:53:20 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <adaWuTR7oCKodH7k@nathan>
References: <CAA5RZ0t6bCR6Pasd_eWqXvyUmrNDWmHUy1yN_Po-c4+b9BtBqQ@mail.gmail.com>
	<adEhRzu4YM7ztLXr@nathan>
	<CAA5RZ0vxby2osMMaCuZ=680tmt583cF9n4rOzTGdsiS-1PJknA@mail.gmail.com>
	<adE9RbcA7lhH_w2d@nathan>
	<CAA5RZ0tYzPDeXbqz9uKacRV2xjD055qrg=s3w2PEQPK8EruuSg@mail.gmail.com>
	<adFiCgN22xJ7Z-oR@nathan>
	<CAA5RZ0vRP-W2wJD2OxEb-=VGj2sp5pMCqHQg9YJiuDVPhaY5jQ@mail.gmail.com>
	<adQsdvPPNviWMCXb@nathan>
	<[email protected]>
	<[email protected]>
	<adaWuTR7oCKodH7k@nathan>

Nathan Bossart <[email protected]> writes:
> Hm.  I can't get excited about checking pgstat_fetch_consistency (as
> proposed in that other report), but I see that commit 02502c1bca added the
> freeing behavior in question.  I wonder if it makes sense to just skip
> freeing when relation_needs_vacanalyze() is called from the view, i.e., not
> an autovacuum worker.  On the other hand, maybe we shouldn't be caching
> entries for a view like this that looks through all tables in the
> database...

<carp>
Oh.  I'm not happy that any part of autovacuum.c is now reachable from
SQL: that's the sort of modularity violation that will bite us on the
ass (indeed just did).  Aside from this problem, the elog's that
relation_needs_vacanalyze emits seem 100% inappropriate and misleading
when it's being called from the view.
</carp>

I think perhaps the right way forward is to rethink the API
guarantees for pgstat_fetch_stat_tabentry_ext, as I speculated
about in 02502c1bc:

    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.

I didn't want to do any such thing in a bug fix that needed to be
back-patched, but I see no reason we couldn't redefine that API
for v19.  Plausible alternatives:

1. Always return a freshly palloc'd struct.  Potentially adds
cycles, adds risk of a leak if caller forgets to pfree.

2. Add a "bool *should_free" parameter, like we have in tuplestores
and some other places.  It's on the caller to pfree if should_free
gets set, but since we'd have to touch every caller, we'd not miss
any.

3. Add a "bool please_palloc" parameter, signaling the caller's
intent to pfree.  Probably has no advantage over #2 though.

			regards, tom lane

view thread (60+ messages)  latest in thread

reply

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: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Add pg_stat_autovacuum_priority
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox