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

From: "Bossart, Nathan" <bossartn(at)amazon(dot)com>
To: "alvherre(at)alvh(dot)no-ip(dot)org" <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, "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-17 21:09:52
Message-ID: 2A53AFB0-9A44-4EFE-BA83-072D000D9319@amazon.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 8/17/21, 1:24 PM, "alvherre(at)alvh(dot)no-ip(dot)org" <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
> The thing I still don't understand about this patch is why we call
> RegisterSegmentBoundaryEntry and NotifySegmentsReadyForArchive in
> XLogInsertRecord.
>
> My model concept of this would have these routines called only in
> XLogFlush / XLogWrite, which are conceptually about transferring data
> from in-memory WAL buffers into the on-disk WAL store (pg_xlog files).
> As I understand, XLogInsertRecord is about copying bytes from the
> high-level operation (heap insert etc) into WAL buffers. So doing the
> register/notify dance in both places should be redundant and
> unnecessary.

The main reason for registering the boundaries in XLogInsertRecord()
is that it has the required information about the WAL record
boundaries. I do not think that XLogWrite() has this information.
If we assumed that write requests always pointed to record boundaries,
we could probably just move the XLogArchiveNotifySeg() calls to the
end of XLogWrite(), which is what my original patch [0] did.

> In the NotifySegmentsReadyForArchive() call at the bottom of XLogWrite,
> we use flushed=InvalidXLogRecPtr. Why is that? Surely we can use
> LogwrtResult.Flush, just like in the other callsite there? (If we're
> covering for somebody advancing FlushRecPtr concurrently, I think we
> add a comment to explain that. But why do we need to do that in the
> first place?)

Good point. I did this in the new version of the patch.

Nathan

[0] https://postgr.es/m/CBDDFA01-6E40-46BB-9F98-9340F4379505%40amazon.com

Attachment Content-Type Size
v8-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 alvherre@alvh.no-ip.org 2021-08-17 21:11:46 Re: archive status ".ready" files may be created too early
Previous Message Zhihong Yu 2021-08-17 20:56:23 Re: Allow parallel DISTINCT