From 7bebf3e0720875ea970a17ae8e360f29fb83e9ac Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Wed, 12 Aug 2026 12:02:21 -0500 Subject: [PATCH v10 8/8] Fix autovacuum's handling of TOAST storage parameters. The previous commit made VACUUM apply a main table's storage parameters to its TOAST table, as CREATE TABLE has long documented. Autovacuum still gets this wrong in two ways. It falls back to the main table's autovacuum parameters only when the TOAST table has no reloptions at all, so setting a single toast.* parameter silently discards the rest. And it never consults the main table for the parameters that only VACUUM reads, since it leaves those for vacuum_rel() to resolve from the TOAST table's own reloptions. To fix, combine the two sets with merge_toast_reloptions() rather than choosing between them, and hand the main table's parameters down to vacuum_rel() the way VACUUM now does. pg_stat_autovacuum_scores uses the combined parameters for TOAST tables as well; it has to collect the main relations' parameters before it can do so, so it now makes a preliminary pass over pg_class. An existing shortcoming that this patch only makes worse is that autovacuum remains oblivious to concurrent storage parameter changes on the main table. That is, the main table's parameters may be captured long before its TOAST table is processed, and a user may very well have altered the settings in the meantime. Fixing that would likely require additional pg_class lookups, and it's not clear if it's worth the trouble. While this is a bug fix, it's too intrusive for back-patching, but the issue seems to have gone unnoticed for a very long time, anyway. --- src/backend/postmaster/autovacuum.c | 147 +++++++++++++----- .../injection_points/expected/vacuum.out | 22 ++- .../modules/injection_points/sql/vacuum.sql | 12 +- 3 files changed, 136 insertions(+), 45 deletions(-) diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 1610c60ec4b..a7e97b880f5 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -1913,6 +1913,36 @@ TableToProcessComparator(const ListCell *a, const ListCell *b) return (t2->score < t1->score) ? -1 : (t2->score > t1->score) ? 1 : 0; } +/* + * get_effective_relopts + * Fetch the storage parameters that apply to a relation. + * + * This looks up and, for TOAST tables, merges reloptions. Returns NULL if the + * relation neither sets nor inherits anything. main_opts returns main table's + * reloptions, if any. free_relopts returns whether return value is palloc'd. + */ +static StdRdOptions * +get_effective_relopts(HeapTuple tup, TupleDesc desc, HTAB *toast_map, + StdRdOptions **main_opts, bool *free_relopts) +{ + Form_pg_class classForm = (Form_pg_class) GETSTRUCT(tup); + StdRdOptions *relopts; + av_relation *hentry; + bool found = false; + + /* look up our relopts */ + relopts = (StdRdOptions *) extractRelOptions(tup, desc, NULL); + *free_relopts = (relopts != NULL); + + /* if we're a TOAST table, look up our parent's relopts, too */ + if (classForm->relkind == RELKIND_TOASTVALUE) + hentry = hash_search(toast_map, &classForm->oid, HASH_FIND, &found); + *main_opts = found ? &hentry->ar_reloptions : NULL; + + /* return the merged reloptions */ + return merge_toast_reloptions(relopts, *main_opts); +} + /* * Process a database table-by-table * @@ -2015,9 +2045,9 @@ do_autovacuum(void) * We do this in two passes: on the first one we collect the list of plain * relations and materialized views, and on the second one we collect * TOAST tables. The reason for doing the second pass is that during it we - * want to use the main relation's pg_class.reloptions entry if the TOAST - * table does not have any, and we cannot obtain it unless we know - * beforehand what's the main table OID. + * want to fill in any storage parameters that the TOAST table does not + * set with the main relation's, and we cannot obtain those values unless + * we know beforehand what's the main table OID. * * We need to check TOAST tables separately because in cases with short, * wide tables there might be proportionally much more activity in the @@ -2128,7 +2158,8 @@ do_autovacuum(void) Form_pg_class classForm = (Form_pg_class) GETSTRUCT(tuple); Oid relid; StdRdOptions *relopts; - bool free_relopts = false; + StdRdOptions *main_relopts; + bool free_relopts; bool dovacuum; bool doanalyze; bool wraparound; @@ -2142,22 +2173,9 @@ do_autovacuum(void) relid = classForm->oid; - /* - * fetch reloptions -- if this toast table does not have them, try the - * main rel - */ - relopts = (StdRdOptions *) extractRelOptions(tuple, pg_class_desc, NULL); - if (relopts) - free_relopts = true; - else - { - av_relation *hentry; - bool found; - - hentry = hash_search(table_toast_map, &relid, HASH_FIND, &found); - if (found) - relopts = &hentry->ar_reloptions; - } + /* fetch reloptions -- merge any unset options from the main rel */ + relopts = get_effective_relopts(tuple, pg_class_desc, table_toast_map, + &main_relopts, &free_relopts); relation_needs_vacanalyze(relid, relopts, classForm, effective_multixact_freeze_max_age, @@ -2782,7 +2800,8 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map, autovac_table *tab = NULL; bool wraparound; StdRdOptions *relopts; - bool free_relopts = false; + StdRdOptions *main_relopts; + bool free_relopts; AutoVacuumScores scores; /* fetch the relation's relcache entry */ @@ -2792,21 +2811,11 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map, classForm = (Form_pg_class) GETSTRUCT(classTup); /* - * Get the applicable reloptions. If it is a TOAST table, try to get the - * main table reloptions if the toast table itself doesn't have. + * Get the applicable reloptions. If it is a TOAST table, merge in the + * main table's reloptions where they are unset. */ - relopts = (StdRdOptions *) extractRelOptions(classTup, pg_class_desc, NULL); - if (relopts) - free_relopts = true; - else if (classForm->relkind == RELKIND_TOASTVALUE) - { - av_relation *hentry; - bool found; - - hentry = hash_search(table_toast_map, &relid, HASH_FIND, &found); - if (found) - relopts = &hentry->ar_reloptions; - } + relopts = get_effective_relopts(classTup, pg_class_desc, table_toast_map, + &main_relopts, &free_relopts); relation_needs_vacanalyze(relid, relopts, classForm, effective_multixact_freeze_max_age, @@ -2891,7 +2900,7 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map, tab->at_params.log_vacuum_min_duration = log_vacuum_min_duration; tab->at_params.log_analyze_min_duration = log_analyze_min_duration; tab->at_params.toast_parent = InvalidOid; - tab->at_params.main_relopts = NULL; + tab->at_params.main_relopts = main_relopts; /* Determine the number of parallel vacuum workers to use */ tab->at_params.nworkers = 0; @@ -2950,8 +2959,8 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map, * being forced because of Xid or multixact wraparound. * * relopts is a pointer to the StdRdOptions options (either for itself in the - * case of a plain table, or for either itself or its parent table in the case - * of a TOAST table), NULL if none. + * case of a plain table, or merged with the main table's for a TOAST table), + * NULL if none. * * A table needs to be vacuumed if the number of dead tuples exceeds a * threshold. This threshold is calculated as @@ -3609,6 +3618,8 @@ pg_stat_get_autovacuum_scores(PG_FUNCTION_ARGS) TableScanDesc scan; HeapTuple tup; ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo; + HTAB *table_toast_map; + HASHCTL ctl; InitMaterializedSRF(fcinfo, 0); @@ -3617,13 +3628,63 @@ pg_stat_get_autovacuum_scores(PG_FUNCTION_ARGS) recentXid = ReadNextTransactionId(); recentMulti = ReadNextMultiXactId(); - /* scan pg_class */ + /* create hash table for toast <-> main relid mapping */ + ctl.keysize = sizeof(Oid); + ctl.entrysize = sizeof(av_relation); + ctl.hcxt = CurrentMemoryContext; + table_toast_map = hash_create("TOAST to main relid map", + 100, + &ctl, + HASH_ELEM | HASH_BLOBS | HASH_CONTEXT); + + /* + * Do an initial pass over pg_class to collect the main relations' + * reloptions, which we need in order to compute their TOAST tables' + * effective options below. + */ rel = table_open(RelationRelationId, AccessShareLock); scan = table_beginscan_catalog(rel, 0, NULL); while ((tup = heap_getnext(scan, ForwardScanDirection)) != NULL) { Form_pg_class form = (Form_pg_class) GETSTRUCT(tup); StdRdOptions *relopts; + av_relation *hentry; + bool found; + + /* skip ineligible entries */ + if (form->relkind != RELKIND_RELATION && + form->relkind != RELKIND_MATVIEW) + continue; + if (form->relpersistence == RELPERSISTENCE_TEMP) + continue; + if (!OidIsValid(form->reltoastrelid)) + continue; + + relopts = (StdRdOptions *) extractRelOptions(tup, RelationGetDescr(rel), NULL); + if (!relopts) + continue; + + hentry = hash_search(table_toast_map, &form->reltoastrelid, + HASH_ENTER, &found); + Assert(!found); /* rels cannot share a TOAST table */ + + /* hash_search already filled in the key */ + memcpy(&hentry->ar_reloptions, relopts, sizeof(StdRdOptions)); + + pfree(relopts); + } + table_endscan(scan); + + /* + * Now that we have all parents' reloptions, we can generate the results. + */ + scan = table_beginscan_catalog(rel, 0, NULL); + while ((tup = heap_getnext(scan, ForwardScanDirection)) != NULL) + { + Form_pg_class form = (Form_pg_class) GETSTRUCT(tup); + StdRdOptions *relopts; + StdRdOptions *main_relopts; + bool free_relopts; bool dovacuum; bool doanalyze; bool wraparound; @@ -3639,13 +3700,16 @@ pg_stat_get_autovacuum_scores(PG_FUNCTION_ARGS) if (form->relpersistence == RELPERSISTENCE_TEMP) continue; - relopts = (StdRdOptions *) extractRelOptions(tup, RelationGetDescr(rel), NULL); + relopts = get_effective_relopts(tup, RelationGetDescr(rel), + table_toast_map, &main_relopts, + &free_relopts); + relation_needs_vacanalyze(form->oid, relopts, form, effective_multixact_freeze_max_age, LOG_NEVER, &dovacuum, &doanalyze, &wraparound, &scores); - if (relopts) + if (free_relopts) pfree(relopts); vals[0] = ObjectIdGetDatum(form->oid); @@ -3663,6 +3727,7 @@ pg_stat_get_autovacuum_scores(PG_FUNCTION_ARGS) } table_endscan(scan); table_close(rel, AccessShareLock); + hash_destroy(table_toast_map); return (Datum) 0; } diff --git a/src/test/modules/injection_points/expected/vacuum.out b/src/test/modules/injection_points/expected/vacuum.out index caf0cc232b4..196be88c6b8 100644 --- a/src/test/modules/injection_points/expected/vacuum.out +++ b/src/test/modules/injection_points/expected/vacuum.out @@ -80,15 +80,33 @@ NOTICE: notice triggered for injection point vacuum-index-cleanup-auto NOTICE: notice triggered for injection point vacuum-truncate-enabled RESET vacuum_truncate; -- A TOAST table inherits what it does not set from its main table. -CREATE TABLE vac_tab_toast_inherit(i int, j text) WITH +CREATE TABLE vac_tab_toast_inherit(i int, j text STORAGE EXTERNAL) WITH (autovacuum_enabled=false, vacuum_index_cleanup=false, - vacuum_truncate=false, toast.vacuum_truncate=true); + vacuum_truncate=false, toast.vacuum_truncate=true, + autovacuum_vacuum_insert_threshold=1, + autovacuum_vacuum_insert_scale_factor=0); VACUUM vac_tab_toast_inherit; NOTICE: notice triggered for injection point vacuum-index-cleanup-disabled NOTICE: notice triggered for injection point vacuum-truncate-disabled NOTICE: notice triggered for injection point vacuum-index-cleanup-disabled NOTICE: notice triggered for injection point vacuum-truncate-enabled +INSERT INTO vac_tab_toast_inherit + VALUES (1, repeat('a', 10000)), (2, repeat('b', 10000)); +SELECT pg_stat_force_next_flush(); + pg_stat_force_next_flush +-------------------------- + +(1 row) + +SELECT s.vacuum_insert_score > 1 AS over + FROM pg_class c, pg_stat_autovacuum_scores s + WHERE s.relid = c.reltoastrelid AND c.relname = 'vac_tab_toast_inherit'; + over +------ + t +(1 row) + DROP TABLE vac_tab_auto; DROP TABLE vac_tab_on_toast_off; DROP TABLE vac_tab_off_toast_on; diff --git a/src/test/modules/injection_points/sql/vacuum.sql b/src/test/modules/injection_points/sql/vacuum.sql index 0a43e14c928..dc5f25dbb58 100644 --- a/src/test/modules/injection_points/sql/vacuum.sql +++ b/src/test/modules/injection_points/sql/vacuum.sql @@ -34,11 +34,19 @@ VACUUM vac_tab_auto; RESET vacuum_truncate; -- A TOAST table inherits what it does not set from its main table. -CREATE TABLE vac_tab_toast_inherit(i int, j text) WITH +CREATE TABLE vac_tab_toast_inherit(i int, j text STORAGE EXTERNAL) WITH (autovacuum_enabled=false, vacuum_index_cleanup=false, - vacuum_truncate=false, toast.vacuum_truncate=true); + vacuum_truncate=false, toast.vacuum_truncate=true, + autovacuum_vacuum_insert_threshold=1, + autovacuum_vacuum_insert_scale_factor=0); VACUUM vac_tab_toast_inherit; +INSERT INTO vac_tab_toast_inherit + VALUES (1, repeat('a', 10000)), (2, repeat('b', 10000)); +SELECT pg_stat_force_next_flush(); +SELECT s.vacuum_insert_score > 1 AS over + FROM pg_class c, pg_stat_autovacuum_scores s + WHERE s.relid = c.reltoastrelid AND c.relname = 'vac_tab_toast_inherit'; DROP TABLE vac_tab_auto; DROP TABLE vac_tab_on_toast_off; -- 2.50.1 (Apple Git-155)