debug log in pg_archivecleanup

Started by Fujii Masaoover 15 years ago5 messages
#1Fujii Masao
masao.fujii@gmail.com

Hi,

Sometimes the postgres server log message and the pg_archivecleanup debug
message are output in the same line as follows. This is a little hard to
read.

-----------------------
LOG: restored log file "00000001000000000000006B" from archive
pg_archivecleanup: keep WAL file 000000010000000000000068 and later
pg_archivecleanup: removing
"/dav/head-pgsql/act.arh/000000010000000000000048"LOG: restored log
file "00000001000000000000006C" from archive

pg_archivecleanup: removing "/dav/head-pgsql/act.arh/000000010000000000000061"
pg_archivecleanup: removing "/dav/head-pgsql/act.arh/00000001000000000000004D"
pg_archivecleanup: removing "/dav/head-pgsql/act.arh/00000001000000000000005C"
-----------------------

This is because pg_archivecleanup puts the line break "\n" in the head of
debug message. Why should we do so?

-----------------------
if (debug)
fprintf(stderr, "\n%s: removing \"%s\"", progname, WALFilePath);
-----------------------

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

#2Takahiro Itagaki
itagaki.takahiro@oss.ntt.co.jp
In reply to: Fujii Masao (#1)
Re: debug log in pg_archivecleanup

Fujii Masao <masao.fujii@gmail.com> wrote:

This is because pg_archivecleanup puts the line break "\n" in the head of
debug message. Why should we do so?

-----------------------
if (debug)
fprintf(stderr, "\n%s: removing \"%s\"", progname, WALFilePath);
-----------------------

We also need "\n" at line 308.
L.125: fprintf(stderr, "\n%s: removing \"%s\"", progname, WALFilePath);
L.308: fprintf(stderr, "%s: keep WAL file %s and later", progname, exclusiveCleanupFileName);

Note that we don't need a line break at Line 130
because strerror() fills the last %s.
L.130: fprintf(stderr, "\n%s: ERROR failed to remove \"%s\": %s",

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center

#3Fujii Masao
masao.fujii@gmail.com
In reply to: Takahiro Itagaki (#2)
1 attachment(s)
Re: debug log in pg_archivecleanup

On Wed, Jun 16, 2010 at 12:24 PM, Takahiro Itagaki
<itagaki.takahiro@oss.ntt.co.jp> wrote:

Fujii Masao <masao.fujii@gmail.com> wrote:

This is because pg_archivecleanup puts the line break "\n" in the head of
debug message. Why should we do so?

-----------------------
 if (debug)
    fprintf(stderr, "\n%s:  removing \"%s\"", progname, WALFilePath);
-----------------------

We also need "\n" at line 308.
 L.125: fprintf(stderr, "\n%s:  removing \"%s\"", progname, WALFilePath);
 L.308: fprintf(stderr, "%s:  keep WAL file %s and later", progname, exclusiveCleanupFileName);

Yes. What about the attached patch?

Note that we don't need a line break at Line 130
because strerror() fills the last %s.
 L.130: fprintf(stderr, "\n%s: ERROR failed to remove \"%s\": %s",

Right.

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Attachments:

archivecleanup_line_break_v1.patchapplication/octet-stream; name=archivecleanup_line_break_v1.patchDownload
*** a/contrib/pg_archivecleanup/pg_archivecleanup.c
--- b/contrib/pg_archivecleanup/pg_archivecleanup.c
***************
*** 122,140 **** CleanupPriorWALFiles(void)
  #endif
  
  				if (debug)
! 					fprintf(stderr, "\n%s:  removing \"%s\"", progname, WALFilePath);
  
  				rc = unlink(WALFilePath);
  				if (rc != 0)
  				{
! 					fprintf(stderr, "\n%s: ERROR failed to remove \"%s\": %s",
  							progname, WALFilePath, strerror(errno));
  					break;
  				}
  			}
  		}
- 		if (debug)
- 			fprintf(stderr, "\n");
  	}
  	else
  		fprintf(stderr, "%s: archiveLocation \"%s\" open error\n", progname, archiveLocation);
--- 122,138 ----
  #endif
  
  				if (debug)
! 					fprintf(stderr, "%s:  removing \"%s\"\n", progname, WALFilePath);
  
  				rc = unlink(WALFilePath);
  				if (rc != 0)
  				{
! 					fprintf(stderr, "%s: ERROR failed to remove \"%s\": %s",
  							progname, WALFilePath, strerror(errno));
  					break;
  				}
  			}
  		}
  	}
  	else
  		fprintf(stderr, "%s: archiveLocation \"%s\" open error\n", progname, archiveLocation);
***************
*** 305,311 **** main(int argc, char **argv)
  
  	if (debug)
  	{
! 		fprintf(stderr, "%s:  keep WAL file %s and later", progname, exclusiveCleanupFileName);
  		fflush(stderr);
  	}
  
--- 303,309 ----
  
  	if (debug)
  	{
! 		fprintf(stderr, "%s:  keep WAL file %s and later\n", progname, exclusiveCleanupFileName);
  		fflush(stderr);
  	}
  
#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Fujii Masao (#3)
Re: debug log in pg_archivecleanup

Fujii Masao <masao.fujii@gmail.com> writes:

On Wed, Jun 16, 2010 at 12:24 PM, Takahiro Itagaki
<itagaki.takahiro@oss.ntt.co.jp> wrote:

This is because pg_archivecleanup puts the line break "\n" in the head of
debug message. Why should we do so?

Yes. What about the attached patch?

Applied along with a bit of further editorialization.

Note that we don't need a line break at Line 130
because strerror() fills the last %s.
�L.130: fprintf(stderr, "\n%s: ERROR failed to remove \"%s\": %s",

Right.

Huh? strerror() doesn't include a newline.

regards, tom lane

#5Simon Riggs
simon@2ndQuadrant.com
In reply to: Tom Lane (#4)
Re: debug log in pg_archivecleanup

On Thu, 2010-06-17 at 13:33 -0400, Tom Lane wrote:

Fujii Masao <masao.fujii@gmail.com> writes:

On Wed, Jun 16, 2010 at 12:24 PM, Takahiro Itagaki
<itagaki.takahiro@oss.ntt.co.jp> wrote:

This is because pg_archivecleanup puts the line break "\n" in the head of
debug message. Why should we do so?

Yes. What about the attached patch?

Applied along with a bit of further editorialization.

Note that we don't need a line break at Line 130
because strerror() fills the last %s.
L.130: fprintf(stderr, "\n%s: ERROR failed to remove \"%s\": %s",

Right.

Huh? strerror() doesn't include a newline.

Thanks.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Training and Services