Re: Dry-run mode for pg_archivecleanup

On Sun, Dec 11, 2011 at 9:52 AM, Gabriele Bartolini
<gabriele.bartolini@2ndquadrant.it> wrote:
> Hi guys,
>
>  I have added the '-n' option to pg_archivecleanup which performs a dry-run
> and outputs the names of the files to be removed to stdout (making possible
> to pass the list via pipe to another process).
>
>  Please find attached the small patch. I submit it to the CommitFest.

Hi Gabriele,

I have signed on to review this patch for the 2012-01 CF. The patch
applies cleanly, includes the necessary documentation, and implements
a useful feature.

I think the actual debugging line:

+                                       fprintf(stdout, "%s\n", WALFilePath);

might need to be tweaked. First, it's printing to stdout, and I think
pg_archivecleanup intentionally sends all its output to stderr, so
that it may show up in the postmaster log. (I expect the dry-run mode
would often be used to test out an archive_cleanup_command, instead of
only in stand-alone mode, where stdout would be fine.)

Also, I think the actual message should be something a little more
descriptive than just the WALFilePath. In debug mode,
pg_archivecleanup prints out a message like:
                   fprintf(stderr, "%s: removing file \"%s\"\n",                           progname, WALFilePath);

I think we'd want to print something similar, i.e. "would remove file ...".

Oh, and I think the "removing file... " debug message above should not
be printed in dryrun-mode, lest we confuse the admin.

Other than that, everything looks good.

Josh