WAL-related tools and .paritial WAL file

Started by Fujii Masaoover 10 years ago10 messages
#1Fujii Masao
masao.fujii@gmail.com

Hi,

WAL-related tools, i.e., pg_archivecleanup, pg_resetxlog and
pg_xlogdump don't seem to properly handle .paritial WAL file.
I think that we should fix at least pg_archivecleanup, otherwise,
in the system using pg_archivecleanup to clean up old archived
files, the archived .paritial WAL file will never be removed.
Thought?

To make pg_archivecleanup detect .paritial WAL file, I'd like to
use IsPartialXLogFileName() macro that commit 179cdd0 added.
Fortunately Michael proposed the patch which makes the macros
in xlog_internal.h usable in WAL-related tools, and it's better
to apply the fix based on his patch. So my plan is to apply his
patch first and then apply the fix to pg_archivecleanup.
/messages/by-id/CAB7nPqSiKU+8HjVe_J05btonC5E7kvmRaLpGS6EaEQe=Dy3jnQ@mail.gmail.com

Regarding pg_resetxlog and pg_xlogdump, do we need to change
also them so that they can handle .paritial file properly?

pg_resetxlog cannot clean up .partial file even if it should be
removed. But the remaining .paritial file is harmless and will be
recycled later by checkpoint, so extra treatment of .paritial
file in pg_resetxlog may not be necessary. IOW, maybe we can
document that's the limitation of pg_resetxlog.

Also regarding pg_xlogdump, we can just document, for example,
"please get rid of .paritial suffix from the WAL file name if
you want to dump it by pg_xlogdump".

Thought?

Regards,

--
Fujii Masao

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

#2Amit Kapila
amit.kapila16@gmail.com
In reply to: Fujii Masao (#1)
Re: WAL-related tools and .paritial WAL file

On Wed, Jul 1, 2015 at 9:09 AM, Fujii Masao <masao.fujii@gmail.com> wrote:

Also regarding pg_xlogdump, we can just document, for example,
"please get rid of .paritial suffix from the WAL file name if
you want to dump it by pg_xlogdump".

Can't we skip such files in pg_xlogdump?

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

#3Michael Paquier
michael.paquier@gmail.com
In reply to: Fujii Masao (#1)
Re: WAL-related tools and .paritial WAL file

On Wed, Jul 1, 2015 at 12:39 PM, Fujii Masao <masao.fujii@gmail.com> wrote:

WAL-related tools, i.e., pg_archivecleanup, pg_resetxlog and
pg_xlogdump don't seem to properly handle .paritial WAL file.
I think that we should fix at least pg_archivecleanup, otherwise,
in the system using pg_archivecleanup to clean up old archived
files, the archived .paritial WAL file will never be removed.
Thought?

+1 to handle it properly in pg_archivecleanup. If people use the
archive cleanup command in recovery.conf and they remove WAL files
before the fork point of promotion they should not see the partial
file as well.

To make pg_archivecleanup detect .paritial WAL file, I'd like to
use IsPartialXLogFileName() macro that commit 179cdd0 added.
Fortunately Michael proposed the patch which makes the macros
in xlog_internal.h usable in WAL-related tools, and it's better
to apply the fix based on his patch. So my plan is to apply his
patch first and then apply the fix to pg_archivecleanup.
/messages/by-id/CAB7nPqSiKU+8HjVe_J05btonC5E7kvmRaLpGS6EaEQe=Dy3jnQ@mail.gmail.com

As we already use extensively XLogFromFileName in a couple of frontend
utilies, I suspect that the buildfarm is not going to blow up, but it
is certainly better to have certitude with a full buildfarm cycle
running on it...

Regarding pg_resetxlog and pg_xlogdump, do we need to change
also them so that they can handle .paritial file properly?
pg_resetxlog cannot clean up .partial file even if it should be
removed. But the remaining .paritial file is harmless and will be
recycled later by checkpoint, so extra treatment of .paritial
file in pg_resetxlog may not be necessary. IOW, maybe we can
document that's the limitation of pg_resetxlog.

I would rather vote for having pg_resetxlog remove the .partial
segment as well. That's a two-liner in KillExistingArchiveStatus and
KillExistingXLOG at quick glance. It looks to me that as pg_resetxlog
is a reset utility, it should do its jib.

Also regarding pg_xlogdump, we can just document, for example,
"please get rid of .paritial suffix from the WAL file name if
you want to dump it by pg_xlogdump".

For pg_xlogdump, I am on board for only documenting the limitation
(renaming the file by yourself) as it is after all only a debug tool.
This would also make fuzzy_open_file more complicated by having to
detect two times more potential grammars... That's a bit crazy for a
very narrow use-case.
--
Michael

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

#4Michael Paquier
michael.paquier@gmail.com
In reply to: Amit Kapila (#2)
Re: WAL-related tools and .paritial WAL file

On Wed, Jul 1, 2015 at 2:52 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:

On Wed, Jul 1, 2015 at 9:09 AM, Fujii Masao <masao.fujii@gmail.com> wrote:

Also regarding pg_xlogdump, we can just document, for example,
"please get rid of .paritial suffix from the WAL file name if
you want to dump it by pg_xlogdump".

Can't we skip such files in pg_xlogdump?

.partial files are at the end of a timeline, so they will be ignored
now (and this even if a node on the old timeline archives the same
segment as the .partial one sent by the promoted standby). And as
pg_xlogdump is not able to follow a timeline jump there is nothing we
would need to do:
$ pg_xlogdump 000000010000000000000006 000000020000000000000008
pg_xlogdump: FATAL: could not find file "000000020000000000000006":
No such file or directory
$ pg_xlogdump -t 1 000000010000000000000006 000000020000000000000008
pg_xlogdump: FATAL: could not find file "000000020000000000000006":
No such file or directory
I am not convinced that it is worth to make pg_xlogdump follow
timeline jumps as well.. One could just run it twice on the old and
new timeline segments to get the output he wants.
--
Michael

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

#5Simon Riggs
simon@2ndQuadrant.com
In reply to: Michael Paquier (#3)
Re: WAL-related tools and .paritial WAL file

On 1 July 2015 at 07:52, Michael Paquier <michael.paquier@gmail.com> wrote:

On Wed, Jul 1, 2015 at 12:39 PM, Fujii Masao <masao.fujii@gmail.com>
wrote:

WAL-related tools, i.e., pg_archivecleanup, pg_resetxlog and
pg_xlogdump don't seem to properly handle .paritial WAL file.
I think that we should fix at least pg_archivecleanup, otherwise,
in the system using pg_archivecleanup to clean up old archived
files, the archived .paritial WAL file will never be removed.
Thought?

+1 to handle it properly in pg_archivecleanup. If people use the
archive cleanup command in recovery.conf and they remove WAL files
before the fork point of promotion they should not see the partial
file as well.

To make pg_archivecleanup detect .paritial WAL file, I'd like to
use IsPartialXLogFileName() macro that commit 179cdd0 added.
Fortunately Michael proposed the patch which makes the macros
in xlog_internal.h usable in WAL-related tools, and it's better
to apply the fix based on his patch. So my plan is to apply his
patch first and then apply the fix to pg_archivecleanup.

/messages/by-id/CAB7nPqSiKU+8HjVe_J05btonC5E7kvmRaLpGS6EaEQe=Dy3jnQ@mail.gmail.com

As we already use extensively XLogFromFileName in a couple of frontend
utilies, I suspect that the buildfarm is not going to blow up, but it
is certainly better to have certitude with a full buildfarm cycle
running on it...

Regarding pg_resetxlog and pg_xlogdump, do we need to change
also them so that they can handle .paritial file properly?
pg_resetxlog cannot clean up .partial file even if it should be
removed. But the remaining .paritial file is harmless and will be
recycled later by checkpoint, so extra treatment of .paritial
file in pg_resetxlog may not be necessary. IOW, maybe we can
document that's the limitation of pg_resetxlog.

I would rather vote for having pg_resetxlog remove the .partial
segment as well. That's a two-liner in KillExistingArchiveStatus and
KillExistingXLOG at quick glance. It looks to me that as pg_resetxlog
is a reset utility, it should do its jib.

Also regarding pg_xlogdump, we can just document, for example,
"please get rid of .paritial suffix from the WAL file name if
you want to dump it by pg_xlogdump".

For pg_xlogdump, I am on board for only documenting the limitation
(renaming the file by yourself) as it is after all only a debug tool.
This would also make fuzzy_open_file more complicated by having to
detect two times more potential grammars... That's a bit crazy for a
very narrow use-case.

+1 to all

--
Simon Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/&gt;
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#6Fujii Masao
masao.fujii@gmail.com
In reply to: Simon Riggs (#5)
1 attachment(s)
Re: WAL-related tools and .paritial WAL file

On Wed, Jul 1, 2015 at 5:14 PM, Simon Riggs <simon@2ndquadrant.com> wrote:

On 1 July 2015 at 07:52, Michael Paquier <michael.paquier@gmail.com> wrote:

On Wed, Jul 1, 2015 at 12:39 PM, Fujii Masao <masao.fujii@gmail.com>
wrote:

WAL-related tools, i.e., pg_archivecleanup, pg_resetxlog and
pg_xlogdump don't seem to properly handle .paritial WAL file.
I think that we should fix at least pg_archivecleanup, otherwise,
in the system using pg_archivecleanup to clean up old archived
files, the archived .paritial WAL file will never be removed.
Thought?

+1 to handle it properly in pg_archivecleanup. If people use the
archive cleanup command in recovery.conf and they remove WAL files
before the fork point of promotion they should not see the partial
file as well.

To make pg_archivecleanup detect .paritial WAL file, I'd like to
use IsPartialXLogFileName() macro that commit 179cdd0 added.
Fortunately Michael proposed the patch which makes the macros
in xlog_internal.h usable in WAL-related tools, and it's better
to apply the fix based on his patch. So my plan is to apply his
patch first and then apply the fix to pg_archivecleanup.

/messages/by-id/CAB7nPqSiKU+8HjVe_J05btonC5E7kvmRaLpGS6EaEQe=Dy3jnQ@mail.gmail.com

As we already use extensively XLogFromFileName in a couple of frontend
utilies, I suspect that the buildfarm is not going to blow up, but it
is certainly better to have certitude with a full buildfarm cycle
running on it...

Regarding pg_resetxlog and pg_xlogdump, do we need to change
also them so that they can handle .paritial file properly?
pg_resetxlog cannot clean up .partial file even if it should be
removed. But the remaining .paritial file is harmless and will be
recycled later by checkpoint, so extra treatment of .paritial
file in pg_resetxlog may not be necessary. IOW, maybe we can
document that's the limitation of pg_resetxlog.

I would rather vote for having pg_resetxlog remove the .partial
segment as well. That's a two-liner in KillExistingArchiveStatus and
KillExistingXLOG at quick glance. It looks to me that as pg_resetxlog
is a reset utility, it should do its jib.

Also regarding pg_xlogdump, we can just document, for example,
"please get rid of .paritial suffix from the WAL file name if
you want to dump it by pg_xlogdump".

For pg_xlogdump, I am on board for only documenting the limitation
(renaming the file by yourself) as it is after all only a debug tool.
This would also make fuzzy_open_file more complicated by having to
detect two times more potential grammars... That's a bit crazy for a
very narrow use-case.

+1 to all

I also agree with Michael. I implemented the patch accordingly. Patch attached.

Regards,

--
Fujii Masao

Attachments:

archivecleanup-partial-file.patchtext/x-patch; charset=US-ASCII; name=archivecleanup-partial-file.patchDownload
*** 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/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,199 **** 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.
  	 */
--- 192,199 ----
  
  	/*
  	 * 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.00000020.backup is after
  	 * 000000010000000000000010.
  	 */
***************
*** 202,207 **** SetWALFileNameForCleanup(void)
--- 202,227 ----
  		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)
#7Michael Paquier
michael.paquier@gmail.com
In reply to: Fujii Masao (#6)
Re: WAL-related tools and .paritial WAL file

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.

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.
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.
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?
Regards,
-- 
Michael

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

#8Fujii Masao
masao.fujii@gmail.com
In reply to: Michael Paquier (#7)
1 attachment(s)
Re: WAL-related tools and .paritial WAL file

On Thu, Jul 2, 2015 at 3:48 PM, Michael Paquier
<michael.paquier@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

Attachments:

archivecleanup-partial-file_v2.patchtext/x-patch; charset=US-ASCII; name=archivecleanup-partial-file_v2.patchDownload
*** 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)
#9Michael Paquier
michael.paquier@gmail.com
In reply to: Fujii Masao (#8)
Re: WAL-related tools and .paritial WAL file

On Thu, Jul 2, 2015 at 8:42 PM, Fujii Masao wrote:

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.

Well, I was just surprised to not see them wiped out. Let's not change
the behavior then that exists for ages. The rest of the patch looks
fine to me (I would add dots at the end of sentences in comment
blocks, but that's a detail). The new macros really make the code
easier to read and understand!
--
Michael

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

#10Fujii Masao
masao.fujii@gmail.com
In reply to: Michael Paquier (#9)
Re: WAL-related tools and .paritial WAL file

On Thu, Jul 2, 2015 at 8:59 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Thu, Jul 2, 2015 at 8:42 PM, Fujii Masao wrote:

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.

Well, I was just surprised to not see them wiped out. Let's not change
the behavior then that exists for ages. The rest of the patch looks
fine to me (I would add dots at the end of sentences in comment
blocks, but that's a detail). The new macros really make the code
easier to read and understand!

Yep!

Applied the patch. Thanks!

Regards,

--
Fujii Masao

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