public inbox for [email protected]
help / color / mirror / Atom feedFrom: shihao zhong <[email protected]>
To: PostgreSQL-development <[email protected]>
Subject: Fixes inconsistent behavior in vacuum when it processes multiple relations
Date: Wed, 18 Jun 2025 11:15:31 -0400
Message-ID: <CAGRkXqTo+aK=GTy5pSc-9cy8H2F2TJvcrZ-zXEiNJj93np1UUw@mail.gmail.com> (raw)
Hi team,
One of our customers recently encountered an issue with PostgreSQL's
pg_cron and VACUUM ANALYZE. They had configured a table with
vacuum_truncate=false to prevent exclusive lock contention, assuming
this would apply globally. However, VACUUM ANALYZE executed across the
entire database doesn't honor this table-specific setting, though
autovacuum does.
I investigated the code and found a small bug with how we're passing
the VacuumParams pointer.
The call flow is
ExecVacuum -> vacuum -> vacuum_rel
The initial VaccumParams pointer is set in ExecVacuum
In vacuum_rel, this pointer might change because it needs to determine
whether to truncate and perform index_cleanup.
Vacuuming a single table works as expected because the options are
applied at the table level. However, when vacuuming multiple tables,
the second table reuses the modified parameters set by the first
table's vacuum_rel.
We can easy repro that with following SQL and same with index_cleanup:
create table test(id int);
create table test_1(id int) with (vacuum_truncate=false);
insert into test select generate_series(1,1000);
insert into test_1 select generate_series(1,1000);
delete from test;
delete from test_1;
vacuum (analyze) test_1, test;
postgres=# \dt+
List of relations
Schema | Name | Type | Owner | Persistence | Access method |
Size | Description
--------+--------+-------+----------+-------------+---------------+-------+-------------
public | test | table | postgres | permanent | heap | 72 kB |
public | test_1 | table | postgres | permanent | heap | 72 kB |
(2 rows)
I've implemented a fix and included a regression test in the patch.
Thanks,
Shihao
Attachments:
[application/octet-stream] vacuum_tables_options.patch (10.5K, 2-vacuum_tables_options.patch)
download | inline diff:
diff --git a/contrib/pgstattuple/expected/pgstattuple.out b/contrib/pgstattuple/expected/pgstattuple.out
index 9176dc98b6a..646c8c11e6e 100644
--- a/contrib/pgstattuple/expected/pgstattuple.out
+++ b/contrib/pgstattuple/expected/pgstattuple.out
@@ -303,3 +303,19 @@ drop view test_view;
drop foreign table test_foreign_table;
drop server dummy_server;
drop foreign data wrapper dummy;
+-- INDEX_CLEANUP option with vacuum more than 1 relation
+CREATE TABLE vac_index_cleanup_on(i INT PRIMARY KEY) WITH (autovacuum_enabled=false, vacuum_index_cleanup=true);
+INSERT INTO vac_index_cleanup_on SELECT generate_series(1, 1000);
+CREATE TABLE vac_index_cleanup_off(i INT PRIMARY KEY) WITH (autovacuum_enabled=false, vacuum_index_cleanup=false);
+INSERT INTO vac_index_cleanup_off SELECT generate_series(1, 1000);
+DELETE FROM vac_index_cleanup_on;
+DELETE FROM vac_index_cleanup_off;
+VACUUM (ANALYZE) vac_index_cleanup_on, vac_index_cleanup_off;
+SELECT deleted_pages = 0 FROM pgstatindex('vac_index_cleanup_off_pkey');
+ ?column?
+----------
+ t
+(1 row)
+
+DROP TABLE vac_index_cleanup_on;
+DROP TABLE vac_index_cleanup_off;
diff --git a/contrib/pgstattuple/sql/pgstattuple.sql b/contrib/pgstattuple/sql/pgstattuple.sql
index 7e72c567a06..660280ee247 100644
--- a/contrib/pgstattuple/sql/pgstattuple.sql
+++ b/contrib/pgstattuple/sql/pgstattuple.sql
@@ -136,3 +136,15 @@ drop view test_view;
drop foreign table test_foreign_table;
drop server dummy_server;
drop foreign data wrapper dummy;
+
+-- INDEX_CLEANUP option with vacuum more than 1 relation
+CREATE TABLE vac_index_cleanup_on(i INT PRIMARY KEY) WITH (autovacuum_enabled=false, vacuum_index_cleanup=true);
+INSERT INTO vac_index_cleanup_on SELECT generate_series(1, 1000);
+CREATE TABLE vac_index_cleanup_off(i INT PRIMARY KEY) WITH (autovacuum_enabled=false, vacuum_index_cleanup=false);
+INSERT INTO vac_index_cleanup_off SELECT generate_series(1, 1000);
+DELETE FROM vac_index_cleanup_on;
+DELETE FROM vac_index_cleanup_off;
+VACUUM (ANALYZE) vac_index_cleanup_on, vac_index_cleanup_off;
+SELECT deleted_pages = 0 FROM pgstatindex('vac_index_cleanup_off_pkey');
+DROP TABLE vac_index_cleanup_on;
+DROP TABLE vac_index_cleanup_off;
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 33a33bf6b1c..1128dfb7985 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);
@@ -634,7 +634,7 @@ vacuum(List *relations, VacuumParams *params, BufferAccessStrategy bstrategy,
if (params->options & VACOPT_VACUUM)
{
- if (!vacuum_rel(vrel->oid, vrel->relation, params, bstrategy))
+ if (!vacuum_rel(vrel->oid, vrel->relation, *params, bstrategy))
continue;
}
@@ -1997,7 +1997,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;
@@ -2009,12 +2009,10 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
int save_sec_context;
int save_nestlevel;
- Assert(params != NULL);
-
/* 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
@@ -2040,7 +2038,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);
@@ -2064,12 +2062,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)
@@ -2084,8 +2082,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);
@@ -2098,7 +2096,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();
@@ -2169,7 +2167,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;
@@ -2180,14 +2178,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;
}
}
@@ -2227,9 +2225,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;
@@ -2252,12 +2250,12 @@ 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};
@@ -2307,11 +2305,11 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
* 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));
+ memcpy(&toast_vacuum_params, ¶ms, sizeof(VacuumParams));
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/test/regress/expected/vacuum.out b/src/test/regress/expected/vacuum.out
index 0abcc99989e..284b5391b48 100644
--- a/src/test/regress/expected/vacuum.out
+++ b/src/test/regress/expected/vacuum.out
@@ -686,3 +686,31 @@ RESET ROLE;
DROP TABLE vacowned;
DROP TABLE vacowned_parted;
DROP ROLE regress_vacuum;
+-- TRUNCATE option with vacuum more than 1 relation
+CREATE TABLE vac_truncate_on(i INT) WITH (autovacuum_enabled=false, vacuum_truncate=true);
+INSERT INTO vac_truncate_on SELECT generate_series(1, 1000);
+CREATE TABLE vac_truncate_off(i INT) WITH (autovacuum_enabled=false, vacuum_truncate=false);
+INSERT INTO vac_truncate_off SELECT generate_series(1, 1000);
+SELECT pg_relation_size('vac_truncate_on') > 0;
+ ?column?
+----------
+ t
+(1 row)
+
+SELECT pg_relation_size('vac_truncate_off') > 0;
+ ?column?
+----------
+ t
+(1 row)
+
+DELETE FROM vac_truncate_on;
+DELETE FROM vac_truncate_off;
+VACUUM (ANALYZE) vac_truncate_on, vac_truncate_off;
+SELECT pg_relation_size('vac_truncate_off') > 0;
+ ?column?
+----------
+ t
+(1 row)
+
+DROP TABLE vac_truncate_on;
+DROP TABLE vac_truncate_off;
diff --git a/src/test/regress/sql/vacuum.sql b/src/test/regress/sql/vacuum.sql
index a72bdb5b619..95330a5d44a 100644
--- a/src/test/regress/sql/vacuum.sql
+++ b/src/test/regress/sql/vacuum.sql
@@ -495,3 +495,17 @@ RESET ROLE;
DROP TABLE vacowned;
DROP TABLE vacowned_parted;
DROP ROLE regress_vacuum;
+
+-- TRUNCATE option with vacuum more than 1 relation
+CREATE TABLE vac_truncate_on(i INT) WITH (autovacuum_enabled=false, vacuum_truncate=true);
+INSERT INTO vac_truncate_on SELECT generate_series(1, 1000);
+CREATE TABLE vac_truncate_off(i INT) WITH (autovacuum_enabled=false, vacuum_truncate=false);
+INSERT INTO vac_truncate_off SELECT generate_series(1, 1000);
+SELECT pg_relation_size('vac_truncate_on') > 0;
+SELECT pg_relation_size('vac_truncate_off') > 0;
+DELETE FROM vac_truncate_on;
+DELETE FROM vac_truncate_off;
+VACUUM (ANALYZE) vac_truncate_on, vac_truncate_off;
+SELECT pg_relation_size('vac_truncate_off') > 0;
+DROP TABLE vac_truncate_on;
+DROP TABLE vac_truncate_off;
view thread (32+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: Fixes inconsistent behavior in vacuum when it processes multiple relations
In-Reply-To: <CAGRkXqTo+aK=GTy5pSc-9cy8H2F2TJvcrZ-zXEiNJj93np1UUw@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox