Re: [HACKERS] pg_archivecleanup debug message consistency

2010-08-22 Thread Tom Lane 
Erik Rijkers e...@xs4all.nl writes:
 If only for consistency, this patch adds the path info to that message.

Seems reasonable, but speaking of consistency:

 +#ifdef WIN32
 + snprintf(WALFilePath, MAXPGPATH, %s\\%s, 
 archiveLocation, exclusiveCleanupFileName);
 +#else
 + snprintf(WALFilePath, MAXPGPATH, %s/%s, 
 archiveLocation, exclusiveCleanupFileName);
 +#endif

I see that you copied-and-pasted this pattern from somewhere else in
pg_archivecleanup.c, but I'd like to argue that it's out of place there
too.  We don't go out of our way to show Windows paths with backslashes
anywhere in the core code, so why is pg_archivecleanup doing it?  I
think we should just drop the ifdef and do %s/%s always.

regards, tom lane

-- 
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] pg_archivecleanup debug message consistency

2010-08-22 Thread Erik Rijkers 
On Sun, August 22, 2010 17:54, Tom Lane wrote:
 Erik Rijkers e...@xs4all.nl writes:
 If only for consistency, this patch adds the path info to that message.

 Seems reasonable, but speaking of consistency:

 +#ifdef WIN32
 +snprintf(WALFilePath, MAXPGPATH, %s\\%s, 
 archiveLocation, exclusiveCleanupFileName);
 +#else
 +snprintf(WALFilePath, MAXPGPATH, %s/%s, 
 archiveLocation, exclusiveCleanupFileName);
 +#endif

 I see that you copied-and-pasted this pattern from somewhere else in
 pg_archivecleanup.c, but I'd like to argue that it's out of place there
 too.  We don't go out of our way to show Windows paths with backslashes
 anywhere in the core code, so why is pg_archivecleanup doing it?  I
 think we should just drop the ifdef and do %s/%s always.


yes, I agree that's better; attached is that change.

And it works fine on linux; but I am not in a position to try it on windows.


Erik Rijkers





--- contrib/pg_archivecleanup/pg_archivecleanup.c.orig	2010-08-22 18:31:03.0 +0200
+++ contrib/pg_archivecleanup/pg_archivecleanup.c	2010-08-22 18:33:12.0 +0200
@@ -117,12 +117,7 @@
 			strspn(xlde-d_name, 0123456789ABCDEF) == XLOG_DATA_FNAME_LEN 
 strcmp(xlde-d_name + 8, exclusiveCleanupFileName + 8)  0)
 			{
-#ifdef WIN32
-snprintf(WALFilePath, MAXPGPATH, %s\\%s, archiveLocation, xlde-d_name);
-#else
 snprintf(WALFilePath, MAXPGPATH, %s/%s, archiveLocation, xlde-d_name);
-#endif
-
 if (debug)
 	fprintf(stderr, %s: removing file \%s\\n,
 			progname, WALFilePath);
@@ -308,8 +303,10 @@
 	SetWALFileNameForCleanup();
 
 	if (debug)
-		fprintf(stderr, %s: keep WAL file \%s\ and later\n,
-progname, exclusiveCleanupFileName);
+	{
+		snprintf(WALFilePath, MAXPGPATH, %s/%s, archiveLocation, exclusiveCleanupFileName);
+		fprintf(stderr, %s: keep WAL file \%s\ and later\n, progname, WALFilePath);
+	}
 
 	/*
 	 * Remove WAL files older than cut-off
-- 
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] pg_archivecleanup debug message consistency

2010-08-22 Thread Tom Lane 
Erik Rijkers e...@xs4all.nl writes:
 yes, I agree that's better; attached is that change.

Looks good, applied to HEAD and 9.0.  (I also snuck in a couple of
cosmetic cleanups while I was looking at the file.)

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] pg_archivecleanup debug message consistency

2010-08-21 Thread Erik Rijkers 
pg_archivecleanup -d (=verbose/DEBUG mode) mainly emits 2 types of messages:


pg_archivecleanup: keep WAL file 00010002 and later

and:

pg_archivecleanup: removing file
/var/data2/pg_stuff/dump/hotprime/replication_archive/0001001B


I found it a bit annoying to not see the full path in the 'keep WAL 
file'-message (esp. when it is
repeated many screenfulls).

If only for consistency, this patch adds the path info to that message.


Erik Rijkers--- contrib/pg_archivecleanup/pg_archivecleanup.c.orig	2010-08-22 00:05:06.0 +0200
+++ contrib/pg_archivecleanup/pg_archivecleanup.c	2010-08-21 23:56:47.0 +0200
@@ -308,8 +308,15 @@
 	SetWALFileNameForCleanup();
 
 	if (debug)
+	{
+#ifdef WIN32
+snprintf(WALFilePath, MAXPGPATH, %s\\%s, archiveLocation, exclusiveCleanupFileName);
+#else
+snprintf(WALFilePath, MAXPGPATH, %s/%s, archiveLocation, exclusiveCleanupFileName);
+#endif
 		fprintf(stderr, %s: keep WAL file \%s\ and later\n,
-progname, exclusiveCleanupFileName);
+progname, WALFilePath);
+	}
 
 	/*
 	 * Remove WAL files older than cut-off
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers