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-05 05:14:01
Message-ID: D4ED6D6A-7999-4F21-B1E4-9C3FCE52A4B5@amazon.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 8/4/21, 6:58 PM, "Kyotaro Horiguchi" <horikyota(dot)ntt(at)gmail(dot)com> wrote:
> Addition to that, while NotifySegmentsReadyForArchive() is notifying
> pending segments, other backends simultaneously reach there are
> blocked until the notification, incuding file creation, finishes. I
> don't think that's great. Couldn't we set lastNotifiedSegment before
> the loop? At the moment a backend decides to notify some segments,
> others no longer need to consider those segments. Even if the backend
> crashes meanwhile, as you mentionied below, it's safe since the
> unnotified segments are notifed after restart.

That seems reasonable to me. It looks like we rely on
RemoveOldXlogFiles() even today for when XLogArchiveNotify() fails. I
updated this in v4 of the patch.

In addition to this change, I also addressed your other feedback by
changing XLogSegNoIsInvalid() to XLogSegNoIsValid() and by moving
record boundary registration to the "if" block for cross-page records.

> Does it work that RegisterRecordBoundaryEntry omits registering of the
> bounary if it finds lastNotifiedSeg have gone too far?

Yeah, there's no reason to add a record boundary if we've already
notified the prior segment. For that to happen, another cross-segment
record would have to be flushed to disk and
NotifySegmentsReadyForArchive() would have to be called before
registering the boundary. With that being said, I don't expect an
extra map entry here and there to impact performance enough for us to
worry about it.

Nathan

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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Bossart, Nathan 2021-08-05 05:15:04 Re: archive status ".ready" files may be created too early
Previous Message Masahiko Sawada 2021-08-05 05:08:57 Re: [BUG] wrong refresh when ALTER SUBSCRIPTION ADD/DROP PUBLICATION