Re: [HACKERS] Dry-run mode for pg_archivecleanup
Excerpts from Gabriele Bartolini's message of mar ene 31 12:29:51 -0300 2012: Hi Robert, sorry for the delay. Il 27/01/12 15:47, Robert Haas ha scritto: This email thread seems to have trailed off without reaching a conclusion. The patch is marked as Waiting on Author in the CommitFest application, but I'm not sure that's accurate. Can we try to nail this down? Here is my final version which embeds comments from Josh. I have also added debug information to be printed in case '-d' is given. FWIW I committed this last week, though I changed the debug message wording slightly -- hope that's OK; it can be adjusted of course. http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=b2e431a4db81a735d1474c4d1565a20b835878c9 -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Dry-run mode for pg_archivecleanup
Ciao Alvaro, Il 06/02/12 16:02, Alvaro Herrera ha scritto: FWIW I committed this last week, though I changed the debug message wording slightly -- hope that's OK; it can be adjusted of course. http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=b2e431a4db81a735d1474c4d1565a20b835878c9 Beautiful! :) Thank you very much. Ciao, Gabriele -- Gabriele Bartolini - 2ndQuadrant Italia PostgreSQL Training, Services and Support gabriele.bartol...@2ndquadrant.it | www.2ndQuadrant.it -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Dry-run mode for pg_archivecleanup
On Tue, Jan 31, 2012 at 10:29 AM, Gabriele Bartolini gabriele.bartol...@2ndquadrant.it wrote: Here is my final version which embeds comments from Josh. I have also added debug information to be printed in case '-d' is given. Looks fine; will mark Ready For Committer. Josh -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Dry-run mode for pg_archivecleanup
Hi Robert,
sorry for the delay.
Il 27/01/12 15:47, Robert Haas ha scritto:
This email thread seems to have trailed off without reaching a
conclusion. The patch is marked as Waiting on Author in the CommitFest
application, but I'm not sure that's accurate. Can we try to nail this
down?
Here is my final version which embeds comments from Josh. I have also
added debug information to be printed in case '-d' is given.
I will update the CommitFest website.
Thanks,
Gabriele
--
Gabriele Bartolini - 2ndQuadrant Italia
PostgreSQL Training, Services and Support
gabriele.bartol...@2ndquadrant.it | www.2ndQuadrant.it
*** a/contrib/pg_archivecleanup/pg_archivecleanup.c
--- b/contrib/pg_archivecleanup/pg_archivecleanup.c
***
*** 36,41 const char *progname;
--- 36,42
/* Options and defaults */
bool debug = false; /* are we debugging? */
+ bool dryrun = false; /* are we performing a dry-run
operation? */
char *archiveLocation;/* where to find the archive? */
char *restartWALFileName; /* the file from which we can restart restore */
***
*** 119,124 CleanupPriorWALFiles(void)
--- 120,137
{
snprintf(WALFilePath, MAXPGPATH, %s/%s,
archiveLocation, xlde-d_name);
+
+ if (dryrun)
+ {
+ /* Prints the name of the file and
+* skips the actual removal of the file
*/
+ fprintf(stdout, %s\n, WALFilePath);
+ if (debug)
+ fprintf(stderr, %s: dry-run
mode suggests removing file \%s\\n,
+ progname,
WALFilePath);
+ continue;
+ }
+
if (debug)
fprintf(stderr, %s: removing file
\%s\\n,
progname, WALFilePath);
***
*** 205,210 usage(void)
--- 218,224
printf( %s [OPTION]... ARCHIVELOCATION OLDESTKEPTWALFILE\n,
progname);
printf(\nOptions:\n);
printf( -d generates debug output (verbose mode)\n);
+ printf( -n shows the names of the files that would
have been removed (dry-run)\n);
printf( --help show this help, then exit\n);
printf( --version output version information, then exit\n);
printf(\n
***
*** 241,253 main(int argc, char **argv)
}
}
! while ((c = getopt(argc, argv, d)) != -1)
{
switch (c)
{
case 'd': /* Debug mode */
debug = true;
break;
default:
fprintf(stderr, Try \%s --help\ for more
information.\n, progname);
exit(2);
--- 255,270
}
}
! while ((c = getopt(argc, argv, dn)) != -1)
{
switch (c)
{
case 'd': /* Debug mode */
debug = true;
break;
+ case 'n': /* Dry-Run mode */
+ dryrun = true;
+ break;
default:
fprintf(stderr, Try \%s --help\ for more
information.\n, progname);
exit(2);
*** a/doc/src/sgml/pgarchivecleanup.sgml
--- b/doc/src/sgml/pgarchivecleanup.sgml
***
*** 98,103 pg_archivecleanup: removing file
archive/00010037000E
--- 98,112
/listitem
/varlistentry
+ varlistentry
+ termoption-n/option/term
+ listitem
+para
+ Print the names of the files that would have been removed on
filenamestdout/ (performs a dry run).
+/para
+ /listitem
+ /varlistentry
+
/variablelist
/para
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Dry-run mode for pg_archivecleanup
Hi Alvaro, Il 27/01/12 23:55, Alvaro Herrera ha scritto: If you do that, please make sure you use two complete separate strings instead of building one from spare parts. This bit is missing the _() stuff though. Currently pg_archivecleanup does not comply with localisation standards. I can put this in the todo list however. Let me know what you think. Cheers, Gabriele -- Gabriele Bartolini - 2ndQuadrant Italia PostgreSQL Training, Services and Support gabriele.bartol...@2ndquadrant.it | www.2ndQuadrant.it -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Dry-run mode for pg_archivecleanup
Excerpts from Gabriele Bartolini's message of mar ene 31 12:31:58 -0300 2012: Hi Alvaro, Il 27/01/12 23:55, Alvaro Herrera ha scritto: If you do that, please make sure you use two complete separate strings instead of building one from spare parts. This bit is missing the _() stuff though. Currently pg_archivecleanup does not comply with localisation standards. I can put this in the todo list however. Let me know what you think. Sorry, for some reason I thought this was pg_basebackup. Programs in contrib don't have localisation support, so it doesn't matter much. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Dry-run mode for pg_archivecleanup
On Sun, Jan 15, 2012 at 5:05 PM, Josh Kupershmidt schmi...@gmail.com wrote: On Sun, Jan 15, 2012 at 3:02 PM, Gabriele Bartolini gabriele.bartol...@2ndquadrant.it wrote: My actual intention was to have the filename as output of the command, in order to easily pipe it to another script. Hence my first choice was to use the stdout channel, considering also that pg_archivecleanup in dry-run mode is harmless and does not touch the content of the directory. Oh, right - I should have re-read your initial email before diving into the patch. That all makes sense given your intended purpose. I guess your goal of constructing some simple way to pass the files which would be removed on to another script is a little different than what I initially thought the patch would be useful for, namely as a testing/debugging aid for an admin. Perhaps both goals could be met by making use of '--debug' together with '--dry-run'. If they are both on, then an additional message like pg_archivecleanup: would remove file ... would be printed to stderr, along with just the filename printed to stdout you already have. This email thread seems to have trailed off without reaching a conclusion. The patch is marked as Waiting on Author in the CommitFest application, but I'm not sure that's accurate. Can we try to nail this down? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Dry-run mode for pg_archivecleanup
On Fri, Jan 27, 2012 at 9:47 AM, Robert Haas robertmh...@gmail.com wrote: On Sun, Jan 15, 2012 at 5:05 PM, Josh Kupershmidt schmi...@gmail.com wrote: On Sun, Jan 15, 2012 at 3:02 PM, Gabriele Bartolini gabriele.bartol...@2ndquadrant.it wrote: My actual intention was to have the filename as output of the command, in order to easily pipe it to another script. Hence my first choice was to use the stdout channel, considering also that pg_archivecleanup in dry-run mode is harmless and does not touch the content of the directory. Oh, right - I should have re-read your initial email before diving into the patch. That all makes sense given your intended purpose. I guess your goal of constructing some simple way to pass the files which would be removed on to another script is a little different than what I initially thought the patch would be useful for, namely as a testing/debugging aid for an admin. Perhaps both goals could be met by making use of '--debug' together with '--dry-run'. If they are both on, then an additional message like pg_archivecleanup: would remove file ... would be printed to stderr, along with just the filename printed to stdout you already have. This email thread seems to have trailed off without reaching a conclusion. The patch is marked as Waiting on Author in the CommitFest application, but I'm not sure that's accurate. Can we try to nail this down? Perhaps my last email was a bit wordy. The only real change I am suggesting for Gabriele's patch is that the message printed to stderr when debug + dryrun are activated be changed to would remove file ... from removing file, i.e around line 124: if (debug) fprintf(stderr, %s: %s file \%s\\n, progname, (dryrun ? would remove : removing), WALFilePath); Other than that little quibble, I thought the patch was fine. Josh -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Dry-run mode for pg_archivecleanup
Excerpts from Josh Kupershmidt's message of vie ene 27 19:43:51 -0300 2012: On Fri, Jan 27, 2012 at 9:47 AM, Robert Haas robertmh...@gmail.com wrote: This email thread seems to have trailed off without reaching a conclusion. The patch is marked as Waiting on Author in the CommitFest application, but I'm not sure that's accurate. Can we try to nail this down? Perhaps my last email was a bit wordy. The only real change I am suggesting for Gabriele's patch is that the message printed to stderr when debug + dryrun are activated be changed to would remove file ... from removing file, i.e around line 124: if (debug) fprintf(stderr, %s: %s file \%s\\n, progname, (dryrun ? would remove : removing), WALFilePath); Other than that little quibble, I thought the patch was fine. If you do that, please make sure you use two complete separate strings instead of building one from spare parts. This bit is missing the _() stuff though. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Dry-run mode for pg_archivecleanup
Hi Josh, Il 15/01/12 01:13, Josh Kupershmidt ha scritto: 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. Thank you for dedicating your time to this matter. I think the actual debugging line: + fprintf(stdout, %s\n, WALFilePath); My actual intention was to have the filename as output of the command, in order to easily pipe it to another script. Hence my first choice was to use the stdout channel, considering also that pg_archivecleanup in dry-run mode is harmless and does not touch the content of the directory. Oh, and I think the removing file... debug message above should not be printed in dryrun-mode, lest we confuse the admin. Yes, I agree with you here. Let me know what you think about my reasons for the stdout channel, then I will send v2. Thanks again! Ciao, Gabriele -- Gabriele Bartolini - 2ndQuadrant Italia PostgreSQL Training, Services and Support gabriele.bartol...@2ndquadrant.it | www.2ndQuadrant.it -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Dry-run mode for pg_archivecleanup
On Sun, Jan 15, 2012 at 3:02 PM, Gabriele Bartolini gabriele.bartol...@2ndquadrant.it wrote: My actual intention was to have the filename as output of the command, in order to easily pipe it to another script. Hence my first choice was to use the stdout channel, considering also that pg_archivecleanup in dry-run mode is harmless and does not touch the content of the directory. Oh, right - I should have re-read your initial email before diving into the patch. That all makes sense given your intended purpose. I guess your goal of constructing some simple way to pass the files which would be removed on to another script is a little different than what I initially thought the patch would be useful for, namely as a testing/debugging aid for an admin. Perhaps both goals could be met by making use of '--debug' together with '--dry-run'. If they are both on, then an additional message like pg_archivecleanup: would remove file ... would be printed to stderr, along with just the filename printed to stdout you already have. Josh -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Dry-run mode for pg_archivecleanup
On Sun, Dec 11, 2011 at 9:52 AM, Gabriele Bartolini gabriele.bartol...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Dry-run mode for pg_archivecleanup
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.
Thanks,
Gabriele
--
Gabriele Bartolini - 2ndQuadrant Italia
PostgreSQL Training, Services and Support
gabriele.bartol...@2ndquadrant.it | www.2ndQuadrant.it
*** a/contrib/pg_archivecleanup/pg_archivecleanup.c
--- b/contrib/pg_archivecleanup/pg_archivecleanup.c
***
*** 36,41 const char *progname;
--- 36,42
/* Options and defaults */
bool debug = false; /* are we debugging? */
+ bool dryrun = false; /* are we performing a dry-run
operation? */
char *archiveLocation;/* where to find the archive? */
char *restartWALFileName; /* the file from which we can restart restore */
***
*** 123,128 CleanupPriorWALFiles(void)
--- 124,136
fprintf(stderr, %s: removing file
\%s\\n,
progname, WALFilePath);
+ if (dryrun)
+ {
+ /* Prints the name of the file and
+* skips the actual removal of the file
*/
+ fprintf(stdout, %s\n, WALFilePath);
+ continue;
+ }
rc = unlink(WALFilePath);
if (rc != 0)
{
***
*** 205,210 usage(void)
--- 213,219
printf( %s [OPTION]... ARCHIVELOCATION OLDESTKEPTWALFILE\n,
progname);
printf(\nOptions:\n);
printf( -d generates debug output (verbose mode)\n);
+ printf( -n shows the names of the files that would
have been removed (dry-run)\n);
printf( --help show this help, then exit\n);
printf( --version output version information, then exit\n);
printf(\n
***
*** 241,253 main(int argc, char **argv)
}
}
! while ((c = getopt(argc, argv, d)) != -1)
{
switch (c)
{
case 'd': /* Debug mode */
debug = true;
break;
default:
fprintf(stderr, Try \%s --help\ for more
information.\n, progname);
exit(2);
--- 250,265
}
}
! while ((c = getopt(argc, argv, dn)) != -1)
{
switch (c)
{
case 'd': /* Debug mode */
debug = true;
break;
+ case 'n': /* Dry-Run mode */
+ dryrun = true;
+ break;
default:
fprintf(stderr, Try \%s --help\ for more
information.\n, progname);
exit(2);
*** a/doc/src/sgml/pgarchivecleanup.sgml
--- b/doc/src/sgml/pgarchivecleanup.sgml
***
*** 98,103 pg_archivecleanup: removing file
archive/00010037000E
--- 98,112
/listitem
/varlistentry
+ varlistentry
+ termoption-n/option/term
+ listitem
+para
+ Print the names of the files that would have been removed on
filenamestdout/ (performs a dry run).
+/para
+ /listitem
+ /varlistentry
+
/variablelist
/para
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Dry-run mode for pg_archivecleanup
On Sun, Dec 11, 2011 at 15:52, Gabriele Bartolini gabriele.bartol...@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. Sounds like a useful feature. But you added it to the wrong commitfest ;) I've moved it to the proper one (2012-01). -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Dry Run mode for pg_archivecleanup
On Tue, Jun 28, 2011 at 6:33 AM, Gabriele Bartolini gabriele.bartol...@2ndquadrant.it wrote: 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. Please add the patch into the next CommitFest. https://commitfest.postgresql.org/action/commitfest_view?id=11 Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Dry Run mode for pg_archivecleanup
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.
Thanks,
Gabriele
--
Gabriele Bartolini - 2ndQuadrant Italia
PostgreSQL Training, Services and Support
gabriele.bartol...@2ndquadrant.it | www.2ndQuadrant.it
From 25fcf05ff787dae2b0d62de423a363f8597f1d42 Mon Sep 17 00:00:00 2001
From: Gabriele Bartolini gabriele.bartol...@2ndquadrant.it
Date: Mon, 27 Jun 2011 12:27:09 +0200
Subject: [PATCH] Add dryrun option to pg_archivecleanup
---
contrib/pg_archivecleanup/pg_archivecleanup.c | 13 -
doc/src/sgml/pgarchivecleanup.sgml|9 +
2 files changed, 21 insertions(+), 1 deletions(-)
diff --git a/contrib/pg_archivecleanup/pg_archivecleanup.c
b/contrib/pg_archivecleanup/pg_archivecleanup.c
index dd8a451..0bbec92 100644
--- a/contrib/pg_archivecleanup/pg_archivecleanup.c
+++ b/contrib/pg_archivecleanup/pg_archivecleanup.c
@@ -36,6 +36,7 @@ const char *progname;
/* Options and defaults */
bool debug = false; /* are we debugging? */
+bool dryrun = false; /* are we performing a dry-run
operation? */
char *archiveLocation;/* where to find the archive? */
char *restartWALFileName; /* the file from which we can restart restore */
@@ -123,6 +124,12 @@ CleanupPriorWALFiles(void)
fprintf(stderr, %s: removing file
\%s\\n,
progname, WALFilePath);
+ if (dryrun) {
+ /* Prints the name of the file and
+* skips the actual removal of the file
*/
+ fprintf(stdout, %s\n, WALFilePath);
+ continue;
+ }
rc = unlink(WALFilePath);
if (rc != 0)
{
@@ -205,6 +212,7 @@ usage(void)
printf( %s [OPTION]... ARCHIVELOCATION OLDESTKEPTWALFILE\n,
progname);
printf(\nOptions:\n);
printf( -d generates debug output (verbose mode)\n);
+ printf( -n shows the names of the files that would
have been removed (dry-run)\n);
printf( --help show this help, then exit\n);
printf( --version output version information, then exit\n);
printf(\n
@@ -241,13 +249,16 @@ main(int argc, char **argv)
}
}
- while ((c = getopt(argc, argv, d)) != -1)
+ while ((c = getopt(argc, argv, dn)) != -1)
{
switch (c)
{
case 'd': /* Debug mode */
debug = true;
break;
+ case 'n': /* Dry-Run mode */
+ dryrun = true;
+ break;
default:
fprintf(stderr, Try \%s --help\ for more
information.\n, progname);
exit(2);
diff --git a/doc/src/sgml/pgarchivecleanup.sgml
b/doc/src/sgml/pgarchivecleanup.sgml
index ddffa32..8148c53 100644
--- a/doc/src/sgml/pgarchivecleanup.sgml
+++ b/doc/src/sgml/pgarchivecleanup.sgml
@@ -98,6 +98,15 @@ pg_archivecleanup: removing file
archive/00010037000E
/listitem
/varlistentry
+ varlistentry
+ termoption-n/option/term
+ listitem
+ para
+Print the names of the files that would have been removed on
filenamestdout/ (performs a dry run).
+ /para
+ /listitem
+ /varlistentry
+
/variablelist
/para
--
1.7.4.1
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
