From 82c1f442ba5e18e3d6dc3cb4ed4f24cd3a8d910f Mon Sep 17 00:00:00 2001 From: Daniil Davidov Date: Tue, 10 Feb 2026 21:31:14 +0700 Subject: [PATCH 7/7] fixes for patch 4 --- src/backend/access/heap/vacuumlazy.c | 7 + src/backend/commands/vacuumparallel.c | 32 ++- src/backend/postmaster/autovacuum.c | 19 +- .../modules/test_autovacuum/t/001_basic.pl | 188 ++++++++---------- 4 files changed, 121 insertions(+), 125 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index d19e15cbcce..2e85f7f17f7 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -151,6 +151,7 @@ #include "storage/freespace.h" #include "storage/lmgr.h" #include "storage/read_stream.h" +#include "utils/injection_point.h" #include "utils/lsyscache.h" #include "utils/pg_rusage.h" #include "utils/timestamp.h" @@ -869,6 +870,12 @@ heap_vacuum_rel(Relation rel, const VacuumParams params, lazy_check_wraparound_failsafe(vacrel); dead_items_alloc(vacrel, params.nworkers); + /* + * Trigger injection point, if parallel autovacuum is about to be started. + */ + if (AmAutoVacuumWorkerProcess() && ParallelVacuumIsActive(vacrel)) + INJECTION_POINT("autovacuum-start-parallel-vacuum", NULL); + /* * Call lazy_scan_heap to perform all required heap pruning, index * vacuuming, and heap vacuuming (plus related processing) diff --git a/src/backend/commands/vacuumparallel.c b/src/backend/commands/vacuumparallel.c index 13649747322..5dad19d8ed8 100644 --- a/src/backend/commands/vacuumparallel.c +++ b/src/backend/commands/vacuumparallel.c @@ -928,6 +928,9 @@ parallel_vacuum_process_all_indexes(ParallelVacuumState *pvs, int num_index_scan * To be able to exercise whether all reserved parallel workers are being * released anyway, allow injection points to trigger a failure at this * point. + * + * This injection point is also used to wait until parallel workers + * finishes their part of index processing. */ if (nworkers > 0) INJECTION_POINT("autovacuum-leader-before-indexes-processing", NULL); @@ -941,15 +944,6 @@ parallel_vacuum_process_all_indexes(ParallelVacuumState *pvs, int num_index_scan */ parallel_vacuum_process_safe_indexes(pvs); - /* - * To be able to exercise whether leader parallel autovacuum worker can - * propagate cost-based params to parallel workers, wait here until - * configuration is changed. I.e. tests are expecting, that during index - * processing vacuum_delay_point have been called (if config was changed). - */ - if (nworkers > 0) - INJECTION_POINT("autovacuum-leader-after-indexes-processing", NULL); - /* * Next, accumulate buffer and WAL usage. (This must wait for the workers * to finish, or we might get incomplete data.) @@ -1315,20 +1309,16 @@ parallel_vacuum_main(dsm_segment *seg, shm_toc *toc) /* Prepare to track buffer usage during parallel execution */ InstrStartParallelQuery(); - INJECTION_POINT("parallel-worker-before-indexes-processing", NULL); - /* Process indexes to perform vacuum/cleanup */ parallel_vacuum_process_safe_indexes(&pvs); #ifdef USE_INJECTION_POINTS /* - * There is no guarantee that each parallel worker will necessarily - * process at least one index. Thus, at this point we cannot be sure that - * worker called vacuum_cost_delay. In order to test cost-based parameters - * propagation (from leader worker), call vacuum_delay_point here, if - * injection point is active. + * If we are parallel autovacuum worker, we can consume delay parameters + * during index processing (via vacuum_delay_point call). This logging + * allows tests to ensure this. */ - if (IS_INJECTION_POINT_ATTACHED("parallel-autovacuum-force-delay-point")) + if (shared->am_parallel_autovacuum) parallel_vacuum_report_cost_based_params(); #endif @@ -1392,6 +1382,7 @@ parallel_vacuum_error_callback(void *arg) static void parallel_vacuum_report_cost_based_params(void) { +#ifdef USE_INJECTION_POINTS StringInfoData buf; /* Simulate config reload during normal processing */ @@ -1402,12 +1393,15 @@ parallel_vacuum_report_cost_based_params(void) initStringInfo(&buf); appendStringInfo(&buf, "Vacuum cost-based delay parameters of parallel worker:\n"); - appendStringInfo(&buf,"vacuum_cost_limit = %d\n",vacuum_cost_limit); + appendStringInfo(&buf, "vacuum_cost_limit = %d\n",vacuum_cost_limit); appendStringInfo(&buf, "vacuum_cost_delay = %g\n", vacuum_cost_delay); appendStringInfo(&buf, "vacuum_cost_page_miss = %d\n", VacuumCostPageMiss); appendStringInfo(&buf, "vacuum_cost_page_dirty = %d\n", VacuumCostPageDirty); appendStringInfo(&buf, "vacuum_cost_page_hit = %d\n", VacuumCostPageHit); - ereport(LOG, errmsg("%s", buf.data)); + ereport(DEBUG2, errmsg("%s", buf.data)); pfree(buf.data); +#else + elog(ERROR, "Injection points are not supported by this build"); +#endif } diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index b9ff60be0f2..7b24a5d6e67 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -800,8 +800,6 @@ ProcessAutoVacLauncherInterrupts(void) /* rebuild the list in case the naptime changed */ rebuild_database_list(InvalidOid); - - INJECTION_POINT("autovacuum-launcher-after-reload-config", NULL); } /* Process barrier events */ @@ -2497,12 +2495,20 @@ do_autovacuum(void) } PG_CATCH(); { + int nreserved_workers = av_nworkers_reserved; + /* * Parallel autovacuum can reserve parallel workers. Make sure * that all reserved workers are released. */ AutoVacuumReleaseAllParallelWorkers(); + if (nreserved_workers > 0) + ereport(DEBUG2, + (errmsg("%d parallel autovacuum workers has been released after occured error", + nreserved_workers), + errhidecontext(true))); + /* * Abort the transaction, start a new one, and proceed with the * next table in our list. @@ -3469,15 +3475,13 @@ AutoVacuumReleaseAllParallelWorkers(void) /* * Get number of free autovacuum parallel workers. - * - * For testing purpose only! */ uint32 AutoVacuumGetFreeParallelWorkers(void) { uint32 nfree_workers; - LWLockAcquire(AutovacuumLock, LW_EXCLUSIVE); + LWLockAcquire(AutovacuumLock, LW_SHARED); nfree_workers = AutoVacuumShmem->av_freeParallelWorkers; LWLockRelease(AutovacuumLock); @@ -3652,5 +3656,10 @@ adjust_free_parallel_workers(int prev_max_parallel_workers) AutoVacuumShmem->av_freeParallelWorkers = Max(nfree_workers, 0); AutoVacuumShmem->av_maxParallelWorkers = autovacuum_max_parallel_workers; + ereport(DEBUG2, + (errmsg("number of free parallel autovacuum workers is set to %u due to config reload", + AutoVacuumShmem->av_freeParallelWorkers), + errhidecontext(true))); + LWLockRelease(AutovacuumLock); } diff --git a/src/test/modules/test_autovacuum/t/001_basic.pl b/src/test/modules/test_autovacuum/t/001_basic.pl index 065a58ef2e6..c5d8fffc47c 100644 --- a/src/test/modules/test_autovacuum/t/001_basic.pl +++ b/src/test/modules/test_autovacuum/t/001_basic.pl @@ -1,3 +1,5 @@ +# Test parallel autovacuum behavior + use warnings FATAL => 'all'; use PostgreSQL::Test::Cluster; use PostgreSQL::Test::Utils; @@ -21,17 +23,9 @@ sub prepare_for_next_test $node->safe_psql('postgres', qq{ UPDATE test_autovac SET col_1 = $test_number; - ANALYZE test_autovac; }); } -sub wait_for_av_log -{ - my ($node, $expected_log) = @_; - - $node->wait_for_log($expected_log); - truncate $node->logfile, 0 or die "truncate failed: $!"; -} my $psql_out; @@ -71,31 +65,30 @@ my $indexes_num = 4; my $initial_rows_num = 10_000; my $autovacuum_parallel_workers = 2; -# Create table with specified number of b-tree indexes on it +# Create table and fill it with some data $node->safe_psql('postgres', qq{ CREATE TABLE test_autovac ( id SERIAL PRIMARY KEY, col_1 INTEGER, col_2 INTEGER, col_3 INTEGER, col_4 INTEGER ) WITH (autovacuum_parallel_workers = $autovacuum_parallel_workers); - DO \$\$ - DECLARE - i INTEGER; - BEGIN - FOR i IN 1..$indexes_num LOOP - EXECUTE format('CREATE INDEX idx_col_\%s ON test_autovac (col_\%s);', i, i); - END LOOP; - END \$\$; + INSERT INTO test_autovac + SELECT + g AS col1, + g + 1 AS col2, + g + 2 AS col3, + g + 3 AS col4 + FROM generate_series(1, $initial_rows_num) AS g; }); -# Insert specified tuples num into the table +# Create specified number of b-tree indexes on the table $node->safe_psql('postgres', qq{ DO \$\$ DECLARE i INTEGER; BEGIN - FOR i IN 1..$initial_rows_num LOOP - INSERT INTO test_autovac VALUES (i, i + 1, i + 2, i + 3); + FOR i IN 1..$indexes_num LOOP + EXECUTE format('CREATE INDEX idx_col_\%s ON test_autovac (col_\%s);', i, i); END LOOP; END \$\$; }); @@ -115,14 +108,15 @@ $node->safe_psql('postgres', qq{ # Wait until the parallel autovacuum on table is completed. At the same time, # we check that the required number of parallel workers has been started. -wait_for_av_log($node, - qr/parallel index vacuum\/cleanup: 2 workers were planned, / . - qr/2 workers were reserved and 2 workers were launched in total/); - -$node->psql('postgres', - "SELECT get_parallel_autovacuum_free_workers();", - stdout => \$psql_out, +$log_start = $node->wait_for_log( + qr/parallel index vacuum: 2 workers were planned, / . + qr/2 workers were reserved and 2 workers were launched in total/, + $log_start ); + +$psql_out = $node->safe_psql('postgres', qq{ + SELECT get_parallel_autovacuum_free_workers(); +}); is($psql_out, 20, 'All parallel workers has been released by the leader'); # Test 2: @@ -132,19 +126,16 @@ is($psql_out, 20, 'All parallel workers has been released by the leader'); prepare_for_next_test($node, 2); $node->safe_psql('postgres', qq{ + SELECT injection_points_attach('autovacuum-start-parallel-vacuum', 'wait'); SELECT injection_points_attach('autovacuum-leader-before-indexes-processing', 'wait'); - SELECT injection_points_attach('autovacuum-leader-after-indexes-processing', 'wait'); - SELECT injection_points_attach('parallel-worker-before-indexes-processing', 'wait'); - SELECT injection_points_attach('parallel-autovacuum-force-delay-point', 'wait'); ALTER TABLE test_autovac SET (autovacuum_parallel_workers = 1, autovacuum_enabled = true); }); -# Wait until parallel autovacuum leader launches parallel worker and falls -# asleep on the injection point +# Wait until parallel autovacuum is inited $node->wait_for_event( 'autovacuum worker', - 'autovacuum-leader-before-indexes-processing' + 'autovacuum-start-parallel-vacuum' ); # Reload config - leader worker must update its own parameters during indexes @@ -158,45 +149,34 @@ $node->safe_psql('postgres', qq{ }); $node->safe_psql('postgres', qq{ - SELECT injection_points_wakeup('autovacuum-leader-before-indexes-processing'); + SELECT injection_points_wakeup('autovacuum-start-parallel-vacuum'); }); -# Wait until leader worker is guaranteed to update parameters and propagate -# their values to the parallel worker +# Now wait until parallel autovacuum leader completes processing table (i.e. +# guaranteed to call vacuum_delay_point) and launches parallel worker. $node->wait_for_event( 'autovacuum worker', - 'autovacuum-leader-after-indexes-processing' + 'autovacuum-leader-before-indexes-processing' ); -$node->safe_psql('postgres', qq{ - SELECT injection_points_wakeup('autovacuum-leader-after-indexes-processing'); -}); - -# Now wake up the parallel worker and force it to call vacuum_delay_point -$node->wait_for_event( - 'parallel worker', - 'parallel-worker-before-indexes-processing' +# Check whether parallel worker successfully updated all parameters during +# index processing +$log_start = $node->wait_for_log( + qr/Vacuum cost-based delay parameters of parallel worker:\n/ . + qr/\tvacuum_cost_limit = 500\n/ . + qr/\tvacuum_cost_delay = 2\n/ . + qr/\tvacuum_cost_page_miss = 10\n/ . + qr/\tvacuum_cost_page_dirty = 10\n/ . + qr/\tvacuum_cost_page_hit = 10\n/, + $log_start ); -$node->safe_psql('postgres', qq{ - SELECT injection_points_wakeup('parallel-worker-before-indexes-processing'); -}); - -# Check whether worker successfully updated all parameters -wait_for_av_log($node, - qr/Vacuum cost-based delay parameters of parallel worker:\n/ . - qr/\tvacuum_cost_limit = 500\n/ . - qr/\tvacuum_cost_delay = 2\n/ . - qr/\tvacuum_cost_page_miss = 10\n/ . - qr/\tvacuum_cost_page_dirty = 10\n/ . - qr/\tvacuum_cost_page_hit = 10\n/); - # Cleanup $node->safe_psql('postgres', qq{ + SELECT injection_points_wakeup('autovacuum-leader-before-indexes-processing'); + + SELECT injection_points_detach('autovacuum-start-parallel-vacuum'); SELECT injection_points_detach('autovacuum-leader-before-indexes-processing'); - SELECT injection_points_detach('autovacuum-leader-after-indexes-processing'); - SELECT injection_points_detach('parallel-worker-before-indexes-processing'); - SELECT injection_points_detach('parallel-autovacuum-force-delay-point'); ALTER TABLE test_autovac SET (autovacuum_parallel_workers = $autovacuum_parallel_workers); }); @@ -209,7 +189,6 @@ prepare_for_next_test($node, 4); $node->safe_psql('postgres', qq{ SELECT injection_points_attach('autovacuum-leader-before-indexes-processing', 'wait'); - SELECT injection_points_attach('autovacuum-launcher-after-reload-config', 'wait'); ALTER TABLE test_autovac SET (autovacuum_enabled = true); }); @@ -223,53 +202,44 @@ $node->safe_psql('postgres', qq{ SELECT pg_reload_conf(); }); -$node->wait_for_event( - 'autovacuum launcher', - 'autovacuum-launcher-after-reload-config' -); - # Since 2 parallel workers already launched and will be released in the future, # we are expecting that : # 1) number of free workers will be '0' after config reload # 2) number of free workers will be '1' after releasing workers # Check statement (1) -$node->psql('postgres', - "SELECT get_parallel_autovacuum_free_workers();", - stdout => \$psql_out, +$log_start = $node->wait_for_log( + qr/number of free parallel autovacuum workers is set to 0 due to config reload/, + $log_start ); -is($psql_out, 0, - 'Number of free parallel workers is consistent'); $node->safe_psql('postgres', qq{ - SELECT injection_points_wakeup('autovacuum-launcher-after-reload-config'); SELECT injection_points_wakeup('autovacuum-leader-before-indexes-processing'); }); # Wait until the end of parallel processing -wait_for_av_log($node, - qr/parallel index vacuum\/cleanup: 2 workers were planned, / . - qr/2 workers were reserved and 2 workers were launched in total/); +$log_start = $node->wait_for_log( + qr/parallel index vacuum: 2 workers were planned, / . + qr/2 workers were reserved and 2 workers were launched in total/, + $log_start +); # Check statement (2) -$node->psql('postgres', - "SELECT get_parallel_autovacuum_free_workers();", - stdout => \$psql_out, -); -is($psql_out, 1, - 'Number of free parallel workers is consistent'); +$psql_out = $node->safe_psql('postgres', qq{ + SELECT get_parallel_autovacuum_free_workers(); +}); +is($psql_out, 1, 'Number of free parallel workers is consistent'); # Cleanup $node->safe_psql('postgres', qq{ SELECT injection_points_detach('autovacuum-leader-before-indexes-processing'); - SELECT injection_points_detach('autovacuum-launcher-after-reload-config'); ALTER SYSTEM SET autovacuum_max_parallel_workers = 10; SELECT pg_reload_conf(); }); # Test 4: # We want parallel autovacuum workers to be released even if leader gets an -# error. At first, simulate situation, when leader exites due to an ERROR. +# error. At first, simulate situation, when leader exits due to an ERROR. prepare_for_next_test($node, 4); @@ -278,16 +248,16 @@ $node->safe_psql('postgres', qq{ ALTER TABLE test_autovac SET (autovacuum_enabled = true); }); -wait_for_av_log($node, - qr/error triggered for injection point / . - qr/autovacuum-leader-before-indexes-processing/); +$log_start = $node->wait_for_log( + qr/error triggered for injection point / . + qr/autovacuum-leader-before-indexes-processing/, + $log_start +); -$node->psql('postgres', - "SELECT get_parallel_autovacuum_free_workers();", - stdout => \$psql_out, +$log_start = $node->wait_for_log( + qr/2 parallel autovacuum workers has been released after occured error/, + $log_start ); -is($psql_out, 10, - 'All parallel workers has been released by the leader after ERROR'); # Cleanup $node->safe_psql('postgres', qq{ @@ -295,15 +265,25 @@ $node->safe_psql('postgres', qq{ }); # Test 5: -# Same as above test, but simulate situation, when leader exites due to FATAL. +# Same as above test, but simulate situation, when leader exits due to FATAL. prepare_for_next_test($node, 5); $node->safe_psql('postgres', qq{ + SELECT injection_points_attach('autovacuum-start-parallel-vacuum', 'wait'); SELECT injection_points_attach('autovacuum-leader-before-indexes-processing', 'wait'); ALTER TABLE test_autovac SET (autovacuum_enabled = true); }); +# Wait until parallel autovacuum is inited and wake up the leader +$node->wait_for_event( + 'autovacuum worker', + 'autovacuum-start-parallel-vacuum' +); +$node->safe_psql('postgres', qq{ + SELECT injection_points_wakeup('autovacuum-start-parallel-vacuum'); +}); + $node->wait_for_event( 'autovacuum worker', 'autovacuum-leader-before-indexes-processing' @@ -327,18 +307,24 @@ $node->safe_psql('postgres', qq{ SELECT pg_terminate_backend('$av_pid'); }); -wait_for_av_log($node, - qr/terminating autovacuum process due to administrator command/); - -$node->psql('postgres', - "SELECT get_parallel_autovacuum_free_workers();", - stdout => \$psql_out, +$log_start = $node->wait_for_log( + qr/terminating autovacuum process due to administrator command/, + $log_start ); -is($psql_out, 10, - 'All parallel workers has been released by the leader after FATAL'); + +# Now it is safe to check the number of free parallel workers, because even if +# autovacuum is trying to vacuum table in parallel mode again, the leader +# worker cannot go any further than "autovacuum-start-parallel-vacuum" point. +# I.e. no one can interfere and change the number of free parallel workers. + +$psql_out = $node->safe_psql('postgres', qq{ + SELECT get_parallel_autovacuum_free_workers(); +}); +is($psql_out, 10, 'All parallel workers has been released by the leader after FATAL'); # Cleanup $node->safe_psql('postgres', qq{ + SELECT injection_points_detach('autovacuum-start-parallel-vacuum'); SELECT injection_points_detach('autovacuum-leader-before-indexes-processing'); }); -- 2.43.0