Re: Add pg_stat_autovacuum_priority

Nathan Bossart <nathandbossart@gmail.com> 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