v1-0009-Avoid-per-relation-leakage-in-autovacuum.patch
text/x-diff
Filename: v1-0009-Avoid-per-relation-leakage-in-autovacuum.patch
Type: text/x-diff
Part: 8
Message:
Re: Why our Valgrind reports suck
Patch
Format: format-patch
Series: patch v1-0009
Subject: Avoid per-relation leakage in autovacuum.
| File | + | − |
|---|---|---|
| src/backend/postmaster/autovacuum.c | 40 | 7 |
From f5b72b015066053ed3396f0ea010ae470911f3b6 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sun, 11 May 2025 13:38:31 -0400
Subject: [PATCH v1 09/11] Avoid per-relation 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.
That API could use a re-think.
Also, free a couple of structures retail when USE_VALGRIND.
This doesn't have any effect on the process's total memory
consumption, but it does reduce noise in valgrind leakage
reports.
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/285483.1746756246@sss.pgh.pa.us
---
src/backend/postmaster/autovacuum.c | 47 ++++++++++++++++++++++++-----
1 file changed, 40 insertions(+), 7 deletions(-)
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 4d4a1a3197e..ebe8af1056d 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2077,6 +2077,12 @@ do_autovacuum(void)
}
}
}
+
+ /* Release stuff to avoid per-relation leakage */
+ if (relopts)
+ pfree(relopts);
+ if (tabentry)
+ pfree(tabentry);
}
table_endscan(relScan);
@@ -2093,7 +2099,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;
@@ -2111,7 +2118,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;
@@ -2132,6 +2141,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);
@@ -2503,6 +2518,8 @@ deleted:
pg_atomic_test_set_flag(&MyWorkerInfo->wi_dobalance);
}
+ list_free(table_oids);
+
/*
* Perform additional work items, as requested by backends.
*/
@@ -2545,8 +2562,15 @@ deleted:
/*
* We leak table_toast_map here (among other things), but since we're
- * going away soon, it's not a problem.
+ * going away soon, it's not a problem normally. But when using Valgrind,
+ * release some stuff to reduce complaints about leaked storage.
*/
+#ifdef USE_VALGRIND
+ hash_destroy(table_toast_map);
+ FreeTupleDesc(pg_class_desc);
+ if (bstrategy)
+ pfree(bstrategy);
+#endif
/*
* Update pg_database.datfrozenxid, and truncate pg_xact if possible. We
@@ -2684,8 +2708,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
@@ -2734,6 +2758,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));
@@ -2746,8 +2771,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;
@@ -2856,6 +2883,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;
}
@@ -2887,6 +2916,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;
--
2.43.5