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>
+ <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)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
