On Wed, Nov 19, 2025 at 6:55 PM Corey Huinker <[email protected]> wrote:
> On Wed, Nov 19, 2025 at 5:44 PM Nathan Bossart <[email protected]> > wrote: > >> On Wed, Nov 19, 2025 at 05:23:48PM -0500, Corey Huinker wrote: >> > Now with zero hangs and some test cases. I didn't create a function >> (yet) >> > as it seemed trivial. >> >> I still think it could be worth moving the dry-run code into >> run_vacuum_command() (which might entail moving the calls to >> ParallelSlotSetHandler() there, too). We can probably piggy-back on the >> "if (echo)" branch in that function. >> > > We _could_ get away with moving ParallelSlotGetIdle() in there too. The > only catch would be that we'd have to refactor prepare_vacuum_command() to > take a serverVersionNumber parameter instead of the whole connection. > Thoughts? > > >> >> Also, we can probably skip the executeCommand() calls for >> --analyze-in-stages. >> > > +1 > Here's a shopping list of incremental changes. I'm happy with whatever makes the most sense to you.
From 396f243e729e2aaaf59cd820def4e093e51033bc Mon Sep 17 00:00:00 2001 From: Corey Huinker <[email protected]> Date: Mon, 10 Nov 2025 14:33:41 -0500 Subject: [PATCH v3 1/5] Add --dry-run to vacuumdb. This option answers the question "what tables would be affected if I ran a command using these options" without actually initiating those actions. --- src/bin/scripts/t/100_vacuumdb.pl | 12 +++++++++ src/bin/scripts/vacuumdb.c | 6 +++++ src/bin/scripts/vacuuming.c | 42 +++++++++++++++++++++++-------- src/bin/scripts/vacuuming.h | 1 + doc/src/sgml/ref/vacuumdb.sgml | 11 ++++++++ 5 files changed, 61 insertions(+), 11 deletions(-) diff --git a/src/bin/scripts/t/100_vacuumdb.pl b/src/bin/scripts/t/100_vacuumdb.pl index a16fad593f7..9fef3cbb80f 100644 --- a/src/bin/scripts/t/100_vacuumdb.pl +++ b/src/bin/scripts/t/100_vacuumdb.pl @@ -169,6 +169,10 @@ $node->issues_sql_like( [ 'vacuumdb', '--schema' => '"Foo"', 'postgres' ], qr/VACUUM \(SKIP_DATABASE_STATS\) "Foo".bar/, 'vacuumdb --schema'); +$node->issues_sql_unlike( + [ 'vacuumdb', '--schema' => '"Foo"', 'postgres', '--dry-run' ], + qr/VACUUM \(SKIP_DATABASE_STATS\) "Foo".bar/, + 'vacuumdb --schema'); $node->issues_sql_like( [ 'vacuumdb', '--schema' => '"Foo"', '--schema' => '"Bar"', 'postgres' ], qr/VACUUM\ \(SKIP_DATABASE_STATS\)\ "Foo".bar @@ -241,6 +245,14 @@ $node->safe_psql('postgres', q| CREATE TABLE regression_vacuumdb_test AS select generate_series(1, 10) a, generate_series(2, 11) b; ALTER TABLE regression_vacuumdb_test ADD COLUMN c INT GENERATED ALWAYS AS (a + b); |); +$node->issues_sql_unlike( + [ + 'vacuumdb', '--analyze-only', '--dry-run', + '--missing-stats-only', '-t', + 'regression_vacuumdb_test', 'postgres' + ], + qr/statement:\ ANALYZE/sx, + '--missing-stats-only --dry-run with missing stats'); $node->issues_sql_like( [ 'vacuumdb', '--analyze-only', diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c index e117dac2242..aa0dc366eb0 100644 --- a/src/bin/scripts/vacuumdb.c +++ b/src/bin/scripts/vacuumdb.c @@ -59,6 +59,7 @@ main(int argc, char *argv[]) {"no-process-main", no_argument, NULL, 12}, {"buffer-usage-limit", required_argument, NULL, 13}, {"missing-stats-only", no_argument, NULL, 14}, + {"dry-run", no_argument, NULL, 15}, {NULL, 0, NULL, 0} }; @@ -86,6 +87,7 @@ main(int argc, char *argv[]) vacopts.do_truncate = true; vacopts.process_main = true; vacopts.process_toast = true; + vacopts.dry_run = false; /* the same for connection parameters */ memset(&cparams, 0, sizeof(cparams)); @@ -213,6 +215,9 @@ main(int argc, char *argv[]) case 14: vacopts.missing_stats_only = true; break; + case 15: + vacopts.dry_run = true; + break; default: /* getopt_long already emitted a complaint */ pg_log_error_hint("Try \"%s --help\" for more information.", progname); @@ -375,6 +380,7 @@ help(const char *progname) printf(_(" -Z, --analyze-only only update optimizer statistics; no vacuum\n")); printf(_(" --analyze-in-stages only update optimizer statistics, in multiple\n" " stages for faster results; no vacuum\n")); + printf(_(" --dry-run do not vacuum/analyze the selected tables, only print\n")); printf(_(" -?, --help show this help, then exit\n")); printf(_("\nConnection options:\n")); printf(_(" -h, --host=HOSTNAME database server host or socket directory\n")); diff --git a/src/bin/scripts/vacuuming.c b/src/bin/scripts/vacuuming.c index f836f21fb03..5d9c07f8715 100644 --- a/src/bin/scripts/vacuuming.c +++ b/src/bin/scripts/vacuuming.c @@ -378,13 +378,25 @@ vacuum_one_database(ConnParams *cparams, prepare_vacuum_command(free_slot->connection, &sql, vacopts, tabname); - /* - * Execute the vacuum. All errors are handled in processQueryResult - * through ParallelSlotsGetIdle. - */ - ParallelSlotSetHandler(free_slot, TableCommandResultHandler, NULL); - run_vacuum_command(free_slot->connection, sql.data, - echo, tabname); + if (vacopts->dry_run) + { + /* + * Print the command that we would have run in a real run, + * the immediately mark the unused slot as free again. + */ + printf("not executed: %s\n", sql.data); + free_slot->inUse = false; + } + else + { + /* + * Execute the vacuum. All errors are handled in processQueryResult + * through ParallelSlotsGetIdle. + */ + ParallelSlotSetHandler(free_slot, TableCommandResultHandler, NULL); + run_vacuum_command(free_slot->connection, sql.data, + echo, tabname); + } cell = cell->next; } while (cell != NULL); @@ -407,11 +419,19 @@ vacuum_one_database(ConnParams *cparams, goto finish; } - ParallelSlotSetHandler(free_slot, TableCommandResultHandler, NULL); - run_vacuum_command(free_slot->connection, cmd, echo, NULL); + if (vacopts->dry_run) + { + printf("not executed: %s\n", cmd); + free_slot->inUse = false; + } + else + { + ParallelSlotSetHandler(free_slot, TableCommandResultHandler, NULL); + run_vacuum_command(free_slot->connection, cmd, echo, NULL); - if (!ParallelSlotsWaitCompletion(sa)) - ret = EXIT_FAILURE; /* error already reported by handler */ + if (!ParallelSlotsWaitCompletion(sa)) + ret = EXIT_FAILURE; /* error already reported by handler */ + } } finish: diff --git a/src/bin/scripts/vacuuming.h b/src/bin/scripts/vacuuming.h index 49f968b32e5..50155239e7e 100644 --- a/src/bin/scripts/vacuuming.h +++ b/src/bin/scripts/vacuuming.h @@ -51,6 +51,7 @@ typedef struct vacuumingOptions bool skip_database_stats; char *buffer_usage_limit; bool missing_stats_only; + bool dry_run; } vacuumingOptions; /* Valid values for vacuumingOptions->objfilter */ diff --git a/doc/src/sgml/ref/vacuumdb.sgml b/doc/src/sgml/ref/vacuumdb.sgml index 84c76d7350c..100691b579a 100644 --- a/doc/src/sgml/ref/vacuumdb.sgml +++ b/doc/src/sgml/ref/vacuumdb.sgml @@ -171,6 +171,17 @@ PostgreSQL documentation </listitem> </varlistentry> + <varlistentry> + <term><option>--dry-run</option></term> + <listitem> + <para> + Print but do not execute the vacuum or analyze commands generated. + This is useful for testing the effects of various command-line options + before actually running the commands. + </para> + </listitem> + </varlistentry> + <varlistentry> <term><option>-e</option></term> <term><option>--echo</option></term> base-commit: 6b46669883fac9521c20fe4e2c55ccfbee778591 -- 2.51.1
From 8ce09dd5c83191827f2faa874edd12610c6852e1 Mon Sep 17 00:00:00 2001 From: Corey Huinker <[email protected]> Date: Wed, 19 Nov 2025 19:13:45 -0500 Subject: [PATCH v3 2/5] switch from passing conn to passing slot --- src/bin/scripts/vacuuming.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/bin/scripts/vacuuming.c b/src/bin/scripts/vacuuming.c index 5d9c07f8715..f1fa565679c 100644 --- a/src/bin/scripts/vacuuming.c +++ b/src/bin/scripts/vacuuming.c @@ -43,8 +43,8 @@ static SimpleStringList *retrieve_objects(PGconn *conn, static void free_retrieved_objects(SimpleStringList *list); static void prepare_vacuum_command(PGconn *conn, PQExpBuffer sql, vacuumingOptions *vacopts, const char *table); -static void run_vacuum_command(PGconn *conn, const char *sql, bool echo, - const char *table); +static void run_vacuum_command(ParallelSlot *free_slot, const char *sql, + bool echo, const char *table); /* * Executes vacuum/analyze as indicated. Returns 0 if the plan is carried @@ -394,7 +394,7 @@ vacuum_one_database(ConnParams *cparams, * through ParallelSlotsGetIdle. */ ParallelSlotSetHandler(free_slot, TableCommandResultHandler, NULL); - run_vacuum_command(free_slot->connection, sql.data, + run_vacuum_command(free_slot, sql.data, echo, tabname); } @@ -427,7 +427,7 @@ vacuum_one_database(ConnParams *cparams, else { ParallelSlotSetHandler(free_slot, TableCommandResultHandler, NULL); - run_vacuum_command(free_slot->connection, cmd, echo, NULL); + run_vacuum_command(free_slot, cmd, echo, NULL); if (!ParallelSlotsWaitCompletion(sa)) ret = EXIT_FAILURE; /* error already reported by handler */ @@ -1021,10 +1021,11 @@ prepare_vacuum_command(PGconn *conn, PQExpBuffer sql, * Any errors during command execution are reported to stderr. */ static void -run_vacuum_command(PGconn *conn, const char *sql, bool echo, +run_vacuum_command(ParallelSlot *free_slot, const char *sql, bool echo, const char *table) { bool status; + PGconn *conn = free_slot->connection; if (echo) printf("%s\n", sql); -- 2.51.1
From 93130f08a24abef26001199ae17c8812f31df3e5 Mon Sep 17 00:00:00 2001 From: Corey Huinker <[email protected]> Date: Wed, 19 Nov 2025 19:24:26 -0500 Subject: [PATCH v3 3/5] move dry_run test inside run_vacuum_command() --- src/bin/scripts/vacuuming.c | 57 ++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 33 deletions(-) diff --git a/src/bin/scripts/vacuuming.c b/src/bin/scripts/vacuuming.c index f1fa565679c..f52af273d7e 100644 --- a/src/bin/scripts/vacuuming.c +++ b/src/bin/scripts/vacuuming.c @@ -44,7 +44,7 @@ static void free_retrieved_objects(SimpleStringList *list); static void prepare_vacuum_command(PGconn *conn, PQExpBuffer sql, vacuumingOptions *vacopts, const char *table); static void run_vacuum_command(ParallelSlot *free_slot, const char *sql, - bool echo, const char *table); + bool echo, bool dry_run, const char *table); /* * Executes vacuum/analyze as indicated. Returns 0 if the plan is carried @@ -378,25 +378,8 @@ vacuum_one_database(ConnParams *cparams, prepare_vacuum_command(free_slot->connection, &sql, vacopts, tabname); - if (vacopts->dry_run) - { - /* - * Print the command that we would have run in a real run, - * the immediately mark the unused slot as free again. - */ - printf("not executed: %s\n", sql.data); - free_slot->inUse = false; - } - else - { - /* - * Execute the vacuum. All errors are handled in processQueryResult - * through ParallelSlotsGetIdle. - */ - ParallelSlotSetHandler(free_slot, TableCommandResultHandler, NULL); - run_vacuum_command(free_slot, sql.data, - echo, tabname); - } + run_vacuum_command(free_slot, sql.data, + echo, vacopts->dry_run, tabname); cell = cell->next; } while (cell != NULL); @@ -419,19 +402,10 @@ vacuum_one_database(ConnParams *cparams, goto finish; } - if (vacopts->dry_run) - { - printf("not executed: %s\n", cmd); - free_slot->inUse = false; - } - else - { - ParallelSlotSetHandler(free_slot, TableCommandResultHandler, NULL); - run_vacuum_command(free_slot, cmd, echo, NULL); + run_vacuum_command(free_slot, cmd, echo, vacopts->dry_run, NULL); - if (!ParallelSlotsWaitCompletion(sa)) - ret = EXIT_FAILURE; /* error already reported by handler */ - } + if (!ParallelSlotsWaitCompletion(sa)) + ret = EXIT_FAILURE; /* error already reported by handler */ } finish: @@ -1022,11 +996,28 @@ prepare_vacuum_command(PGconn *conn, PQExpBuffer sql, */ static void run_vacuum_command(ParallelSlot *free_slot, const char *sql, bool echo, - const char *table) + bool dry_run, const char *table) { bool status; PGconn *conn = free_slot->connection; + if (dry_run) + { + /* + * Print the command that we would have run in a real run, + * the immediately mark the unused slot as free again. + */ + printf("not executed: %s\n", sql); + free_slot->inUse = false; + return; + } + + /* + * Execute the vacuum. All errors are handled in processQueryResult + * through ParallelSlotsGetIdle. + */ + ParallelSlotSetHandler(free_slot, TableCommandResultHandler, NULL); + if (echo) printf("%s\n", sql); -- 2.51.1
From 2c0cf278b8e1c239fe90dcf2320f0c8d5c28646d Mon Sep 17 00:00:00 2001 From: Corey Huinker <[email protected]> Date: Wed, 19 Nov 2025 19:36:59 -0500 Subject: [PATCH v3 4/5] switch prepare from conn to serverVersion --- src/bin/scripts/vacuuming.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/bin/scripts/vacuuming.c b/src/bin/scripts/vacuuming.c index f52af273d7e..14d843bed10 100644 --- a/src/bin/scripts/vacuuming.c +++ b/src/bin/scripts/vacuuming.c @@ -41,7 +41,7 @@ static SimpleStringList *retrieve_objects(PGconn *conn, SimpleStringList *objects, bool echo); static void free_retrieved_objects(SimpleStringList *list); -static void prepare_vacuum_command(PGconn *conn, PQExpBuffer sql, +static void prepare_vacuum_command(int serverVersion, PQExpBuffer sql, vacuumingOptions *vacopts, const char *table); static void run_vacuum_command(ParallelSlot *free_slot, const char *sql, bool echo, bool dry_run, const char *table); @@ -178,6 +178,7 @@ vacuum_one_database(ConnParams *cparams, SimpleStringList *retobjs = NULL; bool free_retobjs = false; int ret = EXIT_SUCCESS; + int serverVersion; const char *stage_commands[] = { "SET default_statistics_target=1; SET vacuum_cost_delay=0;", "SET default_statistics_target=10; RESET vacuum_cost_delay;", @@ -351,6 +352,7 @@ vacuum_one_database(ConnParams *cparams, * for the first slot. If not in parallel mode, the first slot in the * array contains the connection. */ + serverVersion = PQserverVersion(conn); sa = ParallelSlotsSetup(concurrentCons, cparams, progname, echo, initcmd); ParallelSlotsAdoptConn(sa, conn); @@ -375,7 +377,7 @@ vacuum_one_database(ConnParams *cparams, goto finish; } - prepare_vacuum_command(free_slot->connection, &sql, + prepare_vacuum_command(serverVersion, &sql, vacopts, tabname); run_vacuum_command(free_slot, sql.data, @@ -823,10 +825,9 @@ free_retrieved_objects(SimpleStringList *list) * depends on the server version involved and it is semicolon-terminated. */ static void -prepare_vacuum_command(PGconn *conn, PQExpBuffer sql, +prepare_vacuum_command(int serverVersion, PQExpBuffer sql, vacuumingOptions *vacopts, const char *table) { - int serverVersion = PQserverVersion(conn); const char *paren = " ("; const char *comma = ", "; const char *sep = paren; -- 2.51.1
From ebd78b3a71c99ae36d934f844c36296407e3e23e Mon Sep 17 00:00:00 2001 From: Corey Huinker <[email protected]> Date: Wed, 19 Nov 2025 19:50:05 -0500 Subject: [PATCH v3 5/5] handle ParallelSlotGetIdle() inside run_vacuum_command() --- src/bin/scripts/vacuuming.c | 41 ++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/src/bin/scripts/vacuuming.c b/src/bin/scripts/vacuuming.c index 14d843bed10..5bd7ade62a1 100644 --- a/src/bin/scripts/vacuuming.c +++ b/src/bin/scripts/vacuuming.c @@ -43,7 +43,7 @@ static SimpleStringList *retrieve_objects(PGconn *conn, static void free_retrieved_objects(SimpleStringList *list); static void prepare_vacuum_command(int serverVersion, PQExpBuffer sql, vacuumingOptions *vacopts, const char *table); -static void run_vacuum_command(ParallelSlot *free_slot, const char *sql, +static bool run_vacuum_command(ParallelSlotArray *sa, const char *sql, bool echo, bool dry_run, const char *table); /* @@ -362,7 +362,6 @@ vacuum_one_database(ConnParams *cparams, do { const char *tabname = cell->val; - ParallelSlot *free_slot; if (CancelRequested) { @@ -370,19 +369,14 @@ vacuum_one_database(ConnParams *cparams, goto finish; } - free_slot = ParallelSlotsGetIdle(sa, NULL); - if (!free_slot) + prepare_vacuum_command(serverVersion, &sql, vacopts, tabname); + + if (!run_vacuum_command(sa, sql.data, echo, vacopts->dry_run, tabname)) { ret = EXIT_FAILURE; goto finish; } - prepare_vacuum_command(serverVersion, &sql, - vacopts, tabname); - - run_vacuum_command(free_slot, sql.data, - echo, vacopts->dry_run, tabname); - cell = cell->next; } while (cell != NULL); @@ -396,16 +390,13 @@ vacuum_one_database(ConnParams *cparams, if (vacopts->mode == MODE_VACUUM && vacopts->skip_database_stats) { const char *cmd = "VACUUM (ONLY_DATABASE_STATS);"; - ParallelSlot *free_slot = ParallelSlotsGetIdle(sa, NULL); - if (!free_slot) + if (!run_vacuum_command(sa, cmd, echo, vacopts->dry_run, NULL)) { ret = EXIT_FAILURE; goto finish; } - run_vacuum_command(free_slot, cmd, echo, vacopts->dry_run, NULL); - if (!ParallelSlotsWaitCompletion(sa)) ret = EXIT_FAILURE; /* error already reported by handler */ } @@ -995,24 +986,29 @@ prepare_vacuum_command(int serverVersion, PQExpBuffer sql, * * Any errors during command execution are reported to stderr. */ -static void -run_vacuum_command(ParallelSlot *free_slot, const char *sql, bool echo, +static bool +run_vacuum_command(ParallelSlotArray *sa, const char *sql, bool echo, bool dry_run, const char *table) + { + ParallelSlot *free_slot; bool status; - PGconn *conn = free_slot->connection; + PGconn *conn; if (dry_run) { /* - * Print the command that we would have run in a real run, - * the immediately mark the unused slot as free again. + * Print the command that we would have run in a real run. */ printf("not executed: %s\n", sql); - free_slot->inUse = false; - return; + return true; } + free_slot = ParallelSlotsGetIdle(sa, NULL); + + if (!free_slot) + return false; + /* * Execute the vacuum. All errors are handled in processQueryResult * through ParallelSlotsGetIdle. @@ -1022,6 +1018,7 @@ run_vacuum_command(ParallelSlot *free_slot, const char *sql, bool echo, if (echo) printf("%s\n", sql); + conn = free_slot->connection; status = PQsendQuery(conn, sql) == 1; if (!status) @@ -1037,6 +1034,8 @@ run_vacuum_command(ParallelSlot *free_slot, const char *sql, bool echo, PQdb(conn), PQerrorMessage(conn)); } } + + return true; } /* -- 2.51.1
