On Mon, Jun 23, 2025 at 10:38:40AM -0500, Nathan Bossart wrote: > Is the idea to do something like this for v19 and this [0] for the > back-branches?
Yes, that would be the idea. Let's do the bug fix first on all the branches, then do the refactoring. Knowing that I'm indirectly responsible for this mess, I would like to take care of that myself. Would that be OK for you? > I think the recurse-to-TOAST-table case is still broken with your patch. > We should probably move the memcpy() for toast_vacuum_params to the very > top of vacuum_rel(). Otherwise, your patch looks generally reasonable to > me. Ah, right, I have managed to mess up this part. Sounds to me that we should provide more coverage for both the truncate and index cleanup cases, as well. The updated version attached includes tests that were enough to make the recursive TOAST table case fail for TRUNCATE and INDEX_CLEANUP, where I have relied on an EXTERNAL column to fill in the TOAST relation with data cheaply. The pgstattuple case is less cheap but I've minimized the load to be able to trigger the index cleanup. It was tricky to get the threshold right for INDEX_CLEANUP in the case of TOAST table, but that's small enough to have a short run time while it should be large enough to trigger a cleanup of the TOAST indexes (my CI quota has been reached this month, so I can only rely on the CF bot for some pre-validation job). If the thresholds are too low, we may need to bump the number of tuples. I'd be OK to remove the TOAST parts if these prove to be unstable, but let's see. At least these tests are validating the contents of the patch for the TOAST options, and they were looking stable on my machine. Another approach that we could use is an injection point with some LOGs that show some information based on what VacuumParams holds. That would be the cheapest method (no need for any data), and entirely stable as we would look at the stack. Perhaps going down to that is not really necessary for the sake of this thread. Attached are two patches to have a clean git history: - 0001 is the basic fix, to-be-applied first on all the branches. - 0002 is the refactoring, for v19 once it opens up. What do you think? -- Michael
From 2f2390c05725695d2e2d60845e5146710926fbbc Mon Sep 17 00:00:00 2001 From: Michael Paquier <mich...@paquier.xyz> Date: Tue, 24 Jun 2025 10:06:09 +0900 Subject: [PATCH v4 1/2] Avoid scribbling VACUUM options Blah, this is the basic patch that goes through all the back branches, with tests. Author: Nathan B. Backpatch-through: 13 --- src/backend/commands/vacuum.c | 20 ++++-- src/test/regress/expected/vacuum.out | 71 ++++++++++++++++++++ src/test/regress/sql/vacuum.sql | 32 +++++++++ contrib/pgstattuple/expected/pgstattuple.out | 47 +++++++++++++ contrib/pgstattuple/sql/pgstattuple.sql | 32 +++++++++ 5 files changed, 198 insertions(+), 4 deletions(-) diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 33a33bf6b1cf..a43f090ee178 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -634,7 +634,15 @@ vacuum(List *relations, VacuumParams *params, BufferAccessStrategy bstrategy, if (params->options & VACOPT_VACUUM) { - if (!vacuum_rel(vrel->oid, vrel->relation, params, bstrategy)) + VacuumParams params_copy; + + /* + * vacuum_rel() scribbles on the parameters, so give it a copy + * to avoid affecting other relations. + */ + memcpy(¶ms_copy, params, sizeof(VacuumParams)); + + if (!vacuum_rel(vrel->oid, vrel->relation, ¶ms_copy, bstrategy)) continue; } @@ -2008,9 +2016,16 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params, Oid save_userid; int save_sec_context; int save_nestlevel; + VacuumParams toast_vacuum_params; Assert(params != NULL); + /* + * This function scribbles on the parameters, so make a copy early to + * avoid affecting the TOAST table (if we do end up recursing to it). + */ + memcpy(&toast_vacuum_params, params, sizeof(VacuumParams)); + /* Begin a transaction for vacuuming this relation */ StartTransactionCommand(); @@ -2299,15 +2314,12 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params, */ if (toast_relid != InvalidOid) { - VacuumParams toast_vacuum_params; - /* * Force VACOPT_PROCESS_MAIN so vacuum_rel() processes it. Likewise, * set toast_parent so that the privilege checks are done on the main * relation. NB: This is only safe to do because we hold a session * lock on the main relation that prevents concurrent deletion. */ - memcpy(&toast_vacuum_params, params, sizeof(VacuumParams)); toast_vacuum_params.options |= VACOPT_PROCESS_MAIN; toast_vacuum_params.toast_parent = relid; diff --git a/src/test/regress/expected/vacuum.out b/src/test/regress/expected/vacuum.out index 0abcc99989e0..f403ac190601 100644 --- a/src/test/regress/expected/vacuum.out +++ b/src/test/regress/expected/vacuum.out @@ -686,3 +686,74 @@ RESET ROLE; DROP TABLE vacowned; DROP TABLE vacowned_parted; DROP ROLE regress_vacuum; +-- TRUNCATE option with VACUUM of more than 1 relation. +CREATE TABLE vac_truncate_on_toast_off(i int, j text) WITH + (autovacuum_enabled=false, vacuum_truncate=true, toast.vacuum_truncate=false); +CREATE TABLE vac_truncate_off_toast_on(i int, j text) WITH + (autovacuum_enabled=false, vacuum_truncate=false, toast.vacuum_truncate=true); +-- EXTERNAL to force data on TOAST table, uncompressed. +ALTER TABLE vac_truncate_on_toast_off ALTER COLUMN j SET STORAGE EXTERNAL; +ALTER TABLE vac_truncate_off_toast_on ALTER COLUMN j SET STORAGE EXTERNAL; +INSERT INTO vac_truncate_on_toast_off SELECT generate_series(1, 1000), NULL; +INSERT INTO vac_truncate_on_toast_off SELECT generate_series(1000, 1010), repeat('1234567890', 1000); +INSERT INTO vac_truncate_off_toast_on SELECT generate_series(1, 1000), NULL; +INSERT INTO vac_truncate_off_toast_on SELECT generate_series(1000, 1010), repeat('1234567890', 1000); +SELECT pg_relation_size('vac_truncate_on_toast_off') > 0 AS has_data; + has_data +---------- + t +(1 row) + +SELECT pg_relation_size(reltoastrelid) > 0 AS has_data FROM pg_class + WHERE relname = 'vac_truncate_on_toast_off'; + has_data +---------- + t +(1 row) + +SELECT pg_relation_size('vac_truncate_off_toast_on') > 0 AS has_data; + has_data +---------- + t +(1 row) + +SELECT pg_relation_size(reltoastrelid) > 0 AS has_data FROM pg_class + WHERE relname = 'vac_truncate_off_toast_on'; + has_data +---------- + t +(1 row) + +DELETE FROM vac_truncate_on_toast_off; +DELETE FROM vac_truncate_off_toast_on; +-- TRUNCATE options are retrieved from their respective relations. +-- Do an aggressive VACUUM to prevent page-skipping. +VACUUM (FREEZE, ANALYZE) vac_truncate_on_toast_off, vac_truncate_off_toast_on; +SELECT pg_relation_size('vac_truncate_on_toast_off') > 0 AS has_data; + has_data +---------- + f +(1 row) + +SELECT pg_relation_size(reltoastrelid) > 0 AS has_data FROM pg_class + WHERE relname = 'vac_truncate_on_toast_off'; + has_data +---------- + t +(1 row) + +SELECT pg_relation_size('vac_truncate_off_toast_on') > 0 AS has_data; + has_data +---------- + t +(1 row) + +SELECT pg_relation_size(reltoastrelid) > 0 AS has_data FROM pg_class + WHERE relname = 'vac_truncate_off_toast_on'; + has_data +---------- + f +(1 row) + +DROP TABLE vac_truncate_on_toast_off; +DROP TABLE vac_truncate_off_toast_on; diff --git a/src/test/regress/sql/vacuum.sql b/src/test/regress/sql/vacuum.sql index a72bdb5b619d..234753f4e1a1 100644 --- a/src/test/regress/sql/vacuum.sql +++ b/src/test/regress/sql/vacuum.sql @@ -495,3 +495,35 @@ RESET ROLE; DROP TABLE vacowned; DROP TABLE vacowned_parted; DROP ROLE regress_vacuum; + +-- TRUNCATE option with VACUUM of more than 1 relation. +CREATE TABLE vac_truncate_on_toast_off(i int, j text) WITH + (autovacuum_enabled=false, vacuum_truncate=true, toast.vacuum_truncate=false); +CREATE TABLE vac_truncate_off_toast_on(i int, j text) WITH + (autovacuum_enabled=false, vacuum_truncate=false, toast.vacuum_truncate=true); +-- EXTERNAL to force data on TOAST table, uncompressed. +ALTER TABLE vac_truncate_on_toast_off ALTER COLUMN j SET STORAGE EXTERNAL; +ALTER TABLE vac_truncate_off_toast_on ALTER COLUMN j SET STORAGE EXTERNAL; +INSERT INTO vac_truncate_on_toast_off SELECT generate_series(1, 1000), NULL; +INSERT INTO vac_truncate_on_toast_off SELECT generate_series(1000, 1010), repeat('1234567890', 1000); +INSERT INTO vac_truncate_off_toast_on SELECT generate_series(1, 1000), NULL; +INSERT INTO vac_truncate_off_toast_on SELECT generate_series(1000, 1010), repeat('1234567890', 1000); +SELECT pg_relation_size('vac_truncate_on_toast_off') > 0 AS has_data; +SELECT pg_relation_size(reltoastrelid) > 0 AS has_data FROM pg_class + WHERE relname = 'vac_truncate_on_toast_off'; +SELECT pg_relation_size('vac_truncate_off_toast_on') > 0 AS has_data; +SELECT pg_relation_size(reltoastrelid) > 0 AS has_data FROM pg_class + WHERE relname = 'vac_truncate_off_toast_on'; +DELETE FROM vac_truncate_on_toast_off; +DELETE FROM vac_truncate_off_toast_on; +-- TRUNCATE options are retrieved from their respective relations. +-- Do an aggressive VACUUM to prevent page-skipping. +VACUUM (FREEZE, ANALYZE) vac_truncate_on_toast_off, vac_truncate_off_toast_on; +SELECT pg_relation_size('vac_truncate_on_toast_off') > 0 AS has_data; +SELECT pg_relation_size(reltoastrelid) > 0 AS has_data FROM pg_class + WHERE relname = 'vac_truncate_on_toast_off'; +SELECT pg_relation_size('vac_truncate_off_toast_on') > 0 AS has_data; +SELECT pg_relation_size(reltoastrelid) > 0 AS has_data FROM pg_class + WHERE relname = 'vac_truncate_off_toast_on'; +DROP TABLE vac_truncate_on_toast_off; +DROP TABLE vac_truncate_off_toast_on; diff --git a/contrib/pgstattuple/expected/pgstattuple.out b/contrib/pgstattuple/expected/pgstattuple.out index 9176dc98b6a9..f0bdf7717e30 100644 --- a/contrib/pgstattuple/expected/pgstattuple.out +++ b/contrib/pgstattuple/expected/pgstattuple.out @@ -303,3 +303,50 @@ drop view test_view; drop foreign table test_foreign_table; drop server dummy_server; drop foreign data wrapper dummy; +-- INDEX_CLEANUP option with VACUUM of more than 1 relation. +CREATE TABLE vac_index_cleanup_on_toast_off(i int primary key, j text) WITH + (autovacuum_enabled=false, vacuum_index_cleanup=true, toast.vacuum_index_cleanup=false); +CREATE TABLE vac_index_cleanup_off_toast_on(i int primary key, j text) + WITH (autovacuum_enabled=false, vacuum_index_cleanup=false, toast.vacuum_index_cleanup=true); +-- EXTERNAL to force data on TOAST table, uncompressed. +ALTER TABLE vac_index_cleanup_on_toast_off ALTER COLUMN j SET STORAGE EXTERNAL; +ALTER TABLE vac_index_cleanup_off_toast_on ALTER COLUMN j SET STORAGE EXTERNAL; +INSERT INTO vac_index_cleanup_on_toast_off SELECT generate_series(1, 1000), NULL; +INSERT INTO vac_index_cleanup_on_toast_off SELECT generate_series(1001, 1010), repeat('1234567890', 10000); +INSERT INTO vac_index_cleanup_off_toast_on SELECT generate_series(1, 1000), NULL; +INSERT INTO vac_index_cleanup_off_toast_on SELECT generate_series(1001, 1010), repeat('1234567890', 10000); +DELETE FROM vac_index_cleanup_on_toast_off; +DELETE FROM vac_index_cleanup_off_toast_on; +-- Do an aggressive VACUUM to prevent page-skipping +VACUUM (FREEZE, ANALYZE) vac_index_cleanup_on_toast_off, vac_index_cleanup_off_toast_on; +-- Check cleanup state of main table indexes +SELECT deleted_pages > 0 AS has_del_pages FROM pgstatindex('vac_index_cleanup_off_toast_on_pkey'); + has_del_pages +--------------- + f +(1 row) + +SELECT deleted_pages > 0 AS has_del_pages FROM pgstatindex('vac_index_cleanup_on_toast_off_pkey'); + has_del_pages +--------------- + t +(1 row) + +-- Check cleanup state of TOAST indexes +WITH index_data AS ( + SELECT indexrelid::regclass AS indname, c.relname AS tabname FROM pg_class AS c, pg_index AS i + WHERE c.reltoastrelid = i.indrelid AND + c.relname IN ('vac_index_cleanup_off_toast_on', + 'vac_index_cleanup_on_toast_off') +) +SELECT tabname, deleted_pages > 0 AS has_del_pages + FROM index_data, pgstatindex(index_data.indname) + ORDER BY tabname COLLATE "C"; + tabname | has_del_pages +--------------------------------+--------------- + vac_index_cleanup_off_toast_on | t + vac_index_cleanup_on_toast_off | f +(2 rows) + +DROP TABLE vac_index_cleanup_on_toast_off; +DROP TABLE vac_index_cleanup_off_toast_on; diff --git a/contrib/pgstattuple/sql/pgstattuple.sql b/contrib/pgstattuple/sql/pgstattuple.sql index 7e72c567a064..9a0d284f9b14 100644 --- a/contrib/pgstattuple/sql/pgstattuple.sql +++ b/contrib/pgstattuple/sql/pgstattuple.sql @@ -136,3 +136,35 @@ drop view test_view; drop foreign table test_foreign_table; drop server dummy_server; drop foreign data wrapper dummy; + +-- INDEX_CLEANUP option with VACUUM of more than 1 relation. +CREATE TABLE vac_index_cleanup_on_toast_off(i int primary key, j text) WITH + (autovacuum_enabled=false, vacuum_index_cleanup=true, toast.vacuum_index_cleanup=false); +CREATE TABLE vac_index_cleanup_off_toast_on(i int primary key, j text) + WITH (autovacuum_enabled=false, vacuum_index_cleanup=false, toast.vacuum_index_cleanup=true); +-- EXTERNAL to force data on TOAST table, uncompressed. +ALTER TABLE vac_index_cleanup_on_toast_off ALTER COLUMN j SET STORAGE EXTERNAL; +ALTER TABLE vac_index_cleanup_off_toast_on ALTER COLUMN j SET STORAGE EXTERNAL; +INSERT INTO vac_index_cleanup_on_toast_off SELECT generate_series(1, 1000), NULL; +INSERT INTO vac_index_cleanup_on_toast_off SELECT generate_series(1001, 1010), repeat('1234567890', 10000); +INSERT INTO vac_index_cleanup_off_toast_on SELECT generate_series(1, 1000), NULL; +INSERT INTO vac_index_cleanup_off_toast_on SELECT generate_series(1001, 1010), repeat('1234567890', 10000); +DELETE FROM vac_index_cleanup_on_toast_off; +DELETE FROM vac_index_cleanup_off_toast_on; +-- Do an aggressive VACUUM to prevent page-skipping +VACUUM (FREEZE, ANALYZE) vac_index_cleanup_on_toast_off, vac_index_cleanup_off_toast_on; +-- Check cleanup state of main table indexes +SELECT deleted_pages > 0 AS has_del_pages FROM pgstatindex('vac_index_cleanup_off_toast_on_pkey'); +SELECT deleted_pages > 0 AS has_del_pages FROM pgstatindex('vac_index_cleanup_on_toast_off_pkey'); +-- Check cleanup state of TOAST indexes +WITH index_data AS ( + SELECT indexrelid::regclass AS indname, c.relname AS tabname FROM pg_class AS c, pg_index AS i + WHERE c.reltoastrelid = i.indrelid AND + c.relname IN ('vac_index_cleanup_off_toast_on', + 'vac_index_cleanup_on_toast_off') +) +SELECT tabname, deleted_pages > 0 AS has_del_pages + FROM index_data, pgstatindex(index_data.indname) + ORDER BY tabname COLLATE "C"; +DROP TABLE vac_index_cleanup_on_toast_off; +DROP TABLE vac_index_cleanup_off_toast_on; -- 2.50.0
From cd1273aa60f41c02baef9807c9b7d51dd66cbec7 Mon Sep 17 00:00:00 2001 From: Michael Paquier <mich...@paquier.xyz> Date: Tue, 24 Jun 2025 09:43:38 +0900 Subject: [PATCH v4 2/2] Refactor handling of VacuumParams This adds a couple of const markers, while removing pointer uses of VacuumParams to force the following policy among all the VACUUM-related calls: do not touch the values given by the caller. This patch is for HEAD. Author: Michael P. --- src/include/access/heapam.h | 4 +- src/include/access/tableam.h | 6 +- src/include/commands/vacuum.h | 6 +- src/backend/access/heap/vacuumlazy.c | 44 +++++------ src/backend/commands/analyze.c | 26 +++---- src/backend/commands/cluster.c | 2 +- src/backend/commands/vacuum.c | 106 ++++++++++++--------------- src/backend/postmaster/autovacuum.c | 2 +- 8 files changed, 92 insertions(+), 104 deletions(-) diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h index 3a9424c19c9a..a2bd5a897f87 100644 --- a/src/include/access/heapam.h +++ b/src/include/access/heapam.h @@ -21,6 +21,7 @@ #include "access/skey.h" #include "access/table.h" /* for backward compatibility */ #include "access/tableam.h" +#include "commands/vacuum.h" #include "nodes/lockoptions.h" #include "nodes/primnodes.h" #include "storage/bufpage.h" @@ -396,9 +397,8 @@ extern void log_heap_prune_and_freeze(Relation relation, Buffer buffer, OffsetNumber *unused, int nunused); /* in heap/vacuumlazy.c */ -struct VacuumParams; extern void heap_vacuum_rel(Relation rel, - struct VacuumParams *params, BufferAccessStrategy bstrategy); + const VacuumParams params, BufferAccessStrategy bstrategy); /* in heap/heapam_visibility.c */ extern bool HeapTupleSatisfiesVisibility(HeapTuple htup, Snapshot snapshot, diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index 8713e12cbfb9..1c9e802a6b12 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -20,6 +20,7 @@ #include "access/relscan.h" #include "access/sdir.h" #include "access/xact.h" +#include "commands/vacuum.h" #include "executor/tuptable.h" #include "storage/read_stream.h" #include "utils/rel.h" @@ -36,7 +37,6 @@ extern PGDLLIMPORT bool synchronize_seqscans; struct BulkInsertStateData; struct IndexInfo; struct SampleScanState; -struct VacuumParams; struct ValidateIndexState; /* @@ -645,7 +645,7 @@ typedef struct TableAmRoutine * integrate with autovacuum's scheduling. */ void (*relation_vacuum) (Relation rel, - struct VacuumParams *params, + const VacuumParams params, BufferAccessStrategy bstrategy); /* @@ -1664,7 +1664,7 @@ table_relation_copy_for_cluster(Relation OldTable, Relation NewTable, * routine, even if (for ANALYZE) it is part of the same VACUUM command. */ static inline void -table_relation_vacuum(Relation rel, struct VacuumParams *params, +table_relation_vacuum(Relation rel, const VacuumParams params, BufferAccessStrategy bstrategy) { rel->rd_tableam->relation_vacuum(rel, params, bstrategy); diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h index bc37a80dc74f..14eeccbd7185 100644 --- a/src/include/commands/vacuum.h +++ b/src/include/commands/vacuum.h @@ -336,7 +336,7 @@ extern PGDLLIMPORT int64 parallel_vacuum_worker_delay_ns; /* in commands/vacuum.c */ extern void ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel); -extern void vacuum(List *relations, VacuumParams *params, +extern void vacuum(List *relations, const VacuumParams params, BufferAccessStrategy bstrategy, MemoryContext vac_context, bool isTopLevel); extern void vac_open_indexes(Relation relation, LOCKMODE lockmode, @@ -357,7 +357,7 @@ extern void vac_update_relstats(Relation relation, bool *frozenxid_updated, bool *minmulti_updated, bool in_outer_xact); -extern bool vacuum_get_cutoffs(Relation rel, const VacuumParams *params, +extern bool vacuum_get_cutoffs(Relation rel, const VacuumParams params, struct VacuumCutoffs *cutoffs); extern bool vacuum_xid_failsafe_check(const struct VacuumCutoffs *cutoffs); extern void vac_update_datfrozenxid(void); @@ -398,7 +398,7 @@ extern void parallel_vacuum_main(dsm_segment *seg, shm_toc *toc); /* in commands/analyze.c */ extern void analyze_rel(Oid relid, RangeVar *relation, - VacuumParams *params, List *va_cols, bool in_outer_xact, + const VacuumParams params, List *va_cols, bool in_outer_xact, BufferAccessStrategy bstrategy); extern bool std_typanalyze(VacAttrStats *stats); diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 09416450af96..56d456736da0 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -423,7 +423,7 @@ typedef struct LVSavedErrInfo /* non-export function prototypes */ static void lazy_scan_heap(LVRelState *vacrel); static void heap_vacuum_eager_scan_setup(LVRelState *vacrel, - VacuumParams *params); + const VacuumParams params); static BlockNumber heap_vac_scan_next_block(ReadStream *stream, void *callback_private_data, void *per_buffer_data); @@ -485,7 +485,7 @@ static void restore_vacuum_error_info(LVRelState *vacrel, * vacuum options or for relfrozenxid/relminmxid advancement. */ static void -heap_vacuum_eager_scan_setup(LVRelState *vacrel, VacuumParams *params) +heap_vacuum_eager_scan_setup(LVRelState *vacrel, const VacuumParams params) { uint32 randseed; BlockNumber allvisible; @@ -504,7 +504,7 @@ heap_vacuum_eager_scan_setup(LVRelState *vacrel, VacuumParams *params) vacrel->eager_scan_remaining_successes = 0; /* If eager scanning is explicitly disabled, just return. */ - if (params->max_eager_freeze_failure_rate == 0) + if (params.max_eager_freeze_failure_rate == 0) return; /* @@ -581,11 +581,11 @@ heap_vacuum_eager_scan_setup(LVRelState *vacrel, VacuumParams *params) vacrel->next_eager_scan_region_start = randseed % EAGER_SCAN_REGION_SIZE; - Assert(params->max_eager_freeze_failure_rate > 0 && - params->max_eager_freeze_failure_rate <= 1); + Assert(params.max_eager_freeze_failure_rate > 0 && + params.max_eager_freeze_failure_rate <= 1); vacrel->eager_scan_max_fails_per_region = - params->max_eager_freeze_failure_rate * + params.max_eager_freeze_failure_rate * EAGER_SCAN_REGION_SIZE; /* @@ -612,7 +612,7 @@ heap_vacuum_eager_scan_setup(LVRelState *vacrel, VacuumParams *params) * and locked the relation. */ void -heap_vacuum_rel(Relation rel, VacuumParams *params, +heap_vacuum_rel(Relation rel, const VacuumParams params, BufferAccessStrategy bstrategy) { LVRelState *vacrel; @@ -634,9 +634,9 @@ heap_vacuum_rel(Relation rel, VacuumParams *params, ErrorContextCallback errcallback; char **indnames = NULL; - verbose = (params->options & VACOPT_VERBOSE) != 0; + verbose = (params.options & VACOPT_VERBOSE) != 0; instrument = (verbose || (AmAutoVacuumWorkerProcess() && - params->log_min_duration >= 0)); + params.log_min_duration >= 0)); if (instrument) { pg_rusage_init(&ru0); @@ -699,9 +699,9 @@ heap_vacuum_rel(Relation rel, VacuumParams *params, * The truncate param allows user to avoid attempting relation truncation, * though it can't force truncation to happen. */ - Assert(params->index_cleanup != VACOPTVALUE_UNSPECIFIED); - Assert(params->truncate != VACOPTVALUE_UNSPECIFIED && - params->truncate != VACOPTVALUE_AUTO); + Assert(params.index_cleanup != VACOPTVALUE_UNSPECIFIED); + Assert(params.truncate != VACOPTVALUE_UNSPECIFIED && + params.truncate != VACOPTVALUE_AUTO); /* * While VacuumFailSafeActive is reset to false before calling this, we @@ -711,14 +711,14 @@ heap_vacuum_rel(Relation rel, VacuumParams *params, vacrel->consider_bypass_optimization = true; vacrel->do_index_vacuuming = true; vacrel->do_index_cleanup = true; - vacrel->do_rel_truncate = (params->truncate != VACOPTVALUE_DISABLED); - if (params->index_cleanup == VACOPTVALUE_DISABLED) + vacrel->do_rel_truncate = (params.truncate != VACOPTVALUE_DISABLED); + if (params.index_cleanup == VACOPTVALUE_DISABLED) { /* Force disable index vacuuming up-front */ vacrel->do_index_vacuuming = false; vacrel->do_index_cleanup = false; } - else if (params->index_cleanup == VACOPTVALUE_ENABLED) + else if (params.index_cleanup == VACOPTVALUE_ENABLED) { /* Force index vacuuming. Note that failsafe can still bypass. */ vacrel->consider_bypass_optimization = false; @@ -726,7 +726,7 @@ heap_vacuum_rel(Relation rel, VacuumParams *params, else { /* Default/auto, make all decisions dynamically */ - Assert(params->index_cleanup == VACOPTVALUE_AUTO); + Assert(params.index_cleanup == VACOPTVALUE_AUTO); } /* Initialize page counters explicitly (be tidy) */ @@ -789,7 +789,7 @@ heap_vacuum_rel(Relation rel, VacuumParams *params, */ vacrel->skippedallvis = false; skipwithvm = true; - if (params->options & VACOPT_DISABLE_PAGE_SKIPPING) + if (params.options & VACOPT_DISABLE_PAGE_SKIPPING) { /* * Force aggressive mode, and disable skipping blocks using the @@ -830,7 +830,7 @@ heap_vacuum_rel(Relation rel, VacuumParams *params, * is already dangerously old.) */ lazy_check_wraparound_failsafe(vacrel); - dead_items_alloc(vacrel, params->nworkers); + dead_items_alloc(vacrel, params.nworkers); /* * Call lazy_scan_heap to perform all required heap pruning, index @@ -947,9 +947,9 @@ heap_vacuum_rel(Relation rel, VacuumParams *params, { TimestampTz endtime = GetCurrentTimestamp(); - if (verbose || params->log_min_duration == 0 || + if (verbose || params.log_min_duration == 0 || TimestampDifferenceExceeds(starttime, endtime, - params->log_min_duration)) + params.log_min_duration)) { long secs_dur; int usecs_dur; @@ -984,10 +984,10 @@ heap_vacuum_rel(Relation rel, VacuumParams *params, * Aggressiveness already reported earlier, in dedicated * VACUUM VERBOSE ereport */ - Assert(!params->is_wraparound); + Assert(!params.is_wraparound); msgfmt = _("finished vacuuming \"%s.%s.%s\": index scans: %d\n"); } - else if (params->is_wraparound) + else if (params.is_wraparound) { /* * While it's possible for a VACUUM to be both is_wraparound diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index 4fffb76e5573..7111d5d5334f 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -76,7 +76,7 @@ static BufferAccessStrategy vac_strategy; static void do_analyze_rel(Relation onerel, - VacuumParams *params, List *va_cols, + const VacuumParams params, List *va_cols, AcquireSampleRowsFunc acquirefunc, BlockNumber relpages, bool inh, bool in_outer_xact, int elevel); static void compute_index_stats(Relation onerel, double totalrows, @@ -107,7 +107,7 @@ static Datum ind_fetch_func(VacAttrStatsP stats, int rownum, bool *isNull); */ void analyze_rel(Oid relid, RangeVar *relation, - VacuumParams *params, List *va_cols, bool in_outer_xact, + const VacuumParams params, List *va_cols, bool in_outer_xact, BufferAccessStrategy bstrategy) { Relation onerel; @@ -116,7 +116,7 @@ analyze_rel(Oid relid, RangeVar *relation, BlockNumber relpages = 0; /* Select logging level */ - if (params->options & VACOPT_VERBOSE) + if (params.options & VACOPT_VERBOSE) elevel = INFO; else elevel = DEBUG2; @@ -138,8 +138,8 @@ analyze_rel(Oid relid, RangeVar *relation, * * Make sure to generate only logs for ANALYZE in this case. */ - onerel = vacuum_open_relation(relid, relation, params->options & ~(VACOPT_VACUUM), - params->log_min_duration >= 0, + onerel = vacuum_open_relation(relid, relation, params.options & ~(VACOPT_VACUUM), + params.log_min_duration >= 0, ShareUpdateExclusiveLock); /* leave if relation could not be opened or locked */ @@ -155,7 +155,7 @@ analyze_rel(Oid relid, RangeVar *relation, */ if (!vacuum_is_permitted_for_relation(RelationGetRelid(onerel), onerel->rd_rel, - params->options & ~VACOPT_VACUUM)) + params.options & ~VACOPT_VACUUM)) { relation_close(onerel, ShareUpdateExclusiveLock); return; @@ -227,7 +227,7 @@ analyze_rel(Oid relid, RangeVar *relation, else { /* No need for a WARNING if we already complained during VACUUM */ - if (!(params->options & VACOPT_VACUUM)) + if (!(params.options & VACOPT_VACUUM)) ereport(WARNING, (errmsg("skipping \"%s\" --- cannot analyze non-tables or special system tables", RelationGetRelationName(onerel)))); @@ -275,7 +275,7 @@ analyze_rel(Oid relid, RangeVar *relation, * appropriate acquirefunc for each child table. */ static void -do_analyze_rel(Relation onerel, VacuumParams *params, +do_analyze_rel(Relation onerel, const VacuumParams params, List *va_cols, AcquireSampleRowsFunc acquirefunc, BlockNumber relpages, bool inh, bool in_outer_xact, int elevel) @@ -309,9 +309,9 @@ do_analyze_rel(Relation onerel, VacuumParams *params, PgStat_Counter startreadtime = 0; PgStat_Counter startwritetime = 0; - verbose = (params->options & VACOPT_VERBOSE) != 0; + verbose = (params.options & VACOPT_VERBOSE) != 0; instrument = (verbose || (AmAutoVacuumWorkerProcess() && - params->log_min_duration >= 0)); + params.log_min_duration >= 0)); if (inh) ereport(elevel, (errmsg("analyzing \"%s.%s\" inheritance tree", @@ -706,7 +706,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params, * amvacuumcleanup() when called in ANALYZE-only mode. The only exception * among core index AMs is GIN/ginvacuumcleanup(). */ - if (!(params->options & VACOPT_VACUUM)) + if (!(params.options & VACOPT_VACUUM)) { for (ind = 0; ind < nindexes; ind++) { @@ -736,9 +736,9 @@ do_analyze_rel(Relation onerel, VacuumParams *params, { TimestampTz endtime = GetCurrentTimestamp(); - if (verbose || params->log_min_duration == 0 || + if (verbose || params.log_min_duration == 0 || TimestampDifferenceExceeds(starttime, endtime, - params->log_min_duration)) + params.log_min_duration)) { long delay_in_ms; WalUsage walusage; diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 54a08e4102e1..b55221d44cd0 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -917,7 +917,7 @@ copy_table_data(Relation NewHeap, Relation OldHeap, Relation OldIndex, bool verb * not to be aggressive about this. */ memset(¶ms, 0, sizeof(VacuumParams)); - vacuum_get_cutoffs(OldHeap, ¶ms, &cutoffs); + vacuum_get_cutoffs(OldHeap, params, &cutoffs); /* * FreezeXid will become the table's new relfrozenxid, and that mustn't go diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index a43f090ee178..011239588c7c 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -123,7 +123,7 @@ static void vac_truncate_clog(TransactionId frozenXID, MultiXactId minMulti, TransactionId lastSaneFrozenXid, MultiXactId lastSaneMinMulti); -static bool vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params, +static bool vacuum_rel(Oid relid, RangeVar *relation, VacuumParams params, BufferAccessStrategy bstrategy); static double compute_parallel_delay(void); static VacOptValue get_vacoptval_from_boolean(DefElem *def); @@ -464,7 +464,7 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) } /* Now go through the common routine */ - vacuum(vacstmt->rels, ¶ms, bstrategy, vac_context, isTopLevel); + vacuum(vacstmt->rels, params, bstrategy, vac_context, isTopLevel); /* Finally, clean up the vacuum memory context */ MemoryContextDelete(vac_context); @@ -493,7 +493,7 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) * memory context that will not disappear at transaction commit. */ void -vacuum(List *relations, VacuumParams *params, BufferAccessStrategy bstrategy, +vacuum(List *relations, const VacuumParams params, BufferAccessStrategy bstrategy, MemoryContext vac_context, bool isTopLevel) { static bool in_vacuum = false; @@ -502,9 +502,7 @@ vacuum(List *relations, VacuumParams *params, BufferAccessStrategy bstrategy, volatile bool in_outer_xact, use_own_xacts; - Assert(params != NULL); - - stmttype = (params->options & VACOPT_VACUUM) ? "VACUUM" : "ANALYZE"; + stmttype = (params.options & VACOPT_VACUUM) ? "VACUUM" : "ANALYZE"; /* * We cannot run VACUUM inside a user transaction block; if we were inside @@ -514,7 +512,7 @@ vacuum(List *relations, VacuumParams *params, BufferAccessStrategy bstrategy, * * ANALYZE (without VACUUM) can run either way. */ - if (params->options & VACOPT_VACUUM) + if (params.options & VACOPT_VACUUM) { PreventInTransactionBlock(isTopLevel, stmttype); in_outer_xact = false; @@ -537,7 +535,7 @@ vacuum(List *relations, VacuumParams *params, BufferAccessStrategy bstrategy, * Build list of relation(s) to process, putting any new data in * vac_context for safekeeping. */ - if (params->options & VACOPT_ONLY_DATABASE_STATS) + if (params.options & VACOPT_ONLY_DATABASE_STATS) { /* We don't process any tables in this case */ Assert(relations == NIL); @@ -553,7 +551,7 @@ vacuum(List *relations, VacuumParams *params, BufferAccessStrategy bstrategy, List *sublist; MemoryContext old_context; - sublist = expand_vacuum_rel(vrel, vac_context, params->options); + sublist = expand_vacuum_rel(vrel, vac_context, params.options); old_context = MemoryContextSwitchTo(vac_context); newrels = list_concat(newrels, sublist); MemoryContextSwitchTo(old_context); @@ -561,7 +559,7 @@ vacuum(List *relations, VacuumParams *params, BufferAccessStrategy bstrategy, relations = newrels; } else - relations = get_all_vacuum_rels(vac_context, params->options); + relations = get_all_vacuum_rels(vac_context, params.options); /* * Decide whether we need to start/commit our own transactions. @@ -577,11 +575,11 @@ vacuum(List *relations, VacuumParams *params, BufferAccessStrategy bstrategy, * transaction block, and also in an autovacuum worker, use own * transactions so we can release locks sooner. */ - if (params->options & VACOPT_VACUUM) + if (params.options & VACOPT_VACUUM) use_own_xacts = true; else { - Assert(params->options & VACOPT_ANALYZE); + Assert(params.options & VACOPT_ANALYZE); if (AmAutoVacuumWorkerProcess()) use_own_xacts = true; else if (in_outer_xact) @@ -632,21 +630,13 @@ vacuum(List *relations, VacuumParams *params, BufferAccessStrategy bstrategy, { VacuumRelation *vrel = lfirst_node(VacuumRelation, cur); - if (params->options & VACOPT_VACUUM) + if (params.options & VACOPT_VACUUM) { - VacuumParams params_copy; - - /* - * vacuum_rel() scribbles on the parameters, so give it a copy - * to avoid affecting other relations. - */ - memcpy(¶ms_copy, params, sizeof(VacuumParams)); - - if (!vacuum_rel(vrel->oid, vrel->relation, ¶ms_copy, bstrategy)) + if (!vacuum_rel(vrel->oid, vrel->relation, params, bstrategy)) continue; } - if (params->options & VACOPT_ANALYZE) + if (params.options & VACOPT_ANALYZE) { /* * If using separate xacts, start one for analyze. Otherwise, @@ -710,8 +700,8 @@ vacuum(List *relations, VacuumParams *params, BufferAccessStrategy bstrategy, StartTransactionCommand(); } - if ((params->options & VACOPT_VACUUM) && - !(params->options & VACOPT_SKIP_DATABASE_STATS)) + if ((params.options & VACOPT_VACUUM) && + !(params.options & VACOPT_SKIP_DATABASE_STATS)) { /* * Update pg_database.datfrozenxid, and truncate pg_xact if possible. @@ -1109,7 +1099,7 @@ get_all_vacuum_rels(MemoryContext vac_context, int options) * minimum). */ bool -vacuum_get_cutoffs(Relation rel, const VacuumParams *params, +vacuum_get_cutoffs(Relation rel, const VacuumParams params, struct VacuumCutoffs *cutoffs) { int freeze_min_age, @@ -1125,10 +1115,10 @@ vacuum_get_cutoffs(Relation rel, const VacuumParams *params, aggressiveMXIDCutoff; /* Use mutable copies of freeze age parameters */ - freeze_min_age = params->freeze_min_age; - multixact_freeze_min_age = params->multixact_freeze_min_age; - freeze_table_age = params->freeze_table_age; - multixact_freeze_table_age = params->multixact_freeze_table_age; + freeze_min_age = params.freeze_min_age; + multixact_freeze_min_age = params.multixact_freeze_min_age; + freeze_table_age = params.freeze_table_age; + multixact_freeze_table_age = params.multixact_freeze_table_age; /* Set pg_class fields in cutoffs */ cutoffs->relfrozenxid = rel->rd_rel->relfrozenxid; @@ -2005,7 +1995,7 @@ vac_truncate_clog(TransactionId frozenXID, * At entry and exit, we are not inside a transaction. */ static bool -vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params, +vacuum_rel(Oid relid, RangeVar *relation, VacuumParams params, BufferAccessStrategy bstrategy) { LOCKMODE lmode; @@ -2018,18 +2008,16 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params, int save_nestlevel; VacuumParams toast_vacuum_params; - Assert(params != NULL); - /* * This function scribbles on the parameters, so make a copy early to * avoid affecting the TOAST table (if we do end up recursing to it). */ - memcpy(&toast_vacuum_params, params, sizeof(VacuumParams)); + memcpy(&toast_vacuum_params, ¶ms, sizeof(VacuumParams)); /* Begin a transaction for vacuuming this relation */ StartTransactionCommand(); - if (!(params->options & VACOPT_FULL)) + if (!(params.options & VACOPT_FULL)) { /* * In lazy vacuum, we can set the PROC_IN_VACUUM flag, which lets @@ -2055,7 +2043,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params, */ LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE); MyProc->statusFlags |= PROC_IN_VACUUM; - if (params->is_wraparound) + if (params.is_wraparound) MyProc->statusFlags |= PROC_VACUUM_FOR_WRAPAROUND; ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags; LWLockRelease(ProcArrayLock); @@ -2079,12 +2067,12 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params, * vacuum, but just ShareUpdateExclusiveLock for concurrent vacuum. Either * way, we can be sure that no other backend is vacuuming the same table. */ - lmode = (params->options & VACOPT_FULL) ? + lmode = (params.options & VACOPT_FULL) ? AccessExclusiveLock : ShareUpdateExclusiveLock; /* open the relation and get the appropriate lock on it */ - rel = vacuum_open_relation(relid, relation, params->options, - params->log_min_duration >= 0, lmode); + rel = vacuum_open_relation(relid, relation, params.options, + params.log_min_duration >= 0, lmode); /* leave if relation could not be opened or locked */ if (!rel) @@ -2099,8 +2087,8 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params, * This is only safe to do because we hold a session lock on the main * relation that prevents concurrent deletion. */ - if (OidIsValid(params->toast_parent)) - priv_relid = params->toast_parent; + if (OidIsValid(params.toast_parent)) + priv_relid = params.toast_parent; else priv_relid = RelationGetRelid(rel); @@ -2113,7 +2101,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params, */ if (!vacuum_is_permitted_for_relation(priv_relid, rel->rd_rel, - params->options & ~VACOPT_ANALYZE)) + params.options & ~VACOPT_ANALYZE)) { relation_close(rel, lmode); PopActiveSnapshot(); @@ -2184,7 +2172,7 @@ 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 */ - if (params->index_cleanup == VACOPTVALUE_UNSPECIFIED) + if (params.index_cleanup == VACOPTVALUE_UNSPECIFIED) { StdRdOptIndexCleanup vacuum_index_cleanup; @@ -2195,14 +2183,14 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params, ((StdRdOptions *) rel->rd_options)->vacuum_index_cleanup; if (vacuum_index_cleanup == STDRD_OPTION_VACUUM_INDEX_CLEANUP_AUTO) - params->index_cleanup = VACOPTVALUE_AUTO; + params.index_cleanup = VACOPTVALUE_AUTO; else if (vacuum_index_cleanup == STDRD_OPTION_VACUUM_INDEX_CLEANUP_ON) - params->index_cleanup = VACOPTVALUE_ENABLED; + params.index_cleanup = VACOPTVALUE_ENABLED; else { Assert(vacuum_index_cleanup == STDRD_OPTION_VACUUM_INDEX_CLEANUP_OFF); - params->index_cleanup = VACOPTVALUE_DISABLED; + params.index_cleanup = VACOPTVALUE_DISABLED; } } @@ -2212,28 +2200,28 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params, */ if (rel->rd_options != NULL && ((StdRdOptions *) rel->rd_options)->vacuum_max_eager_freeze_failure_rate >= 0) - params->max_eager_freeze_failure_rate = + params.max_eager_freeze_failure_rate = ((StdRdOptions *) rel->rd_options)->vacuum_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 */ - if (params->truncate == VACOPTVALUE_UNSPECIFIED) + if (params.truncate == VACOPTVALUE_UNSPECIFIED) { StdRdOptions *opts = (StdRdOptions *) rel->rd_options; if (opts && opts->vacuum_truncate_set) { if (opts->vacuum_truncate) - params->truncate = VACOPTVALUE_ENABLED; + params.truncate = VACOPTVALUE_ENABLED; else - params->truncate = VACOPTVALUE_DISABLED; + params.truncate = VACOPTVALUE_DISABLED; } else if (vacuum_truncate) - params->truncate = VACOPTVALUE_ENABLED; + params.truncate = VACOPTVALUE_ENABLED; else - params->truncate = VACOPTVALUE_DISABLED; + params.truncate = VACOPTVALUE_DISABLED; } /* @@ -2242,9 +2230,9 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params, * automatically rebuilt by cluster_rel so we shouldn't recurse to it, * unless PROCESS_MAIN is disabled. */ - if ((params->options & VACOPT_PROCESS_TOAST) != 0 && - ((params->options & VACOPT_FULL) == 0 || - (params->options & VACOPT_PROCESS_MAIN) == 0)) + if ((params.options & VACOPT_PROCESS_TOAST) != 0 && + ((params.options & VACOPT_FULL) == 0 || + (params.options & VACOPT_PROCESS_MAIN) == 0)) toast_relid = rel->rd_rel->reltoastrelid; else toast_relid = InvalidOid; @@ -2267,16 +2255,16 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params, * table is required (e.g., PROCESS_TOAST is set), we force PROCESS_MAIN * to be set when we recurse to the TOAST table. */ - if (params->options & VACOPT_PROCESS_MAIN) + if (params.options & VACOPT_PROCESS_MAIN) { /* * Do the actual work --- either FULL or "lazy" vacuum */ - if (params->options & VACOPT_FULL) + if (params.options & VACOPT_FULL) { ClusterParams cluster_params = {0}; - if ((params->options & VACOPT_VERBOSE) != 0) + if ((params.options & VACOPT_VERBOSE) != 0) cluster_params.options |= CLUOPT_VERBOSE; /* VACUUM FULL is now a variant of CLUSTER; see cluster.c */ @@ -2323,7 +2311,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params, toast_vacuum_params.options |= VACOPT_PROCESS_MAIN; toast_vacuum_params.toast_parent = relid; - vacuum_rel(toast_relid, NULL, &toast_vacuum_params, bstrategy); + vacuum_rel(toast_relid, NULL, toast_vacuum_params, bstrategy); } /* diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 451fb90a610a..9474095f271a 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -3190,7 +3190,7 @@ autovacuum_do_vac_analyze(autovac_table *tab, BufferAccessStrategy bstrategy) rel_list = list_make1(rel); MemoryContextSwitchTo(old_context); - vacuum(rel_list, &tab->at_params, bstrategy, vac_context, true); + vacuum(rel_list, tab->at_params, bstrategy, vac_context, true); MemoryContextDelete(vac_context); } -- 2.50.0
signature.asc
Description: PGP signature