diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2025-05-23 14:43:43 -0400 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2025-05-23 14:43:43 -0400 |
commit | 02502c1bca544e25cc891d8ea596f6d89c540bcf (patch) | |
tree | 5b71d0a7d887255ff940566beffd36103d720c5a /src | |
parent | 6aa33afe6da1ce5e6a9c68900727db4621f899cd (diff) | |
download | postgresql-02502c1bca544e25cc891d8ea596f6d89c540bcf.tar.gz postgresql-02502c1bca544e25cc891d8ea596f6d89c540bcf.zip |
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
Diffstat (limited to 'src')
-rw-r--r-- | src/backend/postmaster/autovacuum.c | 50 |
1 files changed, 40 insertions, 10 deletions
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 6bcd797c36a..981be42e3af 100644 --- 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); |