Thread
-
Re: vacuumdb: add --dry-run
Kirill Reshke <reshkekirill@gmail.com> — 2025-12-05T19:56:22Z
On Fri, 5 Dec 2025 at 02:52, Nathan Bossart <nathandbossart@gmail.com> wrote: > > On Thu, Nov 20, 2025 at 04:16:13PM -0600, Nathan Bossart wrote: > > On Thu, Nov 20, 2025 at 05:09:54PM -0500, Corey Huinker wrote: > >> I have no objections to, but I am curious about the factors that went into > >> making dry_run an independent boolean rather than part of vacopts. > > > > My thinking was that it's closer to "echo" and "quiet" than anything in > > vacuumingOptions. Most of that stuff seems geared towards controlling the > > precise behavior of the commands rather than the behavior of the > > application. TBH I think it'd be fine either way. We could probably even > > move "echo" and "quiet" into vacuumingOptions if we really wanted to. > > Yeah, I'm finding myself liking the idea of moving all of these things into > vacuumingOptions so that we don't have to cart around so many bool > arguments. Here's a new patch set that does it this way. > > The remaining question in my mind is where we should let the user know that > we're in dry-run mode. The three options I see are 1) at the beginning of > vacuumdb execution, 2) in the !quiet block for each database, and 3) in > each command (via a comment). In v5, I've added a message to all three, > but I'm eager to hear what folks think. > > -- > nathan Hi! 1) > + <varlistentry> > + <term><option>--dry-run</option></term> > + <listitem> > + <para> > + Print, but do not execute, the vacuum and analyze commands that would > + have been sent to the server (performs a dry run). > + </para> > + </listitem> > + </varlistentry> I compared this smgl section to analogous sections of server utilities (pg_dump, pg_resetwal, pg_rewind), none of them mentions "dry run" in description of what "--dry run" is. So, I think my feeling is that this `(performs a dry run).` is unneeded is correct. 2) > - printf(_("%s: vacuuming database \"%s\"\n"), > - progname, PQdb(conn)); > + printf(_("%s: vacuuming database \"%s\"%s\n"), > + progname, PQdb(conn), > + vacopts->dry_run ? " (dry-run)" : ""); I am also not sure we need this change. Look: ``` reshke@yezzey-cbdb-bench:~/pg$ ./bin/bin/vacuumdb --dry-run vacuumdb: Executing in dry-run mode. vacuumdb: vacuuming database "reshke" (dry-run) ``` We have two lines which say the same. Well, maybe there is value in this change, if we are vacuuming multiple databases, but given that --dry-run produces a lot of `VACUUM ... -- not executed` output, I think It will be obvious that this vacuumdb run does not modify the system. WDYT? Overall, 0001 and 0003 are ok, I don't have an opinion on 0002. -- Best regards, Kirill Reshke