On Thu, Jul 2, 2015 at 3:48 PM, Michael Paquier
<michael.paqu...@gmail.com> wrote:
> On Thu, Jul 2, 2015 at 2:06 PM, Fujii Masao wrote:
>> I implemented the patch accordingly. Patch attached.
>
> Cool, thanks.
>
> I have done some tests with pg_archivecleanup, and it works as
> expected, aka if I define a backup file, the backup file as well as
> the segments equal or newer than it remain. It works as well when
> defining a .partial file. I have done as well some testing with
> pg_resetxlog and the partial segment gets removed.

Thanks for testing the patch!

Attached is the updated version of the patch.

> Here are some comments:
> 1) pgarchivecleanup.sgml needs to be updated:
>    In this mode, if you specify a <filename>.backup</> file name, then
> only the file prefix
> Here we should mention that it is also the case of a .partial file.

Applied.

> 2)
> -        * restartWALFileName is a .backup filename, make sure we use the 
> prefix
> -        * of the filename, otherwise we will remove wrong files since
> +        * restartWALFileName is a .partial or .backup filename, make
> sure we use
> +        * the prefix of the filename, otherwise we will remove wrong
> files since
>          * 000000010000000000000010.00000020.backup is after
>          * 000000010000000000000010.
> Shouldn't this be made clearer as well regarding .partial files? For
> example with a comment like that:
> otherwise we will remove wrong files since
> 000000010000000000000010.00000020.backup or
> 000000010000000000000010.00000020.partial are after
> 000000010000000000000010. Simply not mentioning those file names
> directly is fine for me.

Applied.

> 3) Something not caused by this patch that I just noticed... But
> pg_resetxlog does not remove .backup files in pg_xlog. Shouldn't they
> get moved away as well?

pg_resetxlog doesn't remove also .history file in pg_xlog. Those remaining
.backup and .history files are harmless after pg_resetxlog is executed.
So I don't think that it's required to remove them. Of course we can do that,
but some existing applications might depend on the current behavior...
So unless there is strong reason to do that, I'd like to let it as it is.

Regards,

-- 
Fujii Masao
*** a/doc/src/sgml/ref/pg_xlogdump.sgml
--- b/doc/src/sgml/ref/pg_xlogdump.sgml
***************
*** 215,220 **** PostgreSQL documentation
--- 215,226 ----
      Only the specified timeline is displayed (or the default, if none is
      specified). Records in other timelines are ignored.
    </para>
+ 
+   <para>
+     <application>pg_xlogdump</> cannot read WAL files with suffix
+     <literal>.partial</>. If those files need to be read, <literal>.partial</>
+     suffix needs to be removed from the filename.
+   </para>
   </refsect1>
  
   <refsect1>
*** a/doc/src/sgml/ref/pgarchivecleanup.sgml
--- b/doc/src/sgml/ref/pgarchivecleanup.sgml
***************
*** 60,67 **** archive_cleanup_command = 'pg_archivecleanup <replaceable>archivelocation</> %r'
    <para>
     When used as a standalone program all WAL files logically preceding the
     <replaceable>oldestkeptwalfile</> will be removed from <replaceable>archivelocation</>.
!    In this mode, if you specify a <filename>.backup</> file name, then only the file prefix
!    will be used as the <replaceable>oldestkeptwalfile</>. This allows you to remove
     all WAL files archived prior to a specific base backup without error.
     For example, the following example will remove all files older than
     WAL file name <filename>000000010000003700000010</>:
--- 60,69 ----
    <para>
     When used as a standalone program all WAL files logically preceding the
     <replaceable>oldestkeptwalfile</> will be removed from <replaceable>archivelocation</>.
!    In this mode, if you specify a <filename>.partial</> or <filename>.backup</>
!    file name, then only the file prefix will be used as the
!    <replaceable>oldestkeptwalfile</>. This treatment of <filename>.backup</>
!    file name allows you to remove
     all WAL files archived prior to a specific base backup without error.
     For example, the following example will remove all files older than
     WAL file name <filename>000000010000003700000010</>:
*** a/src/bin/pg_archivecleanup/pg_archivecleanup.c
--- b/src/bin/pg_archivecleanup/pg_archivecleanup.c
***************
*** 125,131 **** CleanupPriorWALFiles(void)
  			 * file. Note that this means files are not removed in the order
  			 * they were originally written, in case this worries you.
  			 */
! 			if (IsXLogFileName(walfile) &&
  				strcmp(walfile + 8, exclusiveCleanupFileName + 8) < 0)
  			{
  				/*
--- 125,131 ----
  			 * file. Note that this means files are not removed in the order
  			 * they were originally written, in case this worries you.
  			 */
! 			if ((IsXLogFileName(walfile) || IsPartialXLogFileName(walfile)) &&
  				strcmp(walfile + 8, exclusiveCleanupFileName + 8) < 0)
  			{
  				/*
***************
*** 181,187 **** CleanupPriorWALFiles(void)
   * SetWALFileNameForCleanup()
   *
   *	  Set the earliest WAL filename that we want to keep on the archive
!  *	  and decide whether we need_cleanup
   */
  static void
  SetWALFileNameForCleanup(void)
--- 181,187 ----
   * SetWALFileNameForCleanup()
   *
   *	  Set the earliest WAL filename that we want to keep on the archive
!  *	  and decide whether we need cleanup
   */
  static void
  SetWALFileNameForCleanup(void)
***************
*** 192,200 **** SetWALFileNameForCleanup(void)
  
  	/*
  	 * If restartWALFileName is a WAL file name then just use it directly. If
! 	 * restartWALFileName is a .backup filename, make sure we use the prefix
! 	 * of the filename, otherwise we will remove wrong files since
! 	 * 000000010000000000000010.00000020.backup is after
  	 * 000000010000000000000010.
  	 */
  	if (IsXLogFileName(restartWALFileName))
--- 192,201 ----
  
  	/*
  	 * If restartWALFileName is a WAL file name then just use it directly. If
! 	 * restartWALFileName is a .partial or .backup filename, make sure we use
! 	 * the prefix of the filename, otherwise we will remove wrong files since
! 	 * 000000010000000000000010.partial and
! 	 * 000000010000000000000010.00000020.backup are after
  	 * 000000010000000000000010.
  	 */
  	if (IsXLogFileName(restartWALFileName))
***************
*** 202,207 **** SetWALFileNameForCleanup(void)
--- 203,228 ----
  		strcpy(exclusiveCleanupFileName, restartWALFileName);
  		fnameOK = true;
  	}
+ 	else if (IsPartialXLogFileName(restartWALFileName))
+ 	{
+ 		int			args;
+ 		uint32		tli = 1,
+ 					log = 0,
+ 					seg = 0;
+ 
+ 		args = sscanf(restartWALFileName, "%08X%08X%08X.partial",
+ 					  &tli, &log, &seg);
+ 		if (args == 3)
+ 		{
+ 			fnameOK = true;
+ 
+ 			/*
+ 			 * Use just the prefix of the filename, ignore everything after
+ 			 * first period
+ 			 */
+ 			XLogFileNameById(exclusiveCleanupFileName, tli, log, seg);
+ 		}
+ 	}
  	else if (IsBackupHistoryFileName(restartWALFileName))
  	{
  		int			args;
*** a/src/bin/pg_resetxlog/pg_resetxlog.c
--- b/src/bin/pg_resetxlog/pg_resetxlog.c
***************
*** 906,912 **** FindEndOfXLOG(void)
  
  	while (errno = 0, (xlde = readdir(xldir)) != NULL)
  	{
! 		if (IsXLogFileName(xlde->d_name))
  		{
  			unsigned int tli,
  						log,
--- 906,913 ----
  
  	while (errno = 0, (xlde = readdir(xldir)) != NULL)
  	{
! 		if (IsXLogFileName(xlde->d_name) ||
! 			IsPartialXLogFileName(xlde->d_name))
  		{
  			unsigned int tli,
  						log,
***************
*** 976,982 **** KillExistingXLOG(void)
  
  	while (errno = 0, (xlde = readdir(xldir)) != NULL)
  	{
! 		if (IsXLogFileName(xlde->d_name))
  		{
  			snprintf(path, MAXPGPATH, "%s/%s", XLOGDIR, xlde->d_name);
  			if (unlink(path) < 0)
--- 977,984 ----
  
  	while (errno = 0, (xlde = readdir(xldir)) != NULL)
  	{
! 		if (IsXLogFileName(xlde->d_name) ||
! 			IsPartialXLogFileName(xlde->d_name))
  		{
  			snprintf(path, MAXPGPATH, "%s/%s", XLOGDIR, xlde->d_name);
  			if (unlink(path) < 0)
***************
*** 1028,1034 **** KillExistingArchiveStatus(void)
  	{
  		if (strspn(xlde->d_name, "0123456789ABCDEF") == XLOG_FNAME_LEN &&
  			(strcmp(xlde->d_name + XLOG_FNAME_LEN, ".ready") == 0 ||
! 			 strcmp(xlde->d_name + XLOG_FNAME_LEN, ".done") == 0))
  		{
  			snprintf(path, MAXPGPATH, "%s/%s", ARCHSTATDIR, xlde->d_name);
  			if (unlink(path) < 0)
--- 1030,1038 ----
  	{
  		if (strspn(xlde->d_name, "0123456789ABCDEF") == XLOG_FNAME_LEN &&
  			(strcmp(xlde->d_name + XLOG_FNAME_LEN, ".ready") == 0 ||
! 			 strcmp(xlde->d_name + XLOG_FNAME_LEN, ".done") == 0 ||
! 			 strcmp(xlde->d_name + XLOG_FNAME_LEN, ".partial.ready") == 0 ||
! 			 strcmp(xlde->d_name + XLOG_FNAME_LEN, ".partial.done") == 0))
  		{
  			snprintf(path, MAXPGPATH, "%s/%s", ARCHSTATDIR, xlde->d_name);
  			if (unlink(path) < 0)
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to