Re: [HACKERS] 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 * 00010010.0020.backup is after * 00010010. 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 00010010.0020.backup or 00010010.0020.partial are after 00010010. 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
Re: [HACKERS] WAL-related tools and .paritial WAL file
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
* 00010010.0020.backup is after
* 00010010.
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
00010010.0020.backup or
00010010.0020.partial are after
00010010. 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
+ applicationpg_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 replaceablearchivelocation/ %r'
para
When used as a standalone program all WAL files logically preceding the
replaceableoldestkeptwalfile/ will be removed from replaceablearchivelocation/.
!In this mode, if you specify a filename.backup/ file name, then only the file prefix
!will be used as the replaceableoldestkeptwalfile/. 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 filename000100370010/:
--- 60,69
para
When used as a standalone program all WAL files logically preceding the
replaceableoldestkeptwalfile/ will be removed from replaceablearchivelocation/.
!In this mode, if you specify a filename.partial/ or filename.backup/
!file name, then only the file prefix will be used as the
!replaceableoldestkeptwalfile/. 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 filename000100370010/:
*** 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
Re: [HACKERS] 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
Re: [HACKERS] WAL-related tools and .paritial WAL file
On Thu, Jul 2, 2015 at 8:59 PM, Michael Paquier michael.paqu...@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
Re: [HACKERS] WAL-related tools and .paritial WAL file
On 1 July 2015 at 07:52, Michael Paquier michael.paqu...@gmail.com wrote: On Wed, Jul 1, 2015 at 12:39 PM, Fujii Masao masao.fu...@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. http://www.postgresql.org/message-id/CAB7nPqSiKU+8HjVe_J05btonC5E7kvmRaLpGS6EaEQe=dy3...@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 Riggshttp://www.2ndQuadrant.com/ http://www.2ndquadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services
Re: [HACKERS] WAL-related tools and .paritial WAL file
On Wed, Jul 1, 2015 at 12:39 PM, Fujii Masao masao.fu...@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. http://www.postgresql.org/message-id/CAB7nPqSiKU+8HjVe_J05btonC5E7kvmRaLpGS6EaEQe=dy3...@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
Re: [HACKERS] WAL-related tools and .paritial WAL file
On Wed, Jul 1, 2015 at 5:14 PM, Simon Riggs si...@2ndquadrant.com wrote:
On 1 July 2015 at 07:52, Michael Paquier michael.paqu...@gmail.com wrote:
On Wed, Jul 1, 2015 at 12:39 PM, Fujii Masao masao.fu...@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.
http://www.postgresql.org/message-id/CAB7nPqSiKU+8HjVe_J05btonC5E7kvmRaLpGS6EaEQe=dy3...@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
*** 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
+ applicationpg_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
* 00010010.0020.backup is after
* 00010010.
*/
--- 192,199
/*
* If restartWALFileName is a WAL file name then just use
Re: [HACKERS] WAL-related tools and .paritial WAL file
On Wed, Jul 1, 2015 at 2:52 PM, Amit Kapila amit.kapil...@gmail.com wrote: On Wed, Jul 1, 2015 at 9:09 AM, Fujii Masao masao.fu...@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 00010006 00020008 pg_xlogdump: FATAL: could not find file 00020006: No such file or directory $ pg_xlogdump -t 1 00010006 00020008 pg_xlogdump: FATAL: could not find file 00020006: 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
Re: [HACKERS] WAL-related tools and .paritial WAL file
On Wed, Jul 1, 2015 at 9:09 AM, Fujii Masao masao.fu...@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
