From: | "Bossart, Nathan" <bossartn(at)amazon(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
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 16:11:23 |
Message-ID: | DB47EBD7-7291-4C39-9F8F-BE42E5193BD5@amazon.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 12/4/18, 7:35 PM, "Michael Paquier" <michael(at)paquier(dot)xyz> wrote:
> 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 :)
That's almost exactly what I was doing, too.
>> 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.
The v4 patch looks good to me!
Nathan
From | Date | Subject | |
---|---|---|---|
Next Message | Daniel Gustafsson | 2018-12-05 16:22:25 | Hint and detail punctuation |
Previous Message | Alvaro Herrera | 2018-12-05 16:09:54 | Re: psql display of foreign keys |