Re: Use durable_unlink for .ready and .done files for WAL segment removal

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: "Bossart, Nathan" <bossartn(at)amazon(dot)com>
Cc: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Use durable_unlink for .ready and .done files for WAL segment removal
Date: 2018-12-05 01:34:43
Message-ID: 20181205013443.GE2407@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On Tue, Dec 04, 2018 at 08:40:40PM +0000, Bossart, Nathan wrote:
> Thanks for the updated patch! The code looks good to me, the patch
> applies cleanly and builds without warnings, and it seems to work well
> in my manual tests. I just have a few wording suggestions.

How are you testing this? I just stop the server and manually touch
some fake status files in archive_status :)

> I would phrase this comment this way:
>
> Since archive_status files are not durably removed, a system
> crash could leave behind .ready files for WAL segments that
> have already been recycled or removed. In this case, simply
> remove the orphan status file and move on.

Fine for me. Thanks!

> + ereport(WARNING,
> + (errmsg("removed orphan archive status file %s",
> + xlogready)));
>
> I think we should put quotes around the file name like we do elsewhere
> in pgarch_ArchiverCopyLoop().

Done.

> + ereport(WARNING,
> + (errmsg("failed removal of \"%s\" too many times, will try again later",
> + xlogready)));
>
> I'd suggest mirroring the log statement for failed archiving commands
> and saying something like, "removing orphan archive status file \"%s\"
> failed too many times, will try again later." IMO that makes it
> clearer what is failing and why we are removing it in the first place.

"removal of" is more consistent here I think, so changed this way with
your wording merged.
--
Michael

Attachment Content-Type Size
archive-missing-v4.patch text/x-diff 2.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Imai, Yoshikazu 2018-12-05 02:29:36 RE: speeding up planning with partitions
Previous Message Amit Langote 2018-12-05 01:28:31 Re: error message when subscription target is a partitioned table