Thread

1. pg_archivecleanup debug message consistency

   Erik Rijkers <er@xs4all.nl> — 2010-08-21T22:28:34Z

   pg_archivecleanup -d (=verbose/DEBUG mode) mainly emits 2 types of messages:
   
   
   pg_archivecleanup: keep WAL file "000000010000000000000002" and later
   
   and:
   
   pg_archivecleanup: removing file
   "/var/data2/pg_stuff/dump/hotprime/replication_archive/00000001000000000000001B"
   
   
   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

2. Re: pg_archivecleanup debug message consistency

   Tom Lane <tgl@sss.pgh.pa.us> — 2010-08-22T15:54:05Z

   "Erik Rijkers" <er@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
   
   

3. Re: pg_archivecleanup debug message consistency

   Erik Rijkers <er@xs4all.nl> — 2010-08-22T17:53:11Z

   On Sun, August 22, 2010 17:54, Tom Lane wrote:
   > "Erik Rijkers" <er@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
   
   
   
   
   
   

4. Re: pg_archivecleanup debug message consistency

   Tom Lane <tgl@sss.pgh.pa.us> — 2010-08-23T02:57:38Z

   "Erik Rijkers" <er@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