Re: archive status ".ready" files may be created too early

From: "Bossart, Nathan" <bossartn(at)amazon(dot)com>
To: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
Cc: "alvherre(at)alvh(dot)no-ip(dot)org" <alvherre(at)alvh(dot)no-ip(dot)org>, "x4mmm(at)yandex-team(dot)ru" <x4mmm(at)yandex-team(dot)ru>, "a(dot)lubennikova(at)postgrespro(dot)ru" <a(dot)lubennikova(at)postgrespro(dot)ru>, "hlinnaka(at)iki(dot)fi" <hlinnaka(at)iki(dot)fi>, "matsumura(dot)ryo(at)fujitsu(dot)com" <matsumura(dot)ryo(at)fujitsu(dot)com>, "masao(dot)fujii(at)gmail(dot)com" <masao(dot)fujii(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: archive status ".ready" files may be created too early
Date: 2021-08-06 00:21:34
Message-ID: 839F3FB0-CAFB-455D-9010-F894A6978FAC@amazon.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 8/5/21, 12:39 AM, "Kyotaro Horiguchi" <horikyota(dot)ntt(at)gmail(dot)com> wrote:
> Honestly I don't like having this initialization in XLogWrite. We
> should and I think can initialize it earlier. It seems to me the most
> appropriate timing to initialize the variable is just before running
> the end-of-recovery checkpoint). Since StartupXLOG knows the first
> segment to write . If it were set to 0, that doesn't matter at all.
> We can get rid of the new symbol by doing this.

This seems like a good idea to me. I made this change in v5. I
performed some basic testing, and it seems to reliably initialize
lastNotifiedSeg correctly.

> + /*
> + * EndOfLog resides on the next segment of the last finished one. Set the
> + * last finished segment as lastNotifiedSeg now. In the case where the
> + * last crash has left the last several segments not being marked as
> + * .ready, the checkpoint just after does that for all finished segments.
> + * There's a corner case where the checkpoint advances segment, but that
> + * ends up at most with a duplicate archive notification.
> + */

I'm not quite following the corner case you've described here. Is it
possible that the segment that EndOfLog points to will be eligible for
removal after the checkpoint?

In v5 of the patch, I've also added an extra call to
NotifySegmentsReadyForArchive() in the same place we previously
created the .ready files. I think this helps notify archiver sooner
in certain cases (e.g., asynchronous commit).

Nathan

Attachment Content-Type Size
v5-0001-Avoid-creating-archive-status-.ready-files-too-ea.patch application/octet-stream 19.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro Horiguchi 2021-08-06 01:26:06 Re: .ready and .done files considered harmful
Previous Message Tom Lane 2021-08-06 00:13:03 Re: Assert triggered during RE_compile_and_cache