1
0
mirror of https://github.com/postgres/postgres.git synced 2025-06-08 22:02:03 +03:00

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
This commit is contained in:
Tom Lane 2025-05-23 14:43:44 -04:00
parent ee58de1008
commit e087b5b794

View File

@ -2211,6 +2211,12 @@ do_autovacuum(void)
} }
} }
} }
/* Release stuff to avoid per-relation leakage */
if (relopts)
pfree(relopts);
if (tabentry)
pfree(tabentry);
} }
table_endscan(relScan); table_endscan(relScan);
@ -2227,7 +2233,8 @@ do_autovacuum(void)
Form_pg_class classForm = (Form_pg_class) GETSTRUCT(tuple); Form_pg_class classForm = (Form_pg_class) GETSTRUCT(tuple);
PgStat_StatTabEntry *tabentry; PgStat_StatTabEntry *tabentry;
Oid relid; Oid relid;
AutoVacOpts *relopts = NULL; AutoVacOpts *relopts;
bool free_relopts = false;
bool dovacuum; bool dovacuum;
bool doanalyze; bool doanalyze;
bool wraparound; bool wraparound;
@ -2245,7 +2252,9 @@ do_autovacuum(void)
* main rel * main rel
*/ */
relopts = extract_autovac_opts(tuple, pg_class_desc); relopts = extract_autovac_opts(tuple, pg_class_desc);
if (relopts == NULL) if (relopts)
free_relopts = true;
else
{ {
av_relation *hentry; av_relation *hentry;
bool found; bool found;
@ -2266,6 +2275,12 @@ do_autovacuum(void)
/* ignore analyze for toast tables */ /* ignore analyze for toast tables */
if (dovacuum) if (dovacuum)
table_oids = lappend_oid(table_oids, relid); table_oids = lappend_oid(table_oids, relid);
/* Release stuff to avoid leakage */
if (free_relopts)
pfree(relopts);
if (tabentry)
pfree(tabentry);
} }
table_endscan(relScan); table_endscan(relScan);
@ -2637,6 +2652,8 @@ deleted:
pg_atomic_test_set_flag(&MyWorkerInfo->wi_dobalance); pg_atomic_test_set_flag(&MyWorkerInfo->wi_dobalance);
} }
list_free(table_oids);
/* /*
* Perform additional work items, as requested by backends. * Perform additional work items, as requested by backends.
*/ */
@ -2818,8 +2835,8 @@ deleted2:
/* /*
* extract_autovac_opts * extract_autovac_opts
* *
* Given a relation's pg_class tuple, return the AutoVacOpts portion of * Given a relation's pg_class tuple, return a palloc'd copy of the
* reloptions, if set; otherwise, return NULL. * AutoVacOpts portion of reloptions, if set; otherwise, return NULL.
* *
* Note: callers do not have a relation lock on the table at this point, * 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 * so the table could have been dropped, and its catalog rows gone, after
@ -2868,6 +2885,7 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
autovac_table *tab = NULL; autovac_table *tab = NULL;
bool wraparound; bool wraparound;
AutoVacOpts *avopts; AutoVacOpts *avopts;
bool free_avopts = false;
/* fetch the relation's relcache entry */ /* fetch the relation's relcache entry */
classTup = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(relid)); classTup = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(relid));
@ -2880,8 +2898,10 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
* main table reloptions if the toast table itself doesn't have. * main table reloptions if the toast table itself doesn't have.
*/ */
avopts = extract_autovac_opts(classTup, pg_class_desc); avopts = extract_autovac_opts(classTup, pg_class_desc);
if (classForm->relkind == RELKIND_TOASTVALUE && if (avopts)
avopts == NULL && table_toast_map != NULL) free_avopts = true;
else if (classForm->relkind == RELKIND_TOASTVALUE &&
table_toast_map != NULL)
{ {
av_relation *hentry; av_relation *hentry;
bool found; bool found;
@ -2983,6 +3003,8 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
avopts->vacuum_cost_delay >= 0)); avopts->vacuum_cost_delay >= 0));
} }
if (free_avopts)
pfree(avopts);
heap_freetuple(classTup); heap_freetuple(classTup);
return tab; return tab;
} }
@ -3014,6 +3036,10 @@ recheck_relation_needs_vacanalyze(Oid relid,
effective_multixact_freeze_max_age, effective_multixact_freeze_max_age,
dovacuum, doanalyze, wraparound); dovacuum, doanalyze, wraparound);
/* Release tabentry to avoid leakage */
if (tabentry)
pfree(tabentry);
/* ignore ANALYZE for toast tables */ /* ignore ANALYZE for toast tables */
if (classForm->relkind == RELKIND_TOASTVALUE) if (classForm->relkind == RELKIND_TOASTVALUE)
*doanalyze = false; *doanalyze = false;
@ -3236,19 +3262,23 @@ autovacuum_do_vac_analyze(autovac_table *tab, BufferAccessStrategy bstrategy)
VacuumRelation *rel; VacuumRelation *rel;
List *rel_list; List *rel_list;
MemoryContext vac_context; MemoryContext vac_context;
MemoryContext old_context;
/* Let pgstat know what we're doing */ /* Let pgstat know what we're doing */
autovac_report_activity(tab); autovac_report_activity(tab);
/* Set up one VacuumRelation target, identified by OID, for vacuum() */ /* Create a context that vacuum() can use as cross-transaction storage */
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, vac_context = AllocSetContextCreate(CurrentMemoryContext,
"Vacuum", "Vacuum",
ALLOCSET_DEFAULT_SIZES); 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);
MemoryContextSwitchTo(old_context);
vacuum(rel_list, &tab->at_params, bstrategy, vac_context, true); vacuum(rel_list, &tab->at_params, bstrategy, vac_context, true);
MemoryContextDelete(vac_context); MemoryContextDelete(vac_context);