From a3f9114efaa75412eeaaede9120b280d46ec3a49 Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Tue, 11 Aug 2026 15:43:15 -0500 Subject: [PATCH v8 7/7] Fix VACUUM and autovacuum handling of TOAST storage parameters. Per the documentation for CREATE TABLE: If a table parameter value is set and the equivalent toast. parameter is not, the TOAST table will use the table's parameter value. Unfortunately, current reality does not match this description. Neither VACUUM nor autovacuum consults the main table's non-autovacuum storage parameters, and autovacuum only consults the main table's autovacuum-related storage parameters if the TOAST table lacks any. One silver lining is that all currently-supported TOAST storage parameters are related to vacuum, whose code paths already access the main table's parameters. This means that our solution needn't involve more lookups; we just need to propagate them correctly. To fix, this commit teaches autovacuum to combine the TOAST storage parameters with the main table's (with the toast.* ones winning if both are set), and it teaches VACUUM to send down the main table's parameters when recursing to a TOAST table. This doesn't fix VACUUM against a TOAST table directly (e.g., VACUUM pg_toast.pg_toast_5432), but that's probably okay because it's not the main supported way to vacuum a TOAST table (see VACUUM's PROCESS_MAIN and PROCESS_TOAST options). An existing shortcoming that this patch only makes worse is that autovacuum/VACUUM remain 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/access/common/reloptions.c | 93 ++++++++++ src/backend/commands/vacuum.c | 30 +++- src/backend/postmaster/autovacuum.c | 159 +++++++++++++++--- src/include/access/reloptions.h | 2 + src/include/commands/vacuum.h | 8 + .../injection_points/expected/vacuum.out | 29 ++++ .../modules/injection_points/sql/vacuum.sql | 16 ++ 7 files changed, 311 insertions(+), 26 deletions(-) diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c index 4548eb02676..60f1dc7ddd5 100644 --- a/src/backend/access/common/reloptions.c +++ b/src/backend/access/common/reloptions.c @@ -2114,6 +2114,99 @@ default_reloptions(Datum reloptions, bool validate, relopt_kind kind) lengthof(stdRdOptionsTab)); } +/* + * find_reloption + * Look up a reloption of the given kind by name. + * + * Returns NULL if no such option can be set on relations of that kind. Note + * that names are unique only within a kind; "fillfactor", for example, is + * declared separately for heaps and for each index access method. + */ +static relopt_gen * +find_reloption(const char *name, relopt_kind kind) +{ + if (need_initialization) + initialize_reloptions(); + + for (int i = 0; relOpts[i]; i++) + { + if ((relOpts[i]->kinds & kind) != 0 && + strcmp(relOpts[i]->name, name) == 0) + return relOpts[i]; + } + + return NULL; +} + +/* + * merge_toast_reloptions + * Fill in a TOAST table's unset options from its main table's. + * + * Any option that may be set on a TOAST table but was not is taken from + * main_opts. Either argument may be NULL; if both are, NULL is returned. + * Otherwise, the options to use are returned. + * + * An option counts as unset while it still holds the default declared for it + * above, which works because nothing a TOAST table accepts has a default the + * user could also set (see assert_toast_defaults_unsettable()). + * + * NB: This destructively modifies toast_opts, and what it returns may be + * either argument, so the caller must know which of the two it owns. + */ +StdRdOptions * +merge_toast_reloptions(StdRdOptions *toast_opts, StdRdOptions *main_opts) +{ + if (toast_opts == NULL) + return main_opts; + if (main_opts == NULL) + return toast_opts; + + for (int i = 0; i < lengthof(stdRdOptionsTab); i++) + { + const relopt_parse_elt *elem = &stdRdOptionsTab[i]; + relopt_gen *gen; + char *toast_val; + char *main_val; + + /* Skip anything that cannot be set on a TOAST table. */ + gen = find_reloption(elem->optname, RELOPT_KIND_TOAST); + if (gen == NULL) + continue; + + toast_val = (char *) toast_opts + elem->offset; + main_val = (char *) main_opts + elem->offset; + + switch (gen->type) + { + case RELOPT_TYPE_TERNARY: + if (*(pg_ternary *) toast_val == PG_TERNARY_UNSET) + *(pg_ternary *) toast_val = *(pg_ternary *) main_val; + break; + + case RELOPT_TYPE_INT: + if (*(int *) toast_val == ((relopt_int *) gen)->default_val) + *(int *) toast_val = *(int *) main_val; + break; + + case RELOPT_TYPE_REAL: + if (*(double *) toast_val == ((relopt_real *) gen)->default_val) + *(double *) toast_val = *(double *) main_val; + break; + + case RELOPT_TYPE_ENUM: + if (*(int *) toast_val == ((relopt_enum *) gen)->default_val) + *(int *) toast_val = *(int *) main_val; + break; + + default: + elog(ERROR, "reloption \"%s\" has a type a TOAST table cannot inherit", + elem->optname); + } + } + + return toast_opts; +} + /* * build_reloptions * diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 52116c02b59..3af66006586 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -187,6 +187,9 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) /* Will be set later if we recurse to a TOAST table. */ params.toast_parent = InvalidOid; + params.main_index_cleanup = VACOPTVALUE_UNSPECIFIED; + params.main_truncate = VACOPTVALUE_UNSPECIFIED; + params.main_max_eager_freeze_failure_rate = -1.0; /* * Set this to an invalid value so it is clear whether or not a @@ -2203,7 +2206,8 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams params, /* * Set index_cleanup option based on index_cleanup reloption if it wasn't - * specified in VACUUM command, or when running in an autovacuum worker + * specified in VACUUM command, or when running in an autovacuum worker. A + * TOAST table with no setting of its own inherits the main table's value. */ if (params.index_cleanup == VACOPTVALUE_UNSPECIFIED) { @@ -2227,11 +2231,16 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams params, params.index_cleanup = VACOPTVALUE_AUTO; break; case STDRD_OPTION_VACUUM_INDEX_CLEANUP_NOT_SET: - params.index_cleanup = VACOPTVALUE_AUTO; + if (params.main_index_cleanup != VACOPTVALUE_UNSPECIFIED) + params.index_cleanup = params.main_index_cleanup; + else + params.index_cleanup = VACOPTVALUE_AUTO; break; } } + toast_vacuum_params.main_index_cleanup = params.index_cleanup; + #ifdef USE_INJECTION_POINTS if (params.index_cleanup == VACOPTVALUE_AUTO) INJECTION_POINT("vacuum-index-cleanup-auto", NULL); @@ -2243,16 +2252,25 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams params, /* * Check if the vacuum_max_eager_freeze_failure_rate table storage - * parameter was specified. This overrides the GUC value. + * parameter was specified. This overrides the GUC value. A TOAST table + * with no setting of its own inherits the main table's value. */ if (rel->rd_options != NULL && ((StdRdOptions *) rel->rd_options)->vacuum_max_eager_freeze_failure_rate >= 0) params.max_eager_freeze_failure_rate = ((StdRdOptions *) rel->rd_options)->vacuum_max_eager_freeze_failure_rate; + else if (params.main_max_eager_freeze_failure_rate >= 0.0) + params.max_eager_freeze_failure_rate = + params.main_max_eager_freeze_failure_rate; + + toast_vacuum_params.main_max_eager_freeze_failure_rate = + params.max_eager_freeze_failure_rate; /* * Set truncate option based on truncate reloption or GUC if it wasn't - * specified in VACUUM command, or when running in an autovacuum worker + * specified in VACUUM command, or when running in an autovacuum worker. A + * TOAST table with no setting of its own inherits the main table's value + * before falling back to the GUC. */ if (params.truncate == VACOPTVALUE_UNSPECIFIED) { @@ -2265,12 +2283,16 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams params, else params.truncate = VACOPTVALUE_DISABLED; } + else if (params.main_truncate != VACOPTVALUE_UNSPECIFIED) + params.truncate = params.main_truncate; else if (vacuum_truncate) params.truncate = VACOPTVALUE_ENABLED; else params.truncate = VACOPTVALUE_DISABLED; } + toast_vacuum_params.main_truncate = params.truncate; + #ifdef USE_INJECTION_POINTS if (params.truncate == VACOPTVALUE_AUTO) INJECTION_POINT("vacuum-truncate-auto", NULL); diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index a99f7108636..881e07ecf9e 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -2015,9 +2015,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 @@ -2133,6 +2133,8 @@ do_autovacuum(void) bool doanalyze; bool wraparound; AutoVacuumScores scores; + av_relation *hentry; + bool found; /* * We cannot safely process other backends' temp tables, so skip 'em. @@ -2143,21 +2145,19 @@ do_autovacuum(void) relid = classForm->oid; /* - * fetch reloptions -- if this toast table does not have them, try the - * main rel + * fetch reloptions -- merge any unset options from the main rel + * + * Note that this fills in the storage parameters only VACUUM + * consults, too. That does no harm here; whether we process the + * table depends on the autovacuum parameters alone. */ 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; - } + hentry = hash_search(table_toast_map, &relid, HASH_FIND, &found); + if (found) + relopts = merge_toast_reloptions(relopts, &hentry->ar_reloptions); relation_needs_vacanalyze(relid, relopts, classForm, effective_multixact_freeze_max_age, @@ -2782,6 +2782,7 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map, autovac_table *tab = NULL; bool wraparound; StdRdOptions *relopts; + StdRdOptions *main_relopts = NULL; bool free_relopts = false; AutoVacuumScores scores; @@ -2792,20 +2793,24 @@ 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) + + 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; + { + main_relopts = &hentry->ar_reloptions; + relopts = merge_toast_reloptions(relopts, main_relopts); + } } relation_needs_vacanalyze(relid, relopts, classForm, @@ -2892,6 +2897,49 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map, tab->at_params.log_analyze_min_duration = log_analyze_min_duration; tab->at_params.toast_parent = InvalidOid; + /* + * For TOAST tables, provide fallbacks for the options that + * vacuum_rel() resolves from the relation's own reloptions, which do + * not have the main table's values merged in. + */ + tab->at_params.main_index_cleanup = VACOPTVALUE_UNSPECIFIED; + tab->at_params.main_truncate = VACOPTVALUE_UNSPECIFIED; + tab->at_params.main_max_eager_freeze_failure_rate = -1.0; + + if (main_relopts != NULL) + { + switch (main_relopts->vacuum_index_cleanup) + { + case STDRD_OPTION_VACUUM_INDEX_CLEANUP_ON: + tab->at_params.main_index_cleanup = VACOPTVALUE_ENABLED; + break; + case STDRD_OPTION_VACUUM_INDEX_CLEANUP_OFF: + tab->at_params.main_index_cleanup = VACOPTVALUE_DISABLED; + break; + case STDRD_OPTION_VACUUM_INDEX_CLEANUP_AUTO: + tab->at_params.main_index_cleanup = VACOPTVALUE_AUTO; + break; + case STDRD_OPTION_VACUUM_INDEX_CLEANUP_NOT_SET: + break; + } + + switch (main_relopts->vacuum_truncate) + { + case PG_TERNARY_TRUE: + tab->at_params.main_truncate = VACOPTVALUE_ENABLED; + break; + case PG_TERNARY_FALSE: + tab->at_params.main_truncate = VACOPTVALUE_DISABLED; + break; + case PG_TERNARY_UNSET: + break; + } + + if (main_relopts->vacuum_max_eager_freeze_failure_rate >= 0.0) + tab->at_params.main_max_eager_freeze_failure_rate = + main_relopts->vacuum_max_eager_freeze_failure_rate; + } + /* Determine the number of parallel vacuum workers to use */ tab->at_params.nworkers = 0; if (avopts) @@ -2948,9 +2996,9 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map, * "dovacuum" and "doanalyze", respectively. Also return whether the vacuum is * 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. + * relopts is a pointer to the StdRdOptions options (the relation's own for a + * plain table, or those 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 @@ -3607,6 +3655,8 @@ pg_stat_get_autovacuum_scores(PG_FUNCTION_ARGS) Relation rel; TableScanDesc scan; HeapTuple tup; + HTAB *table_toast_map; + HASHCTL ctl; ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo; InitMaterializedSRF(fcinfo, 0); @@ -3616,13 +3666,64 @@ 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); + rel = table_open(RelationRelationId, AccessShareLock); + + /* + * 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. + */ 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 parent tables' 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; + bool free_relopts = false; bool dovacuum; bool doanalyze; bool wraparound; @@ -3639,12 +3740,25 @@ pg_stat_get_autovacuum_scores(PG_FUNCTION_ARGS) continue; relopts = (StdRdOptions *) extractRelOptions(tup, RelationGetDescr(rel), NULL); + if (relopts) + free_relopts = true; + if (form->relkind == RELKIND_TOASTVALUE) + { + av_relation *hentry; + bool found; + + hentry = hash_search(table_toast_map, &form->oid, + HASH_FIND, &found); + if (found) + relopts = merge_toast_reloptions(relopts, &hentry->ar_reloptions); + } + 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); @@ -3662,6 +3776,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/include/access/reloptions.h b/src/include/access/reloptions.h index e8cb7f7a627..6c599382f02 100644 --- a/src/include/access/reloptions.h +++ b/src/include/access/reloptions.h @@ -247,6 +247,8 @@ extern void *build_local_reloptions(local_relopts *relopts, Datum options, extern bytea *default_reloptions(Datum reloptions, bool validate, relopt_kind kind); +extern struct StdRdOptions *merge_toast_reloptions(struct StdRdOptions *toast_opts, + struct StdRdOptions *main_opts); extern bytea *heap_reloptions(char relkind, Datum reloptions, bool validate); extern bytea *view_reloptions(Datum reloptions, bool validate); extern bytea *partitioned_table_reloptions(Datum reloptions, bool validate); diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h index e62f23748dc..fdfceb38396 100644 --- a/src/include/commands/vacuum.h +++ b/src/include/commands/vacuum.h @@ -248,6 +248,14 @@ typedef struct VacuumParams * disabled. */ int nworkers; + + /* + * Main table fallback values for TOAST tables to inherit when they have + * no setting of their own. + */ + VacOptValue main_index_cleanup; + VacOptValue main_truncate; + double main_max_eager_freeze_failure_rate; } VacuumParams; /* diff --git a/src/test/modules/injection_points/expected/vacuum.out b/src/test/modules/injection_points/expected/vacuum.out index 58df59fa927..1f9acc73594 100644 --- a/src/test/modules/injection_points/expected/vacuum.out +++ b/src/test/modules/injection_points/expected/vacuum.out @@ -79,9 +79,38 @@ NOTICE: notice triggered for injection point vacuum-truncate-enabled NOTICE: notice triggered for injection point vacuum-index-cleanup-auto NOTICE: notice triggered for injection point vacuum-truncate-enabled RESET vacuum_truncate; +-- TOAST table inherits main table's resolved values +CREATE TABLE vac_tab_toast_inherit(i int, j text STORAGE EXTERNAL) WITH + (autovacuum_enabled=false, + vacuum_index_cleanup=false, + autovacuum_vacuum_insert_threshold=1, + autovacuum_vacuum_insert_scale_factor=0, + vacuum_truncate=false, toast.vacuum_truncate=true); +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; +DROP TABLE vac_tab_toast_inherit; -- Cleanup SELECT injection_points_detach('vacuum-index-cleanup-auto'); injection_points_detach diff --git a/src/test/modules/injection_points/sql/vacuum.sql b/src/test/modules/injection_points/sql/vacuum.sql index 23760dd0f38..085f64337ad 100644 --- a/src/test/modules/injection_points/sql/vacuum.sql +++ b/src/test/modules/injection_points/sql/vacuum.sql @@ -33,9 +33,25 @@ SET vacuum_truncate = true; VACUUM vac_tab_auto; RESET vacuum_truncate; +-- TOAST table inherits main table's resolved values +CREATE TABLE vac_tab_toast_inherit(i int, j text STORAGE EXTERNAL) WITH + (autovacuum_enabled=false, + vacuum_index_cleanup=false, + autovacuum_vacuum_insert_threshold=1, + autovacuum_vacuum_insert_scale_factor=0, + vacuum_truncate=false, toast.vacuum_truncate=true); +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; DROP TABLE vac_tab_off_toast_on; +DROP TABLE vac_tab_toast_inherit; -- Cleanup SELECT injection_points_detach('vacuum-index-cleanup-auto'); -- 2.50.1 (Apple Git-155)