8000 Fix per-relation memory leakage in autovacuum. · postgres/postgres@02502c1 · GitHub
[go: up one dir, main page]

Skip to content
8000

Commit 02502c1

Browse files
committed
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> 8000 ; Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/285483.1746756246@sss.pgh.pa.us Backpatch-through: 15
1 parent 6aa33af commit 02502c1

File tree

1 file changed

+40
-10
lines changed

1 file changed

+40
-10
lines changed

src/backend/postmaster/autovacuum.c

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2073,6 +2073,12 @@ do_autovacuum(void)
20732073
}
20742074
}
20752075
}
2076+
2077+
/* Release stuff to avoid per-relation leakage */
2078+
if (relopts)
2079+
pfree(relopts);
2080+
if (tabentry)
2081+
pfree(tabentry);
20762082
}
20772083

20782084
table_endscan(relScan);
@@ -2089,7 +2095,8 @@ do_autovacuum(void)
20892095
Form_pg_class classForm = (Form_pg_class) GETSTRUCT(tuple);
20902096
PgStat_StatTabEntry *tabentry;
20912097
Oid relid;
2092-
AutoVacOpts *relopts = NULL;
2098+
AutoVacOpts *relopts;
2099+
bool free_relopts = false;
20932100
bool dovacuum;
20942101
bool doanalyze;
20952102
bool wraparound;
@@ -2107,7 +2114,9 @@ do_autovacuum(void)
21072114
* main rel
21082115
*/
21092116
relopts = extract_autovac_opts(tuple, pg_class_desc);
2110-
if (relopts == NULL)
2117+
if (relopts)
2118+
free_relopts = true;
2119+
else
21112120
{
21122121
av_relation *hentry;
21132122
bool found;
@@ -2128,6 +2137,12 @@ do_autovacuum(void)
21282137
/* ignore analyze for toast tables */
21292138
if (dovacuum)
21302139
table_oids = lappend_oid(table_oids, relid);
2140+
2141+
/* Release stuff to avoid leakage */
2142+
if (free_relopts)
2143+
pfree(relopts);
2144+
if (tabentry)
2145+
pfree(tabentry);
21312146
}
21322147

21332148
table_endscan(relScan);
@@ -2499,6 +2514,8 @@ do_autovacuum(void)
24992514
pg_atomic_test_set_flag(&MyWorkerInfo->wi_dobalance);
25002515
}
25012516

2517+
list_free(table_oids);
2518+
25022519
/*
25032520
* Perform additional work items, as requested by backends.
25042521
*/
@@ -2680,8 +2697,8 @@ perform_work_item(AutoVacuumWorkItem *workitem)
26802697
/*
26812698
* extract_autovac_opts
26822699
*
2683-
* Given a relation's pg_class tuple, return the AutoVacOpts portion of
2684-
* reloptions, if set; otherwise, return NULL.
2700+
* Given a relation's pg_class tuple, return a palloc'd copy of the
2701+
* AutoVacOpts portion of reloptions, if set; otherwise, return NULL.
26852702
*
26862703
* Note: callers do not have a relation lock on the table at this point,
26872704
* 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,
27302747
autovac_table *tab = NULL;
27312748
bool wraparound;
27322749
AutoVacOpts *avopts;
2750+
bool free_avopts = false;
27332751

27342752
/* fetch the relation's relcache entry */
27352753
classTup = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(relid));
@@ -2742,8 +2760,10 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
27422760
* main table reloptions if the toast table itself doesn't have.
27432761
*/
27442762
avopts = extract_autovac_opts(classTup, pg_class_desc);
2745-
if (classForm->relkind == RELKIND_TOASTVALUE &&
2746-
avopts == NULL && table_toast_map != NULL)
2763+
if (avopts)
2764+
free_avopts = true;
2765+
else if (classForm->relkind == RELKIND_TOASTVALUE &&
2766+
table_toast_map != NULL)
27472767
{
27482768
av_relation *hentry;
27492769
bool found;
@@ -2852,6 +2872,8 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
28522872
avopts->vacuum_cost_delay >= 0));
28532873
}
28542874

2875+
if (free_avopts)
2876+
pfree(avopts);
28552877
heap_freetuple(classTup);
28562878
return tab;
28572879
}
@@ -2883,6 +2905,10 @@ recheck_relation_needs_vacanalyze(Oid relid,
28832905
effective_multixact_freeze_max_age,
28842906
dovacuum, doanalyze, wraparound);
28852907

2908+
/* Release tabentry to avoid leakage */
2909+
if (tabentry)
2910+
pfree(tabentry);
2911+
28862912
/* ignore ANALYZE for toast tables */
28872913
if (classForm->relkind == RELKIND_TOASTVALUE)
28882914
*doanalyze = false;
@@ -3140,18 +3166,22 @@ autovacuum_do_vac_analyze(autovac_table *tab, BufferAccessStrategy bstrategy)
31403166
VacuumRelation *rel;
31413167
List *rel_list;
31423168
MemoryContext vac_context;
3169+
MemoryContext old_context;
31433170

31443171
/* Let pgstat know what we're doing */
31453172
autovac_report_activity(tab);
31463173

3174+
/* Create a context that vacuum() can use as cross-transaction storage */
3175+
vac_context = AllocSetContextCreate(CurrentMemoryContext,
3176+
"Vacuum",
3177+
ALLOCSET_DEFAULT_SIZES);
3178+
31473179
/* Set up one VacuumRelation target, identified by OID, for vacuum() */
3180+
old_context = MemoryContextSwitchTo(vac_context);
31483181
rangevar = makeRangeVar(tab->at_nspname, tab->at_relname, -1);
31493182
rel = makeVacuumRelation(rangevar, tab->at_relid, NIL);
31503183
rel_list = list_make1(rel);
3151-
3152-
vac_context = AllocSetContextCreate(CurrentMemoryContext,
3153-
"Vacuum",
3154-
ALLOCSET_DEFAULT_SIZES);
3184+
MemoryContextSwitchTo(old_context);
31553185

31563186
vacuum(rel_list, &tab->at_params, bstrategy, vac_context, true);
31573187

0 commit comments

Comments
 (0)
0