From: | Josh Kupershmidt <schmiddy(at)gmail(dot)com> |
---|---|
To: | Gabriele Bartolini <gabriele(dot)bartolini(at)2ndquadrant(dot)it> |
Cc: | PgHacker <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Dry-run mode for pg_archivecleanup |
Date: | 2012-01-15 00:13:05 |
Message-ID: | CAK3UJREBEv+AV2a9HRuaUWXjzKwbgNo9n-QNHwGcMpUn8eGBGA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sun, Dec 11, 2011 at 9:52 AM, Gabriele Bartolini
<gabriele(dot)bartolini(at)2ndquadrant(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Andrew Dunstan | 2012-01-15 00:13:52 | Re: JSON for PG 9.2 |
Previous Message | Joey Adams | 2012-01-14 23:11:57 | Re: JSON for PG 9.2 |