Fix per-relation memory leakage in autovacuum.

PgStat_StatTabEntry and AutoVacOpts structs were leaked until
the end of the autovacuum worker's run, which is bad news if
there are a lot of relations in the database.

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.

Also ensure that the VacuumRelation structure passed to
vacuum() is in recoverable storage.

Back-patch to v15 where we started to manage table statistics
this way.  (The AutoVacOpts leakage is probably older, but
I'm not excited enough to worry about just that part.)

Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/285483.1746756246@sss.pgh.pa.us
Backpatch-through: 15

diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 6bcd797c36a1ff5ca56d8d86e8e5a3eedc1e87ef..981be42e3afc89c0e8c31cdf17c96cc07c31703f 100644 (file)
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2073,6 +2073,12 @@ do_autovacuum(void)
                }
            }
        }
+
+       /* Release stuff to avoid per-relation leakage */
+       if (relopts)
+           pfree(relopts);
+       if (tabentry)
+           pfree(tabentry);
    }
 
    table_endscan(relScan);
@@ -2089,7 +2095,8 @@ do_autovacuum(void)
        Form_pg_class classForm = (Form_pg_class) GETSTRUCT(tuple);
        PgStat_StatTabEntry *tabentry;
        Oid         relid;
-       AutoVacOpts *relopts = NULL;
+       AutoVacOpts *relopts;
+       bool        free_relopts = false;
        bool        dovacuum;
        bool        doanalyze;
        bool        wraparound;
@@ -2107,7 +2114,9 @@ do_autovacuum(void)
         * main rel
         */
        relopts = extract_autovac_opts(tuple, pg_class_desc);
-       if (relopts == NULL)
+       if (relopts)
+           free_relopts = true;
+       else
        {
            av_relation *hentry;
            bool        found;
@@ -2128,6 +2137,12 @@ do_autovacuum(void)
        /* ignore analyze for toast tables */
        if (dovacuum)
            table_oids = lappend_oid(table_oids, relid);
+
+       /* Release stuff to avoid leakage */
+       if (free_relopts)
+           pfree(relopts);
+       if (tabentry)
+           pfree(tabentry);
    }
 
    table_endscan(relScan);
@@ -2499,6 +2514,8 @@ deleted:
        pg_atomic_test_set_flag(&MyWorkerInfo->wi_dobalance);
    }
 
+   list_free(table_oids);
+
    /*
     * Perform additional work items, as requested by backends.
     */
@@ -2680,8 +2697,8 @@ deleted2:
 /*
  * extract_autovac_opts
  *
- * Given a relation's pg_class tuple, return the AutoVacOpts portion of
- * reloptions, if set; otherwise, return NULL.
+ * Given a relation's pg_class tuple, return a palloc'd copy of the
+ * AutoVacOpts portion of reloptions, if set; otherwise, return NULL.
  *
  * Note: callers do not have a relation lock on the table at this point,
  * so the table could have been dropped, and its catalog rows gone, after
@@ -2730,6 +2747,7 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
    autovac_table *tab = NULL;
    bool        wraparound;
    AutoVacOpts *avopts;
+   bool        free_avopts = false;
 
    /* fetch the relation's relcache entry */
    classTup = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(relid));
@@ -2742,8 +2760,10 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
     * main table reloptions if the toast table itself doesn't have.
     */
    avopts = extract_autovac_opts(classTup, pg_class_desc);
-   if (classForm->relkind == RELKIND_TOASTVALUE &&
-       avopts == NULL && table_toast_map != NULL)
+   if (avopts)
+       free_avopts = true;
+   else if (classForm->relkind == RELKIND_TOASTVALUE &&
+            table_toast_map != NULL)
    {
        av_relation *hentry;
        bool        found;
@@ -2852,6 +2872,8 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
                         avopts->vacuum_cost_delay >= 0));
    }
 
+   if (free_avopts)
+       pfree(avopts);
    heap_freetuple(classTup);
    return tab;
 }
@@ -2883,6 +2905,10 @@ recheck_relation_needs_vacanalyze(Oid relid,
                              effective_multixact_freeze_max_age,
                              dovacuum, doanalyze, wraparound);
 
+   /* Release tabentry to avoid leakage */
+   if (tabentry)
+       pfree(tabentry);
+
    /* ignore ANALYZE for toast tables */
    if (classForm->relkind == RELKIND_TOASTVALUE)
        *doanalyze = false;
@@ -3140,18 +3166,22 @@ autovacuum_do_vac_analyze(autovac_table *tab, BufferAccessStrategy bstrategy)
    VacuumRelation *rel;
    List       *rel_list;
    MemoryContext vac_context;
+   MemoryContext old_context;
 
    /* Let pgstat know what we're doing */
    autovac_report_activity(tab);
 
+   /* Create a context that vacuum() can use as cross-transaction storage */
+   vac_context = AllocSetContextCreate(CurrentMemoryContext,
+                                       "Vacuum",
+                                       ALLOCSET_DEFAULT_SIZES);
+
    /* Set up one VacuumRelation target, identified by OID, for vacuum() */
+   old_context = MemoryContextSwitchTo(vac_context);
    rangevar = makeRangeVar(tab->at_nspname, tab->at_relname, -1);
    rel = makeVacuumRelation(rangevar, tab->at_relid, NIL);
    rel_list = list_make1(rel);
-
-   vac_context = AllocSetContextCreate(CurrentMemoryContext,
-                                       "Vacuum",
-                                       ALLOCSET_DEFAULT_SIZES);
+   MemoryContextSwitchTo(old_context);
 
    vacuum(rel_list, &tab->at_params, bstrategy, vac_context, true);