From 85fdedf8d4f4183e7eb8b02b6d55a482730a4718 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Thu, 24 Sep 2026 18:42:47 +0000 Subject: [PATCH v3] Refresh autovacuum cost parameters while waiting for parallel workers An autovacuum leader publishes its cost-based delay parameters to its parallel vacuum workers at its cost delay points. Once it has finished its own share of the indexes and is only waiting for its workers, it has no delay points left, so it stops publishing for as long as that wait lasts, which can be as long as the largest index takes. Two kinds of changes were lost for that whole time. 1. A config reload. The signal wakes the leader, but the wait does not act on it, so the reload stays pending until the wait ends. 2. A change in the number of autovacuum workers sharing the cost limit. Nothing signals that at all, and a waiting leader never re-reads the count because it no longer naps. Fix the first by refreshing and publishing the parameters from the wait itself, on every wakeup, when the process is an autovacuum worker. Fix the second by waking the workers that share the limit whenever the count changes, so a waiting leader picks it up the same way. A worker that is busy vacuuming ignores the extra wakeup. Reported-by: Nikolay Samokhvalov Discussion: https://postgr.es/m/CAM527d-GL%3DJp2EJXBSnVBGPK-4XEZwWof5Cv8P0hghS_og6oAg%40mail.gmail.com --- src/backend/access/transam/parallel.c | 8 + src/backend/commands/vacuumparallel.c | 44 ++++++ src/backend/postmaster/autovacuum.c | 19 +++ src/include/commands/vacuum.h | 1 + .../t/001_parallel_autovacuum.pl | 142 ++++++++++++++++++ 5 files changed, 214 insertions(+) diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c index e1806a9a28a..89e45bdeb9c 100644 --- a/src/backend/access/transam/parallel.c +++ b/src/backend/access/transam/parallel.c @@ -812,6 +812,14 @@ WaitForParallelWorkersToFinish(ParallelContext *pcxt) */ CHECK_FOR_INTERRUPTS(); + /* + * An autovacuum leader publishes cost parameter changes to its + * parallel workers at its cost delay points, which it no longer + * reaches while waiting here. Do it here instead, on every wakeup. + */ + if (AmAutoVacuumWorkerProcess()) + parallel_vacuum_refresh_cost_params(); + for (i = 0; i < pcxt->nworkers_launched; ++i) { /* diff --git a/src/backend/commands/vacuumparallel.c b/src/backend/commands/vacuumparallel.c index 767d162e578..07720609aea 100644 --- a/src/backend/commands/vacuumparallel.c +++ b/src/backend/commands/vacuumparallel.c @@ -43,9 +43,11 @@ #include "executor/instrument.h" #include "optimizer/paths.h" #include "pgstat.h" +#include "postmaster/interrupt.h" #include "storage/bufmgr.h" #include "storage/proc.h" #include "tcop/tcopprot.h" +#include "utils/injection_point.h" #include "utils/lsyscache.h" #include "utils/rel.h" @@ -725,6 +727,42 @@ parallel_vacuum_propagate_shared_delay_params(void) pg_atomic_fetch_add_u32(&pv_shared_cost_params->generation, 1); } +/* + * Refresh the leader's cost-based vacuum delay parameters and propagate them + * to its parallel vacuum workers. + * + * The leader normally does this at its cost delay points, which it no longer + * reaches once it is only waiting for its workers to finish. It calls this + * from that wait instead, on every wakeup, to pick up a config reload and a + * change in the number of autovacuum workers sharing the cost limit. Both of + * those wake the leader, the first by signal and the second when the count is + * recalculated. + */ +void +parallel_vacuum_refresh_cost_params(void) +{ + Assert(AmAutoVacuumWorkerProcess()); + + /* + * Quick return if the leader process is not sharing the delay parameters. + */ + if (pv_shared_cost_params == NULL) + return; + + if (ConfigReloadPending) + { + ConfigReloadPending = false; + ProcessConfigFile(PGC_SIGHUP); + } + + /* + * Recompute from the (possibly reloaded) GUCs and the current number of + * workers sharing the cost limit, then publish any change. + */ + VacuumUpdateCosts(); + parallel_vacuum_propagate_shared_delay_params(); +} + /* * Compute the number of parallel worker processes to request. Both index * vacuum and index cleanup can be executed with parallel workers. @@ -929,6 +967,9 @@ parallel_vacuum_process_all_indexes(ParallelVacuumState *pvs, int num_index_scan /* Vacuum the indexes that can be processed by only leader process */ parallel_vacuum_process_unsafe_indexes(pvs); + if (pvs->shared->is_autovacuum) + INJECTION_POINT("parallel-autovacuum-leader-before-index", NULL); + /* * Join as a parallel worker. The leader vacuums alone processes all * parallel-safe indexes in the case where no workers are launched. @@ -1010,6 +1051,9 @@ parallel_vacuum_process_safe_indexes(ParallelVacuumState *pvs) if (!indstats->parallel_workers_can_process) continue; + if (IsParallelWorker() && pvs->shared->is_autovacuum) + INJECTION_POINT("parallel-autovacuum-worker-before-index", NULL); + /* Do vacuum or cleanup of the index */ parallel_vacuum_process_one_index(pvs, pvs->indrels[idx], indstats); } diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 60ebe828900..d45f361ac0d 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -1814,8 +1814,27 @@ autovac_recalculate_workers_for_balance(void) } if (nworkers_for_balance != orig_nworkers_for_balance) + { pg_atomic_write_u32(&AutoVacuumShmem->av_nworkersForBalance, nworkers_for_balance); + + /* + * Wake up the workers that share the limit. One that is vacuuming + * picks up the new count on its next nap, but one that is only + * waiting for its parallel vacuum workers never naps, and nothing + * else wakes it while they keep running at the old share. + */ + dlist_foreach(iter, &AutoVacuumShmem->av_runningWorkers) + { + WorkerInfo worker = dlist_container(WorkerInfoData, wi_links, iter.cur); + + if (worker->wi_proc == NULL || + pg_atomic_unlocked_test_flag(&worker->wi_dobalance)) + continue; + + SetLatch(&worker->wi_proc->procLatch); + } + } } /* diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h index 6e3c912bf5c..89fa1fcd261 100644 --- a/src/include/commands/vacuum.h +++ b/src/include/commands/vacuum.h @@ -432,6 +432,7 @@ extern void parallel_vacuum_cleanup_all_indexes(ParallelVacuumState *pvs, PVWorkerStats *wstats); extern void parallel_vacuum_update_shared_delay_params(void); extern void parallel_vacuum_propagate_shared_delay_params(void); +extern void parallel_vacuum_refresh_cost_params(void); extern void parallel_vacuum_main(dsm_segment *seg, shm_toc *toc); /* in commands/analyze.c */ diff --git a/src/test/modules/test_autovacuum/t/001_parallel_autovacuum.pl b/src/test/modules/test_autovacuum/t/001_parallel_autovacuum.pl index 33c86bbdc94..10232795e02 100644 --- a/src/test/modules/test_autovacuum/t/001_parallel_autovacuum.pl +++ b/src/test/modules/test_autovacuum/t/001_parallel_autovacuum.pl @@ -247,6 +247,148 @@ my @limits = note("parallel worker cost_limit sequence: @limits"); is($limits[0], '250', 'parallel workers see the rebalanced cost limit'); +# Release the second worker. +$node->safe_psql('postgres', + "SELECT injection_points_wakeup('autovacuum-worker-cost-balanced')"); +$node->safe_psql('postgres', + "SELECT injection_points_detach('autovacuum-worker-cost-balanced')"); + +# Wait for the second worker from test 3 to finish, and go back to a single +# autovacuum worker so that the balance below is known. +$node->poll_query_until( + 'postgres', q{ + SELECT count(*) = 0 FROM pg_stat_activity + WHERE backend_type = 'autovacuum worker' AND datname = 'regress_db2' +}) or die "second autovacuum worker did not finish"; +$node->safe_psql( + 'postgres', qq{ + ALTER SYSTEM SET autovacuum_max_workers = 1; + SELECT pg_reload_conf(); +}); + +my $postgresoid = $node->safe_psql('postgres', + "SELECT oid FROM pg_database WHERE datname = 'postgres'"); +my $testautovacoid = + $node->safe_psql('postgres', "SELECT 'test_autovac'::regclass::oid"); + +# Start a parallel autovacuum on test_autovac and leave it with the leader +# idle in WaitForParallelWorkersToFinish() while its parallel worker is held +# on the first index it claimed. The leader is held before it starts on the +# indexes, so that the worker gets to claim one before the leader takes them +# all. +sub start_leader_waiting_for_worker +{ + my ($node) = @_; + + $node->safe_psql( + 'postgres', q{ + SELECT injection_points_attach('parallel-autovacuum-worker-before-index', 'wait'); + SELECT injection_points_attach('parallel-autovacuum-leader-before-index', 'wait'); + ALTER TABLE test_autovac SET (autovacuum_enabled = true); + }); + $node->wait_for_event('autovacuum worker', + 'parallel-autovacuum-leader-before-index'); + $node->wait_for_event('parallel worker', + 'parallel-autovacuum-worker-before-index'); + $node->safe_psql( + 'postgres', q{ + SELECT injection_points_wakeup('parallel-autovacuum-leader-before-index'); + SELECT injection_points_detach('parallel-autovacuum-leader-before-index'); + }); + $node->poll_query_until( + 'postgres', q{ + SELECT count(*) > 0 FROM pg_stat_activity + WHERE backend_type = 'autovacuum worker' + AND wait_event = 'ParallelFinish' + }) or die "autovacuum leader did not enter ParallelFinish"; +} + +sub release_worker +{ + my ($node) = @_; + + $node->safe_psql( + 'postgres', q{ + SELECT injection_points_wakeup('parallel-autovacuum-worker-before-index'); + SELECT injection_points_detach('parallel-autovacuum-worker-before-index'); + }); +} + +# Test 4: +# A config reload arriving while the leader only waits for its parallel +# worker must still reach the worker before it finishes. + +prepare_for_next_test($node, 4); +$log_offset = -s $node->logfile; + +start_leader_waiting_for_worker($node); + +$node->safe_psql( + 'postgres', qq{ + ALTER SYSTEM SET autovacuum_vacuum_cost_limit = 800; + ALTER SYSTEM SET autovacuum_vacuum_cost_delay = 8; + ALTER SYSTEM SET vacuum_cost_page_miss = 11; + ALTER SYSTEM SET vacuum_cost_page_dirty = 12; + ALTER SYSTEM SET vacuum_cost_page_hit = 13; + SELECT pg_reload_conf(); +}); + +# The waiting leader must pick up the reload on its own. +$node->wait_for_log( + qr/Autovacuum VacuumUpdateCosts\(db=$postgresoid, rel=$testautovacoid, dobalance=yes, cost_limit=800, cost_delay=8 /, + $log_offset); + +release_worker($node); +$node->wait_for_log( + qr/parallel autovacuum worker updated cost params: cost_limit=800, cost_delay=8, cost_page_miss=11, cost_page_dirty=12, cost_page_hit=13/, + $log_offset); +$node->wait_for_log( + qr/automatic vacuum of table "postgres\.public\.test_autovac"/, + $log_offset); +ok(1, "config reload reaches parallel workers while the leader waits"); + +# Test 5: +# Same for a cost limit rebalance, which unlike a reload sends no signal: +# a second autovacuum worker joins the balance while the leader waits. + +$node->safe_psql( + 'postgres', qq{ + ALTER SYSTEM SET autovacuum_max_workers = 2; + SELECT pg_reload_conf(); +}); + +prepare_for_next_test($node, 5); +$node->safe_psql('regress_db2', + 'ALTER TABLE filler SET (autovacuum_enabled = false)'); +$node->safe_psql('regress_db2', 'UPDATE filler SET id = id + 1'); +$log_offset = -s $node->logfile; + +start_leader_waiting_for_worker($node); + +# Second worker -> balance = 2, held there as in test 3. +$node->safe_psql('postgres', + "SELECT injection_points_attach('autovacuum-worker-cost-balanced', 'wait')" +); +$node->safe_psql('regress_db2', + 'ALTER TABLE filler SET (autovacuum_enabled = true)'); +$node->wait_for_log( + qr/VacuumUpdateCosts\(db=$db2oid, rel=$filleroid, dobalance=yes, cost_limit=400,/, + $log_offset); + +# The waiting leader must notice the rebalance on its own. +$node->wait_for_log( + qr/Autovacuum VacuumUpdateCosts\(db=$postgresoid, rel=$testautovacoid, dobalance=yes, cost_limit=400,/, + $log_offset); + +release_worker($node); +$node->wait_for_log( + qr/parallel autovacuum worker updated cost params: cost_limit=400,/, + $log_offset); +$node->wait_for_log( + qr/automatic vacuum of table "postgres\.public\.test_autovac"/, + $log_offset); +ok(1, "cost limit rebalance reaches parallel workers while the leader waits"); + # Release the second worker. $node->safe_psql('postgres', "SELECT injection_points_wakeup('autovacuum-worker-cost-balanced')"); -- 2.55.0