From cfa73c9086f737ec103b3caa03175b837c5565cb Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Sat, 25 Mar 2023 12:05:18 -0400 Subject: [PATCH v10 1/3] Make VacuumCostActive failsafe-aware While vacuuming a table in failsafe mode, VacuumCostActive should not be re-enabled. This currently isn't a problem because vacuum cost parameters are only refreshed in between vacuuming tables and failsafe status is reset for every table. In preparation for allowing vacuum cost parameters to be updated more frequently, make vacuum cost status more expressive. VacuumCostActive is now VacuumCostInactive, as it can only be active in one way but it can be inactive in two ways. If performing a failsafe vacuum, the vacuum cost status cannot be enabled and is effectively "locked". If performing a non-failsafe vacuum, the vacuum cost status may be active or inactive. To express this, VacuumCostInactive can be one of three statuses: VACUUM_COST_INACTIVE_AND_LOCKED, VACUUM_COST_ACTIVE_AND_LOCKED, and VACUUM_COST_ACTIVE. VacuumCostInactive is defined as an integer because we do not want non-vacuum code concerning itself with the distinction between the three statuses -- only with whether or not VacuumCostInactive == 0 or not. --- src/backend/access/heap/vacuumlazy.c | 2 +- src/backend/commands/vacuum.c | 24 +++++++++++++++++++++--- src/backend/commands/vacuumparallel.c | 8 ++++++-- src/backend/storage/buffer/bufmgr.c | 8 ++++---- src/backend/utils/init/globals.c | 2 +- src/include/commands/vacuum.h | 8 ++++++++ src/include/miscadmin.h | 3 +-- 7 files changed, 42 insertions(+), 13 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 8f14cf85f3..040a4e931b 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -2637,7 +2637,7 @@ lazy_check_wraparound_failsafe(LVRelState *vacrel) "You might also need to consider other ways for VACUUM to keep up with the allocation of transaction IDs."))); /* Stop applying cost limits from this point on */ - VacuumCostActive = false; + VacuumCostInactive = VACUUM_COST_INACTIVE_AND_LOCKED; VacuumCostBalance = 0; return true; diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index c54360a6a0..eb126f2247 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -491,7 +491,6 @@ vacuum(List *relations, VacuumParams *params, ListCell *cur; in_vacuum = true; - VacuumCostActive = (VacuumCostDelay > 0); VacuumCostBalance = 0; VacuumPageHit = 0; VacuumPageMiss = 0; @@ -507,6 +506,24 @@ vacuum(List *relations, VacuumParams *params, { VacuumRelation *vrel = lfirst_node(VacuumRelation, cur); + /* + * failsafe_active is reset per relation, so we must be sure that + * VacuumCostInactive is set to either VACUUM_COST_INACTIVE or + * VACUUM_COST_INACTIVE_AND_UNLOCKED in between vacuuming + * relations. + */ + VacuumCostInactive = VacuumCostDelay > 0 ? VACUUM_COST_ACTIVE : + VACUUM_COST_INACTIVE_AND_UNLOCKED; + + /* + * We should not have transitioned VacuumCostInactive from + * VACUUM_COST_ACTIVE to VACUUM_COST_INACTIVE_AND_UNLOCKED above, + * as that should have happened when we changed the value of + * VacuumCostDelay. + */ + Assert(VacuumCostInactive == VACUUM_COST_ACTIVE || + VacuumCostBalance == 0); + if (params->options & VACOPT_VACUUM) { if (!vacuum_rel(vrel->oid, vrel->relation, params, false)) @@ -549,7 +566,8 @@ vacuum(List *relations, VacuumParams *params, PG_FINALLY(); { in_vacuum = false; - VacuumCostActive = false; + VacuumCostInactive = VACUUM_COST_INACTIVE_AND_UNLOCKED; + VacuumCostBalance = 0; } PG_END_TRY(); @@ -2215,7 +2233,7 @@ vacuum_delay_point(void) /* Always check for interrupts */ CHECK_FOR_INTERRUPTS(); - if (!VacuumCostActive || InterruptPending) + if (VacuumCostInactive || InterruptPending) return; /* diff --git a/src/backend/commands/vacuumparallel.c b/src/backend/commands/vacuumparallel.c index bcd40c80a1..266bf6bb4c 100644 --- a/src/backend/commands/vacuumparallel.c +++ b/src/backend/commands/vacuumparallel.c @@ -989,8 +989,12 @@ parallel_vacuum_main(dsm_segment *seg, shm_toc *toc) PARALLEL_VACUUM_KEY_DEAD_ITEMS, false); - /* Set cost-based vacuum delay */ - VacuumCostActive = (VacuumCostDelay > 0); + /* + * Set cost-based vacuum delay Parallel vacuum workers will not execute + * failsafe VACUUM. + */ + VacuumCostInactive = VacuumCostDelay > 0 ? VACUUM_COST_ACTIVE : + VACUUM_COST_INACTIVE_AND_UNLOCKED; VacuumCostBalance = 0; VacuumPageHit = 0; VacuumPageMiss = 0; diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 95212a3941..6d3dd26fc7 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -893,7 +893,7 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum, *hit = true; VacuumPageHit++; - if (VacuumCostActive) + if (!VacuumCostInactive) VacuumCostBalance += VacuumCostPageHit; TRACE_POSTGRESQL_BUFFER_READ_DONE(forkNum, blockNum, @@ -1098,7 +1098,7 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum, } VacuumPageMiss++; - if (VacuumCostActive) + if (!VacuumCostInactive) VacuumCostBalance += VacuumCostPageMiss; TRACE_POSTGRESQL_BUFFER_READ_DONE(forkNum, blockNum, @@ -1672,7 +1672,7 @@ MarkBufferDirty(Buffer buffer) { VacuumPageDirty++; pgBufferUsage.shared_blks_dirtied++; - if (VacuumCostActive) + if (!VacuumCostInactive) VacuumCostBalance += VacuumCostPageDirty; } } @@ -4199,7 +4199,7 @@ MarkBufferDirtyHint(Buffer buffer, bool buffer_std) { VacuumPageDirty++; pgBufferUsage.shared_blks_dirtied++; - if (VacuumCostActive) + if (!VacuumCostInactive) VacuumCostBalance += VacuumCostPageDirty; } } diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c index 1b1d814254..608ebb9182 100644 --- a/src/backend/utils/init/globals.c +++ b/src/backend/utils/init/globals.c @@ -150,4 +150,4 @@ int64 VacuumPageMiss = 0; int64 VacuumPageDirty = 0; int VacuumCostBalance = 0; /* working state for vacuum */ -bool VacuumCostActive = false; +int VacuumCostInactive = 1; diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h index bdfd96cfec..5c3e250b06 100644 --- a/src/include/commands/vacuum.h +++ b/src/include/commands/vacuum.h @@ -302,6 +302,14 @@ extern PGDLLIMPORT int vacuum_failsafe_age; extern PGDLLIMPORT int vacuum_multixact_failsafe_age; /* Variables for cost-based parallel vacuum */ + +typedef enum VacuumCostStatus +{ + VACUUM_COST_INACTIVE_AND_LOCKED = -1, + VACUUM_COST_ACTIVE = 0, + VACUUM_COST_INACTIVE_AND_UNLOCKED = 1, +} VacuumCostStatus; + extern PGDLLIMPORT pg_atomic_uint32 *VacuumSharedCostBalance; extern PGDLLIMPORT pg_atomic_uint32 *VacuumActiveNWorkers; extern PGDLLIMPORT int VacuumCostBalanceLocal; diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h index 06a86f9ac1..33e22733ae 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -274,8 +274,7 @@ extern PGDLLIMPORT int64 VacuumPageMiss; extern PGDLLIMPORT int64 VacuumPageDirty; extern PGDLLIMPORT int VacuumCostBalance; -extern PGDLLIMPORT bool VacuumCostActive; - +extern PGDLLIMPORT int VacuumCostInactive; /* in tcop/postgres.c */ -- 2.37.2